Skip to content
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

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

zratkai
Copy link
Contributor

@zratkai zratkai commented Sep 18, 2024

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.

@difin
Copy link
Contributor

difin commented Sep 18, 2024

Looks good to me, pending tests and minor ask related to the new unit test.

Copy link
Contributor Author

@zratkai zratkai left a 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.

Copy link
Contributor Author

@zratkai zratkai left a 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!

Copy link
Member

@deniskuzZ deniskuzZ left a 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.

@zratkai
Copy link
Contributor Author

zratkai commented Sep 24, 2024

@deniskuzZ I deleted the qtest.

Copy link
Member

@deniskuzZ deniskuzZ left a 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
Copy link

sonarcloud bot commented Sep 25, 2024

@deniskuzZ deniskuzZ merged commit 2185a2f into apache:master Sep 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants