Skip to content

Infer required Delta Lake version from table features#16310

Merged
findepi merged 3 commits intotrinodb:masterfrom
alexjo2144:delta/implicit-versioning
Mar 2, 2023
Merged

Infer required Delta Lake version from table features#16310
findepi merged 3 commits intotrinodb:masterfrom
alexjo2144:delta/implicit-versioning

Conversation

@alexjo2144
Copy link
Copy Markdown
Member

Description

Rather than require users upgrade the protocol version on their tables before using features which require newer versions of the spec, automatically determine which protocol version is required and upgrade the table to it.

Additional context and related issues

Includes one commit from @homar's #16221

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Delta Lake
* Infer required Delta Lake version when creating new tables or enabling new table features.

@cla-bot cla-bot bot added the cla-signed label Feb 28, 2023
@homar
Copy link
Copy Markdown
Member

homar commented Feb 28, 2023

Why didn't you add testTurningOnAndOffCdfFromTrino that I added to testing/trino-product-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksChangeDataFeedCompatibility.java ?

@alexjo2144
Copy link
Copy Markdown
Member Author

Because the commit message made it sound like I could skip it "Add flaky to all tests...", maybe it should be in the second commit, or a separate one all together?

@alexjo2144 alexjo2144 self-assigned this Feb 28, 2023
@alexjo2144 alexjo2144 added the delta-lake Delta Lake connector label Feb 28, 2023
@homar
Copy link
Copy Markdown
Member

homar commented Feb 28, 2023

Because the commit message made it sound like I could skip it "Add flaky to all tests...", maybe it should be in the second commit, or a separate one all together?

I added Flaky annotations to all methods in that class in a first commit and it made sense to skip that commit, but in the second commit that you actually used I added a test case there, and @ebyhr thought it is important because he wanted me to test some particular cases there so I think that one test should be here.

I will add @flaky to all tests in that class in a separate pr

@alexjo2144 alexjo2144 force-pushed the delta/implicit-versioning branch from 2e9b822 to fd2e2bc Compare February 28, 2023 18:34
@alexjo2144 alexjo2144 force-pushed the delta/implicit-versioning branch from fd2e2bc to db8783a Compare March 1, 2023 17:18
Add missing `@Flaky` annotations to the tests in
TestDeltaLakeDatabricksChangeDataFeedCompatibility
@alexjo2144 alexjo2144 force-pushed the delta/implicit-versioning branch from db8783a to e8c39a7 Compare March 1, 2023 17:20
@alexjo2144
Copy link
Copy Markdown
Member Author

Thanks for the reviews, all addressed

Copy link
Copy Markdown
Contributor

@krvikash krvikash left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

double space

homar and others added 2 commits March 1, 2023 14:45
Rather than require users upgrade the protocol version on their tables
before using features which require newer versions of the spec,
automatically determine which protocol version is required and upgrade
the table to it.
@alexjo2144 alexjo2144 force-pushed the delta/implicit-versioning branch from e8c39a7 to af89f83 Compare March 1, 2023 19:50
@findepi
Copy link
Copy Markdown
Member

findepi commented Mar 1, 2023

cc @trinodb/maintainers @colebow i marked this one as a release-blocker, because it's important to get this into the release.
This is not a typical release blocker. It removes some recently introduced properties (#16332) and the goal here is to minimize the time window during which these are available to effectively minimize any negative consequences.

@findepi findepi merged commit af4d430 into trinodb:master Mar 2, 2023
@github-actions github-actions bot added this to the 409 milestone Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants