Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Spark] Only enable a single legacy feature with legacy metadata properties #3657

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

bart-samwel
Copy link
Collaborator

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

Currently enabling legacy features on legacy protocols with metadata properties results to enabling all preceding legacy features. For example, enabling enableChangeDataFeed results to protocol (1, 4). This is inconsistent with the rest protocol operations. In this PR, we fix this inconsistency by always enabling only the requested feature. This is a behavioral change.

How was this patch tested?

Existing and new unit tests.

Does this PR introduce any user-facing changes?

Yes. When enabling a feature using a table property, e.g. by setting delta.enableChangeDataFeed to true, then in the previous situation you would typically get protocol (1, 4). Now you would get (1, 7, changeDataFeed). The user can get (1, 4) by also asking for delta.minWriterVersion = 4. This change is OK now because (a) enabling fewer features is safer than enabling more features, and (b) Deletion Vectors requires table features support, and it is very popular to implement, so many clients have added support table features, (c) users can easily get back to the legacy protocol by ALTERing the protocol and asking for delta.minWriterVersion = 4.

Copy link
Contributor

@larsk-db larsk-db left a comment

Choose a reason for hiding this comment

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

LGTM

@bart-samwel
Copy link
Collaborator Author

Spark master test failures are unrelated.

@bart-samwel bart-samwel merged commit 15da4aa into master Sep 9, 2024
28 of 34 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants