Allow updating `reader_version` and `writer_version` properties for Delta tables by krvikash · Pull Request #16165 · trinodb/trino

krvikash · 2023-02-18T00:37:16Z

Description

Release notes

(X) Release notes are required, with the following suggested text:

# Delta Lake
* Allow updating `reader_version` and `writer_version` table properties. ({issue}`16165`)

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java

homar

As @findinpath wrote, I think some tests could be splitted apart from that looks very good to me

krvikash · 2023-02-20T17:44:12Z

Thanks @findinpath | @homar for the review. Addressed review comments.

ebyhr · 2023-02-21T04:55:10Z

@krvikash Could you rebase to resolve conflicts?

krvikash · 2023-02-21T05:43:34Z

Rebased with master and resolve conflicts.

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableProperties.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java

krvikash · 2023-02-21T15:09:03Z

Thanks, @ebyhr for the review. I have addressed the comments.

alexjo2144

Overall, looks pretty good. Couple small things and behavior questions

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java

alexjo2144 · 2023-02-21T15:40:35Z

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

I don't think we should do this. If the user sets the writer version to 2, when they do a show create table it should say 2.

If the user sets the writer version to less than 4 and cdf is enabled then it will throw an exception.

Got it, I see what you're doing now 👍

Though, if some has set writer version to 5 explicitly, this will set it to 4. I think you only want to do this if writerVersion hasn't been set explicitly

I know you can't set it to 5 yet, but you will be able to soon.

I think you only want to do this if writerVersion hasn't been set explicitly

Addressed.

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

alexjo2144 · 2023-02-21T15:45:55Z

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

This could be follow-up in a separate PR, but I think we want to remove these constants and add default reader/writer versions as config properties in DeltaLakeConfig.

If you're going to enable CDF on every table you make, you'd want to have v4 be the default.

Follow up PR: #16208

krvikash · 2023-02-21T18:06:21Z

Thanks, @alexjo2144 for the review. Addressed comments.

alexjo2144

Code looks good. Last nitpick is that your commit messages are too long

https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#commits-and-pull-requests

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

krvikash · 2023-02-22T06:27:08Z

Addressed comments.

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java

ebyhr · 2023-02-22T08:16:12Z

Could you fix error-prone failure?

