-
Notifications
You must be signed in to change notification settings - Fork 315
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
[AMORO-3424] Add the self-optimizing.partition-filter parameter #3426
[AMORO-3424] Add the self-optimizing.partition-filter parameter #3426
Conversation
Here is my current simple implementation code. Please provide some feedback, and I will continue to improve it. |
Thanks for the contribution! The Spark core is a somewhat heavy lib to add to the common module. |
Okay, I'll research JSqlParser. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3426 +/- ##
============================================
+ Coverage 21.59% 27.88% +6.29%
- Complexity 2353 3660 +1307
============================================
Files 431 603 +172
Lines 40347 49328 +8981
Branches 5711 6367 +656
============================================
+ Hits 8712 13756 +5044
- Misses 30903 34607 +3704
- Partials 732 965 +233
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@zhoujinsong @czy006 I have finished the development. Please help review it. I hope it can be merged into the master before the release of 0.8.0. |
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.
@lintingbin Thanks for the work!
I left some comments, PTAL.
amoro-format-iceberg/src/main/java/org/apache/amoro/table/TableProperties.java
Outdated
Show resolved
Hide resolved
...rmat-iceberg/src/main/java/org/apache/amoro/optimizing/plan/AbstractOptimizingEvaluator.java
Outdated
Show resolved
Hide resolved
It has been resolved. Please review it again. |
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.
Thank you for your contribution. I commented on some of the doubts.
amoro-format-iceberg/src/main/java/org/apache/amoro/table/TableProperties.java
Show resolved
Hide resolved
amoro-format-iceberg/src/main/java/org/apache/amoro/utils/ExpressionUtil.java
Show resolved
Hide resolved
Thanks for the work! @lintingbin |
Why are the changes needed?
Close #3424 .
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation