-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
HIVE-28343 Iceberg: Major QB Compaction support filter in compaction … #5455
Conversation
1cd83f8
to
3332d30
Compare
parser/src/test/org/apache/hadoop/hive/ql/parse/TestParseOptimizeTable.java
Outdated
Show resolved
Hide resolved
Looks good to me, pending tests and minor ask related to the new unit test. |
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 the review! Please check my reply.
3332d30
to
2be2b9c
Compare
2be2b9c
to
d6dd6e8
Compare
...r/src/test/queries/positive/iceberg_optimize_table_partition_evolution_w_dyn_spec_w_filter.q
Outdated
Show resolved
Hide resolved
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.
Please check my answer!
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.
You haven't implemented any new compaction functionality, but added just a parser sugar. There is no need to test the whole Hive for such a minor change.
The new test is a complete copy-paste of the existing test without sugar, so it's just a waste of Jenkins resources. qtests are expensive and are overkill for such minor changes.
You need to test that optimize command is being correctly rewritten to alter and that is it.
d6dd6e8
to
c20a141
Compare
@deniskuzZ I deleted the qtest. |
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.
LGTM +1
…request in OPTIMIZE TABLE command Change-Id: I6e3c09f67423cf559f315d6f448877c967da7479
c20a141
to
f05b2ce
Compare
Quality Gate passedIssues Measures |
HIVE-28343 Iceberg: Major QB Compaction support filter in compaction request in OPTIMIZE TABLE command
Change-Id: I6e3c09f67423cf559f315d6f448877c967da7479
What changes were proposed in this pull request?
With this PR user can execute optimize table command with condition.
Why are the changes needed?
To execute optimize table with condition.
Does this PR introduce any user-facing change?
Yes, user. can use optimize table with condition.
Is the change a dependency upgrade?
No.
How was this patch tested?
Unit test, qtest.