[SPARK-30245][SQL][FOLLOWUP] Improve regex expression when pattern not changed #27497
Conversation
|
Test build #118058 has finished for PR 27497 at commit
|
|
Test build #118065 has finished for PR 27497 at commit
|
|
If you're interested in this, could you answer the @HyukjinKwon comment? #26875 (comment) |
Oh, yeah. I see that. We need more investigation. |
| Pattern.compile(escape(str)) | ||
| } | ||
|
|
||
| var lastPatternStr: String = null |
There was a problem hiding this comment.
You can keep lastPattern as Pattern, and compare it to the current pattern via lastPattern.pattern() == patternStr. No need to keep lastPattern as a string.
There was a problem hiding this comment.
Thanks for your review.
Of course，I can do like this. I just followed the way of RegExpExtract and RegExpReplace here.
| val regex = if (cache == null) compile(s) else cache | ||
| val patternStr = input2.asInstanceOf[UTF8String].toString | ||
| val regex = if (cache == null) { | ||
| if (!(patternStr).equals(lastPatternStr)) { |
There was a problem hiding this comment.
Are there any reasons to not use patternStr != lastPatternStr?
There was a problem hiding this comment.
oh, I see this is a copy-paste from Java codegen
|
I agree with @maropu and @HyukjinKwon 's opinion. Without resolving their concerns, we should not merge this because this might not be the final follow-up. |
|
@dongjoon-hyun @HyukjinKwon Yes. maybe we need more performance investigation, but I find I think the pattern string or regex string not too long. |
|
I will close this and the jira because of #26875 (comment) |
|
@maropu Thanks! |
|
Thank you so much for investigations @beliefer. |
|
@HyukjinKwon With pleasure do it. |
6 participants
What changes were proposed in this pull request?
This PR follows up #26875.
Why are the changes needed?
When pattern is not static, we should avoid compile pattern every time if some pattern is same.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Exists UT.