-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-35111][SPARK-35112][SQL][FOLLOWUP] Rename ANSI interval patterns and regexps #32444
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
Conversation
|
FYI @MaxGekk |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138177 has finished for PR 32444 at commit
|
| private val yearMonthStringPattern = | ||
| "(?i)^(INTERVAL\\s+)([+|-])?(')([+|-])?(\\d+)-(\\d+)(')(\\s+YEAR\\s+TO\\s+MONTH)$".r | ||
| private val unquotedYearMonthRegex = "([+|-])?(\\d+)-(\\d+)".r | ||
| private val quotedYearMonthPattern = (s"^$unquotedYearMonthRegex$$").r |
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.
I don't really see what "quoted" means here...
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.
I don't really see what "quoted" means here...
he may want to mean it without ^ and $
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.
that's not quoted. To me it's more like regex string vs compiled regex.
How about yearMonthPatternString, yearMonthRegex and yearMontnLiteralRegex?
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.
Look good, done
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138252 has finished for PR 32444 at commit
|
|
Test build #138254 has finished for PR 32444 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #138274 has finished for PR 32444 at commit
|
|
+1, LGTM. Merging to master. |
What changes were proposed in this pull request?
Rename pattern strings and regexps of year-month and day-time intervals.
Why are the changes needed?
To improve code maintainability.
Does this PR introduce any user-facing change?
No
How was this patch tested?
By existing test suites.