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

Support indicating partition transform when add column to Iceberg table #21206

Merged

Conversation

hantangwangd
Copy link
Member

@hantangwangd hantangwangd commented Oct 22, 2023

Description

Through this PR, when we alter an Iceberg table to add a new column, we could also indicate it as a partition column. Moreover, we could use the transform functions and partition the table by the transformed value of the column, for example:

  • ALTER TABLE iceberg.web.page_views ADD COLUMN zipcode VARCHAR WITH (partitioning = 'identity');

  • ALTER TABLE iceberg.web.page_views ADD COLUMN location VARCHAR WITH (partitioning = 'truncate(2)');

  • ALTER TABLE iceberg.web.page_views ADD COLUMN location VARCHAR WITH (partitioning = 'bucket(8)');

Note: Day, Month, Year, Hour partition column transform functions are not supported in Presto Iceberg connector yet.

Test Plan

  • Newly added IcebergDistributedTestBase.testAddPartitionColumn()

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== RELEASE NOTES ==

General Changes
* The Iceberg connector now supports `ALTER TABLE <table> ADD COLUMN <column> [WITH (partitioning = '<transform_func>')]` statements to indicate partition transform when add column to table

@hantangwangd hantangwangd requested a review from a team as a code owner October 22, 2023 05:47
@github-actions
Copy link

github-actions bot commented Oct 22, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff 82a3bc8...3db9205.

Notify File(s)
@steveburnett presto-docs/src/main/sphinx/connector/iceberg.rst

Copy link
Contributor

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working well for me! Nice work. I have two small comments about the docs below

presto-docs/src/main/sphinx/connector/iceberg.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/iceberg.rst Outdated Show resolved Hide resolved
@hantangwangd hantangwangd force-pushed the iceberg_partition_evolution branch from b6c19be to 523bcc2 Compare October 25, 2023 00:07
@tdcmeehan tdcmeehan self-assigned this Oct 25, 2023
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! I appreciate your attention to accurate formatting of the code.

I made a few style and revision suggestions, and I explained my thoughts in the comments. If my suggested edits change the meaning of your text in a way you disagree with, let me know if I overlooked your intended meaning.

presto-docs/src/main/sphinx/connector/iceberg.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/iceberg.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/iceberg.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@ZacBlanco ZacBlanco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two questions, otherwise PR looks good!

@hantangwangd hantangwangd force-pushed the iceberg_partition_evolution branch from 9e5a64e to 79ab64a Compare October 26, 2023 01:38
@hantangwangd hantangwangd force-pushed the iceberg_partition_evolution branch from 0aa06b2 to 3db9205 Compare October 27, 2023 00:02
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (docs)

Local build of docs, everything looks good.

@hantangwangd hantangwangd merged commit b67703a into prestodb:master Oct 27, 2023
@hantangwangd hantangwangd deleted the iceberg_partition_evolution branch October 27, 2023 14:32
@wanglinsong wanglinsong mentioned this pull request Dec 8, 2023
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants