Skip to content

Show Iceberg table version in SHOW CREATE TABLE#11980

Merged
findepi merged 1 commit intotrinodb:masterfrom
polaris6:support-iceberg-format-version
Apr 21, 2022
Merged

Show Iceberg table version in SHOW CREATE TABLE#11980
findepi merged 1 commit intotrinodb:masterfrom
polaris6:support-iceberg-format-version

Conversation

@polaris6
Copy link
Member

@polaris6 polaris6 commented Apr 15, 2022

When doing a show create table operation for a iceberg table, trino does not return the format-version property:
image

Therefore the content of this part of the document is incorrect:
image

I made some modifications to show this property. The first step is to create a table:
image

Before the modification, the result of show create table is like this:
image

After the modification, the result of show create table is like this:
image

@cla-bot
Copy link

cla-bot bot commented Apr 15, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@polaris6
Copy link
Member Author

@findepi Hi findepi, excuse me, can you help me to review this pr? In addition, I submitted a cla yesterday, but I haven't received a reply yet, so the current pr submission may fail the verification, which can be ignored for now. Thank you very much!
image

@findepi findepi requested a review from alexjo2144 April 15, 2022 12:07
@findepi
Copy link
Member

findepi commented Apr 15, 2022

@polaris6 thank you for your PR!

cc @martint on the CLA question

cc @alexjo2144

@cla-bot
Copy link

cla-bot bot commented Apr 15, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@github-actions github-actions bot added the docs label Apr 15, 2022
Copy link
Member

Choose a reason for hiding this comment

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

That table is based on org.apache.iceberg.Table#properties so the contents of the table aren't very strict. There could be a key/value entry for anything in this class, if the table was made or altered in Spark https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/TableProperties.java

I"m not sure how to make that clear in the docs though

Copy link
Member

Choose a reason for hiding this comment

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

The example should include properties a user is most likely to see.
format-version doesn't seem to be one of them, so it's a good change to remove it from here.

let's consider this sufficient for this PR

Copy link
Member Author

@polaris6 polaris6 Apr 15, 2022

Choose a reason for hiding this comment

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

In iceberg's metadata file, format-version is an element at the same level as properties and will not be included in
properties. A sample metadata file is as follows:

{
  "format-version" : 1,
  "location" : "hdfs://testOnline/data/iceberg/warehouse/test.db/demo",
  "partition-spec" : [ {
    "name" : "dteventtimestamp_hour",
    "transform" : "hour",
    "source-id" : 5,
    "field-id" : 1000
  } ],
  "properties" : {
    "commit.retry.max-wait-ms" : "3000",
    "max.compact.file.size" : "4194304",
    "commit.retry.total-timeout-ms" : "100000",
    "preserve.snapshot.days" : "3",
    "preserve.snapshot.nums" : "10",
    "write.format.default" : "parquet",
    "max.rewrite.files" : "3000",
    "write.metadata.delete-after-commit.enabled" : "true",
    "commit.retry.num-retries" : "2000",
    "commit.retry.min-wait-ms" : "100"
  },
  "current-snapshot-id" : 1234567890123,
  "...": "..."
}

We can refer to these, show create table returns elements such as format-version, location, partition, etc. $properties returns properties in iceberg metadata.

@findepi
Copy link
Member

findepi commented Apr 15, 2022

@polaris6 can you please squash commits and perhaps change commit message to something like the following

Show Iceberg table version in SHOW CREATE TABLE

Include `format_version` table property in `SHOW CREATE TABLE` output.

This also fixes a doc entry about `$properties` system table. The
`format-version` property isn't returned there.

@findepi findepi changed the title support show iceberg table format_version and fix incorrect doc Show Iceberg table version in SHOW CREATE TABLE Apr 15, 2022
@polaris6 polaris6 force-pushed the support-iceberg-format-version branch from 2823bc3 to 73d5f1d Compare April 15, 2022 14:33
@cla-bot
Copy link

cla-bot bot commented Apr 15, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@polaris6
Copy link
Member Author

@findepi Thank you for your suggestion, I resubmitted it, do you think this is ok?

@findepi
Copy link
Member

findepi commented Apr 15, 2022

@polaris6 thanks!
can you please make the first line of commit message shorter? it looks quite long.
You can refer to #11980 (comment) for my suggested commit message (the line breaks matter).

@polaris6 polaris6 force-pushed the support-iceberg-format-version branch from 73d5f1d to 59d629c Compare April 15, 2022 15:11
@cla-bot
Copy link

cla-bot bot commented Apr 15, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@polaris6
Copy link
Member Author

@findepi Thank you for your suggestion, I resubmitted it.

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Thanks!

@cla-bot
Copy link

cla-bot bot commented Apr 16, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@polaris6
Copy link
Member Author

@findepi Hello findepi, sorry to bother you, I fixed the failed test case and submitted a commit, could you please help me to trigger the workflow? After the workflow is executed, I will squash commits.
I'm so sorry, it's really my problem.
I have executed all the verifications of the workflow myself, and I will pay attention in the future. Before submitting the pr to this repository, I will run the workflow test first, and submit it after all the verifications are passed.
image

@cla-bot
Copy link

cla-bot bot commented Apr 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@findepi
Copy link
Member

findepi commented Apr 19, 2022

@polaris6 please squash the commits and ping here for workflow re-run.
(i wish i could approve it in advance, but i don't see an option for this, sorry)

Include `format_version` table property in `SHOW CREATE TABLE` output.

This also fixes a doc entry about `$properties` system table. The
`format-version` property isn't returned there.
@polaris6 polaris6 force-pushed the support-iceberg-format-version branch from 785b873 to efa9198 Compare April 19, 2022 12:04
@cla-bot
Copy link

cla-bot bot commented Apr 19, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@martint
Copy link
Member

martint commented Apr 20, 2022

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Apr 20, 2022

The cla-bot has been summoned, and re-checked this pull request!

@findepi findepi merged commit 4ddffd0 into trinodb:master Apr 21, 2022
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Apr 21, 2022
@github-actions github-actions bot added this to the 378 milestone Apr 21, 2022
@polaris6 polaris6 deleted the support-iceberg-format-version branch April 25, 2022 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed docs no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

4 participants