Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(DAT-18966): hide delta properties for tables/views #209

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ static AbstractAlterPropertiesChangeDatabricks[] getAlterTablePropertiesChangeDa

static AbstractAlterPropertiesChangeDatabricks[] getAbstractTablePropertiesChangeDatabricks(AbstractDatabaseObject changedObject, DiffOutputControl control, Difference difference, Class<? extends AbstractAlterPropertiesChangeDatabricks> clazz) {
AbstractAlterPropertiesChangeDatabricks[] changes = new AbstractAlterPropertiesChangeDatabricks[0];
Map<String, String> referencedValuesMap = convertToMap(difference.getReferenceValue());
Map<String, String> comparedValuesMap = convertToMap(difference.getComparedValue());
Map<String, String> referencedValuesMap = convertToMapExcludingDeltaParameters(difference.getReferenceValue());
Map<String, String> comparedValuesMap = convertToMapExcludingDeltaParameters(difference.getComparedValue());

Map<String, String> addPropertiesMap = new HashMap<>();
//first we add the missing or changed properties
Expand Down Expand Up @@ -89,7 +89,10 @@ static AbstractAlterPropertiesChangeDatabricks[] getAbstractTablePropertiesChang
return changes;
}

private static Map<String, String> convertToMap(Object referenceValueObject) {
/**
* Convert the reference value to a map excluding delta parameters
*/
private static Map<String, String> convertToMapExcludingDeltaParameters(Object referenceValueObject) {
String referenceValue = referenceValueObject == null ? "" : referenceValueObject.toString();
return Arrays.stream(referenceValue.split(SPLIT_ON_COMMAS))
.map(s -> s.split(SPLIT_ON_EQUALS))
Expand All @@ -99,6 +102,14 @@ private static Map<String, String> convertToMap(Object referenceValueObject) {
.collect(Collectors.toMap(a -> a[0], a -> a[1]));
}

/**
* Get the extended properties excluding delta parameters
*/
public static String getExtendedProperties(String tblProperties) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Getting me bugged a bit, as far as we have an object ExtendedTableProperties, and this one represents filtered tblProperties. Could we rename this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SvampX, sorry, I will merge this ticket without this change to unblock testing of other tickets. Can we do this change is scope of some other PR?

Copy link
Contributor Author

@filipelautert filipelautert Nov 5, 2024

Choose a reason for hiding this comment

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

@SvampX you are right, naming it getFilteredTblProperties or removeDeltaPropertiesFromTblProperties would make more sense?

Map<String, String> properties = convertToMapExcludingDeltaParameters(tblProperties);
return properties.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(","));
}

private static AbstractAlterPropertiesChangeDatabricks getAbstractAlterPropertiesChangeDatabricks(AbstractDatabaseObject changedObject, DiffOutputControl control, Class<? extends AbstractAlterPropertiesChangeDatabricks> clazz) {
AbstractAlterPropertiesChangeDatabricks change;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import liquibase.structure.DatabaseObject;
import liquibase.structure.core.Table;

import static liquibase.ext.databricks.diff.output.changelog.ChangedTblPropertiesUtil.getExtendedProperties;

public class MissingTableChangeGeneratorDatabricks extends MissingTableChangeGenerator {

@Override
Expand All @@ -33,7 +35,7 @@ public Change[] fixMissing(DatabaseObject missingObject, DiffOutputControl contr
//so far we intentionally omit tableLocation in generated changelog
ExtendedTableProperties extendedTableProperties = new ExtendedTableProperties(
null,
missingObject.getAttribute("tblProperties", String.class));
getExtendedProperties(missingObject.getAttribute("tblProperties", String.class)));
String clusterColumns = missingObject.getAttribute("clusteringColumns", "");

changes[0] = getCreateTableChangeDatabricks(extendedTableProperties, changes, clusterColumns);
Expand Down Expand Up @@ -65,4 +67,4 @@ private CreateTableChangeDatabricks getCreateTableChangeDatabricks(ExtendedTable
protected CreateTableChange createCreateTableChange() {
return new CreateTableChangeDatabricks();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import liquibase.structure.DatabaseObject;
import liquibase.structure.core.View;

import static liquibase.ext.databricks.diff.output.changelog.ChangedTblPropertiesUtil.getExtendedProperties;

/**
* Custom implementation of {@link MissingViewChangeGenerator} for Databricks.
*/
Expand All @@ -31,7 +33,7 @@ public Change[] fixMissing(DatabaseObject missingObject, DiffOutputControl contr
if (changes == null || changes.length == 0) {
return changes;
}
changes[0] = getCreateViewChangeDatabricks(missingObject.getAttribute("tblProperties", String.class), changes);
changes[0] = getCreateViewChangeDatabricks(getExtendedProperties(missingObject.getAttribute("tblProperties", String.class)), changes);
return changes;
}

Expand Down
Loading