Error:  /home/runner/work/trino/trino/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java:[1228,20] [UnusedVariable] The parameter 'nodeVersion' is never read.
Error:      (see https://errorprone.info/bugpattern/UnusedVariable)
Error:    Did you mean 'session,'?
Error:  /home/runner/work/trino/trino/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java:[1229,20] [UnusedVariable] The parameter 'nodeId' is never read.
Error:      (see https://errorprone.info/bugpattern/UnusedVariable)
Error:    Did you mean 'nodeVersion,'?

krvikash · 2023-02-22T09:14:01Z

Fixed CI failure.

ebyhr · 2023-02-22T09:15:37Z

/test-with-secrets sha=2630c3b47f47bf4b5193e3cb72e8f1d51de1445d

github-actions · 2023-02-22T19:20:48Z

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4242355164

krvikash · 2023-02-22T20:45:36Z

Fixed product test:

io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCheckpointsCompatibility.testTrinoUsesCheckpointInterval (Groups: profile_specific_tests, delta-lake-databricks) took 4.6 seconds

ebyhr · 2023-02-22T22:49:48Z

/test-with-secrets sha=082a0c2caba8e5ef24eda0a3615b591a07b8f0cc

github-actions · 2023-02-23T09:28:56Z

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4249084669

cla-bot bot added the cla-signed label Feb 18, 2023

krvikash changed the title ~~Support read version and write version in delta~~ Allow setting reader_version and writer_version properties with CREATE TABLE and ALTER TABLE in Delta table Feb 18, 2023

krvikash changed the title ~~Allow setting reader_version and writer_version properties with CREATE TABLE and ALTER TABLE in Delta table~~ Allow setting reader_version and writer_version properties in Delta table Feb 18, 2023

github-actions bot added docs tests:hive labels Feb 18, 2023

krvikash added the delta-lake Delta Lake connector label Feb 18, 2023

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from d128e82 to 8f7c145 Compare February 18, 2023 09:21

krvikash marked this pull request as ready for review February 18, 2023 09:22

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 8f7c145 to 1a0ad16 Compare February 18, 2023 12:47

krvikash requested review from alexjo2144, ebyhr, findinpath and homar February 20, 2023 10:18

findinpath reviewed Feb 20, 2023

View reviewed changes

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java Outdated Show resolved Hide resolved

findinpath reviewed Feb 20, 2023

View reviewed changes

krvikash changed the title ~~Allow setting reader_version and writer_version properties in Delta table~~ Allow updating reader_version and writer_version properties for Delta tables Feb 20, 2023

homar approved these changes Feb 20, 2023

View reviewed changes

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 1a0ad16 to 6206c66 Compare February 20, 2023 17:41

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 6206c66 to 69f1bc2 Compare February 21, 2023 05:41

ebyhr reviewed Feb 21, 2023

View reviewed changes

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch 3 times, most recently from 529f058 to 18570c8 Compare February 21, 2023 15:07

alexjo2144 reviewed Feb 21, 2023

View reviewed changes

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 18570c8 to e210fa3 Compare February 21, 2023 17:57

alexjo2144 approved these changes Feb 21, 2023

View reviewed changes

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch 2 times, most recently from 3622e94 to ce40c8a Compare February 21, 2023 20:57

ebyhr reviewed Feb 21, 2023

View reviewed changes

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch 2 times, most recently from 2a9161f to be93e8c Compare February 22, 2023 06:26

ebyhr approved these changes Feb 22, 2023

View reviewed changes

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java Outdated Show resolved Hide resolved

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from be93e8c to 853b8e6 Compare February 22, 2023 06:47

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 853b8e6 to 2630c3b Compare February 22, 2023 09:13

trinodb deleted a comment from github-actions bot Feb 22, 2023

krvikash added 4 commits February 23, 2023 02:10

Support reader_version and writer_version properties using CREATE TABLE

7d7a940

Set writer_version to v4 when change_data_feed_enabled is set to true

1974db2

Create DeltaLakeMetadata#getCommitInfoEntry to reduce code duplication

79b50b9

Support reader_version and writer_version properties using ALTER TABLE

082a0c2

krvikash force-pushed the support-readVersion-and-writeVersion-in-delta branch from 2630c3b to 082a0c2 Compare February 22, 2023 20:45

homar mentioned this pull request Feb 22, 2023

Allow enabling CDF using ALTER TABLE SET PROPERTIES #16221

Closed

ebyhr merged commit 6496778 into trinodb:master Feb 23, 2023

ebyhr mentioned this pull request Feb 23, 2023

Release notes for 408 #16147

Closed

krvikash deleted the support-readVersion-and-writeVersion-in-delta branch February 23, 2023 11:49

github-actions bot added this to the 408 milestone Feb 23, 2023

colebow mentioned this pull request Feb 23, 2023

Add Trino 408 release notes #16209

Merged

findepi mentioned this pull request Mar 1, 2023

Configure Delta reader and writer version automatically #16332

Closed

Conversation

krvikash commented Feb 18, 2023 • edited by ebyhr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

homar left a comment

Choose a reason for hiding this comment

Uh oh!

krvikash commented Feb 20, 2023

Uh oh!

ebyhr commented Feb 21, 2023

Uh oh!

krvikash commented Feb 21, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krvikash commented Feb 21, 2023

Uh oh!

alexjo2144 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexjo2144 Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

krvikash Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

alexjo2144 Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

alexjo2144 Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krvikash Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexjo2144 Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

krvikash Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

krvikash commented Feb 21, 2023

Uh oh!

alexjo2144 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krvikash commented Feb 22, 2023

Uh oh!

Uh oh!

ebyhr commented Feb 22, 2023

Uh oh!

krvikash commented Feb 22, 2023

Uh oh!

ebyhr commented Feb 22, 2023

Uh oh!

github-actions bot commented Feb 22, 2023

krvikash commented Feb 18, 2023 •

edited by ebyhr

Loading

alexjo2144 Feb 21, 2023 •

edited

Loading

alexjo2144 left a comment •

edited

Loading