New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-30759][SQL][3.0] Fix cache initialization in StringRegexExpression #27713
[SPARK-30759][SQL][3.0] Fix cache initialization in StringRegexExpression #27713
Conversation
…ingRegexExpression ### What changes were proposed in this pull request? Added new test to `RegexpExpressionsSuite` which checks that `cache` of compiled pattern is set when the `right` expression (pattern in `LIKE`) is a foldable expression. ### Why are the changes needed? To be sure that `cache` in `StringRegexExpression` is initialized for foldable patterns. ### Does this PR introduce any user-facing change? No ### How was this patch tested? By running the added test in `RegexpExpressionsSuite`. Closes apache#27547 from MaxGekk/regexp-cache-test. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan @dongjoon-hyun Please, take a look at the PR. |
I'm fine to have it in 3.0. It's definitely a bug. |
LGTM +1 as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. Thank you, @MaxGekk , @cloud-fan , @rednaxelafx .
Test build #118985 has finished for PR 27713 at commit
|
…sion ### What changes were proposed in this pull request? In the PR, I propose to fix `cache` initialization in `StringRegexExpression` by changing of expected value type in `case Literal(value: String, StringType)` from `String` to `UTF8String`. This is a backport of #27502 and #27547 ### Why are the changes needed? Actually, the case doesn't work at all because `Literal`'s value has type `UTF8String`, see <img width="649" alt="Screen Shot 2020-02-08 at 22 45 50" src="https://user-images.githubusercontent.com/1580697/74091681-0d4a2180-4acb-11ea-8a0d-7e8c65f4214e.png"> ### Does this PR introduce any user-facing change? No ### How was this patch tested? Added new test by `RegexpExpressionsSuite`. Closes #27713 from MaxGekk/str-regexp-foldable-pattern-backport. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
…sion In the PR, I propose to fix `cache` initialization in `StringRegexExpression` by changing of expected value type in `case Literal(value: String, StringType)` from `String` to `UTF8String`. This is a backport of #27502 and #27547 Actually, the case doesn't work at all because `Literal`'s value has type `UTF8String`, see <img width="649" alt="Screen Shot 2020-02-08 at 22 45 50" src="https://user-images.githubusercontent.com/1580697/74091681-0d4a2180-4acb-11ea-8a0d-7e8c65f4214e.png"> No Added new test by `RegexpExpressionsSuite`. Closes #27713 from MaxGekk/str-regexp-foldable-pattern-backport. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit cfc48a8) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Merged to |
What changes were proposed in this pull request?
In the PR, I propose to fix
cache
initialization inStringRegexExpression
by changing of expected value type incase Literal(value: String, StringType)
fromString
toUTF8String
.This is a backport of #27502 and #27547
Why are the changes needed?
Actually, the case doesn't work at all because
Literal
's value has typeUTF8String
, seeDoes this PR introduce any user-facing change?
No
How was this patch tested?
Added new test by
RegexpExpressionsSuite
.