Skip to content

Conversation

@zratkai
Copy link
Contributor

@zratkai zratkai commented Oct 3, 2024

Change-Id: Ic85efd70599413cdb96073c6cb50690fbc1c11b0

What changes were proposed in this pull request?

Limiting the columns in where clause in compaction to partition columns.

Why are the changes needed?

Currently Hive Iceberg compaction supports any columns in the WHERE clause predicate. However, compaction happens only on partition level. Performance can be improved if the columns in WHERE clause predicate will be limited to partition columns.

Does this PR introduce any user-facing change?

Yes, user will get an exception if uses non partition columns in alter table compaction query.

Is the change a dependency upgrade?

No.

How was this patch tested?

With qtest.

@difin
Copy link
Contributor

difin commented Oct 3, 2024

+1, Looks good to me.

Added a few minor formatting comments.
There is also a q-test failure iceberg_major_compaction_partition_evolution_w_id_spec_w_filter
The test needs to be adjusted for these code changes.

@zratkai zratkai force-pushed the HIVE-28537-only-partcol-compaction branch from ad45a83 to 51d4295 Compare October 7, 2024 08:58
@zratkai zratkai force-pushed the HIVE-28537-only-partcol-compaction branch 2 times, most recently from 63ad837 to ed57046 Compare October 7, 2024 11:33
Change-Id: Ic85efd70599413cdb96073c6cb50690fbc1c11b0
@zratkai zratkai force-pushed the HIVE-28537-only-partcol-compaction branch from a92fb56 to 0d33cda Compare October 22, 2024 08:42
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, pending tests

@sonarqubecloud
Copy link

@deniskuzZ deniskuzZ merged commit ddcba0b into apache:master Oct 25, 2024
@deniskuzZ deniskuzZ changed the title HIVE-28537: Iceberg: allow only partition columns in the WHERE clause HIVE-28537: Iceberg: Compaction: Allow only partition columns in the WHERE clause Oct 25, 2024
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.

4 participants