DAT-19308 - feat(databricks): ensure essential table properties Delta.* #233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CharlesQueiroz
requested review from
KushnirykOleh,
PavloTytarchuk and
suryaaki2
filipelautert
requested changes
Jan 6, 2025
src/main/java/liquibase/ext/databricks/sqlgenerator/CreateTableGeneratorDatabricks.java
Outdated
Show resolved
Hide resolved
filipelautert
requested changes
Jan 9, 2025
src/main/java/liquibase/ext/databricks/sqlgenerator/CreateTableGeneratorDatabricks.java
Outdated
Show resolved
Hide resolved
filipelautert
approved these changes
Jan 9, 2025
SvampX
approved these changes
Jan 9, 2025
SvampX
requested changes
Jan 9, 2025
There was a problem hiding this comment.
@CharlesQueiroz we need to update expected SQL-s, see failed harness tests.
…reserved in TBLPROPERTIES clause Added logic to maintain essential Delta properties when creating tables, ensuring critical parameters are always present with appropriate values, whether user-specified or defaults.
filipelautert
force-pushed
the
DAT-19308
branch
from
e52d3fd to
6dd2ef1
Compare
SvampX
approved these changes
Jan 10, 2025
KushnirykOleh
force-pushed
the
DAT-19308
branch
from
bfb9e90 to
92b5816
Compare
| @@ -66,7 +66,7 @@ | |||
| <liquibase-test-harness.version>1.0.10</liquibase-test-harness.version> | |||
| <junit.version>5.11.4</junit.version> | |||
| <junit-platform.version>1.11.4</junit-platform.version> | |||
| <mockito.version>5.15.2</mockito.version> | |||
| <mockito.version>4.11.0</mockito.version> | |||
There was a problem hiding this comment.
extension should work on java 8, never mockito requires java 11
|
filipelautert
approved these changes
Jan 24, 2025
|
|
||
| public class CreateTableGeneratorDatabricks extends CreateTableGenerator { | ||
|
|
||
| private static final String[] PROPERTY_ORDER = { |
There was a problem hiding this comment.
If there is still some spare time to fix we could get rid of this array and replace it by the keys from DEFAULT_VALUES .
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR addresses the handling of essential Delta table properties in the Databricks extension, specifically ensuring critical properties are always included in table creation statements regardless of user input.
Problem
Previously, when users specified custom TBLPROPERTIES during table creation, the system would not guarantee the presence of essential Delta properties.
Solution
The implementation introduces a property management system that:
Key Changes
processTablePropertiesmethodTesting
The changes are covered by unit tests that verify: