-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[CALCITE-6015] AssertionError during optimization of EXTRACT expression #3435
Conversation
Kudos, SonarCloud Quality Gate passed! |
f7f8ab3
to
3674357
Compare
Kudos, SonarCloud Quality Gate passed! |
This PR implements several instances of the EXTRACT function which weren't supported. I would appreciate a review. |
Can anyone please review this PR? |
b385f8c
to
6090510
Compare
core/src/main/java/org/apache/calcite/sql/fun/SqlExtractFunction.java
Outdated
Show resolved
Hide resolved
Thanks @mihaibudiu , no further comment from my side. LGTM. |
Squashed the commits in preparation for merging. |
Thanks @mihaibudiu ! |
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.
Thanks for asking
and thanks for addressing comments
it looks ok from my side
f.checkFails("^extract(decade from time '12:34:56')^", fail, false); | ||
f.checkFails("^extract(century from time '12:34:56')^", fail, false); | ||
f.checkFails("^extract(century from time '12:34:56')^", fail, false); | ||
f.checkFails("^extract(century from time '12:34:56')^", fail, false); |
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.
noticed some duplicated tests, I will delete these before merging.
Signed-off-by: Mihai Budiu <[email protected]>
Quality Gate passedIssues Measures |
While attempting to fix the bug described in the issue I have discovered that many cases for EXTRACT are actually not implemented. So I implemented most of them, and I have improved the validation as well; the case indicated in the JIRA issue will now be rejected statically.
There is one case for EXTRACT which I haven't implemented:
EXTRACT(EPOCH FROM INTERVAL '...' YEAR TO MONTH)
because I wasn't sure how to properly use the convertlets to do it. Help would be appreciated.This PR also makes some unexpected tiny changes in SqlValidatorImpl and SqlCall, so I would appreciate comments from people who are more knowledgeable.