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.

Sign up for GitHub

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

Jump to bottom

DAT-19050: adding missing column is splitted on addColumn and add{Constraint} changetypes #216

Merged
merged 4 commits into from
Nov 21, 2024
Merged

DAT-19050: adding missing column is splitted on addColumn and add{Constraint} changetypes #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95d6fbe
DAT-19050: adding missing column with default value is split on 2 cha…
Nov 20, 2024
74d6fac
DAT-19050: adding fix for miscreating/misdropping default value colum…
Nov 20, 2024
2d094f9
DAT-19050: refactored, added not null constraint handling as it was f…
Nov 20, 2024
557045e
DAT-19050: several more fixes, slightlu changed regex to not include …
Nov 21, 2024
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
157 changes: 157 additions & 0 deletions ...iquibase/ext/databricks/diff/output/changelog/MissingColumnChangeGeneratorDatabricks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package liquibase.ext.databricks.diff.output.changelog;

import liquibase.change.AddColumnConfig;
import liquibase.change.Change;
import liquibase.change.ConstraintsConfig;
import liquibase.change.core.AddColumnChange;
import liquibase.change.core.AddDefaultValueChange;
import liquibase.change.core.AddNotNullConstraintChange;
import liquibase.database.Database;
import liquibase.diff.output.DiffOutputControl;
import liquibase.diff.output.changelog.ChangeGeneratorChain;
import liquibase.diff.output.changelog.core.MissingColumnChangeGenerator;
import liquibase.ext.databricks.database.DatabricksDatabase;
import liquibase.statement.DatabaseFunction;
import liquibase.structure.DatabaseObject;
import liquibase.structure.core.Column;
import org.apache.commons.lang3.ObjectUtils;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

/**
* Custom diff change generator for Databricks
*/
public class MissingColumnChangeGeneratorDatabricks extends MissingColumnChangeGenerator {

@Override
public int getPriority(Class<? extends DatabaseObject> objectType, Database database) {
if (database instanceof DatabricksDatabase && super.getPriority(objectType, database) > PRIORITY_NONE) {
return PRIORITY_DATABASE;
}
return PRIORITY_NONE;
}

@Override
public Change[] fixMissing(DatabaseObject missingObject, DiffOutputControl control, Database referenceDatabase, Database comparisonDatabase, ChangeGeneratorChain chain) {
Change[] changes = super.fixMissing(missingObject, control, referenceDatabase, comparisonDatabase, chain);
changes = handleMissingColumnConstraints((Column) missingObject, control, changes);
return changes;
}

private Change[] handleMissingColumnConstraints(Column column, DiffOutputControl control, Change[] changes) {
Optional<AddColumnChange> addColumnOptional = Arrays.stream(changes)
.filter(change -> isCurrentColumnChange(change, column, control))
.map(AddColumnChange.class::cast).findFirst();
if(addColumnOptional.isPresent()) {
AddColumnChange addColumnChange = addColumnOptional.get();
changes = splitAddColumnChange(column, control, changes, addColumnChange);
}
return changes;
}

private Change[] splitAddColumnChange(Column column, DiffOutputControl control, Change[] changes, AddColumnChange addColumnChange) {
List<Change> changeList = new ArrayList<>(Arrays.asList(changes));
AddColumnConfig addColumnConfig = addColumnChange.getColumns().get(0);
if(addColumnConfig.getDefaultValue() != null || addColumnConfig.getDefaultValueComputed() != null) {
AddDefaultValueChange addDefaultValueChange = handleDefaultValue(column, control, addColumnChange);
changeList.add(addDefaultValueChange);
}
if(addColumnConfig.getConstraints() != null && Objects.equals(addColumnConfig.getConstraints().isNullable(), Boolean.FALSE)) {
AddNotNullConstraintChange addNotNullConstraintChange = handleNotNull(column, control, addColumnChange);
changeList.add(addNotNullConstraintChange);
}
if(constraintsAreEmpty(addColumnConfig, addColumnConfig.getConstraints())) {
addColumnConfig.setConstraints(null);
}
changes = changeList.toArray(new Change[0]);
return changes;
}

private AddDefaultValueChange handleDefaultValue(Column column, DiffOutputControl control, AddColumnChange addColumnChange) {
AddColumnConfig addColumnConfig = addColumnChange.getColumns().get(0);
String defaultValue = addColumnConfig.getDefaultValue();
DatabaseFunction defaultValueComputed = addColumnConfig.getDefaultValueComputed();
String columnDataType = addColumnConfig.getType();
addColumnConfig.setDefaultValue(null);
addColumnConfig.setDefaultValueComputed(null);
addColumnConfig.setComputed(null);
AddDefaultValueChange addDefaultValueChange = new AddDefaultValueChange();
if (control.getIncludeCatalog()) {
addDefaultValueChange.setCatalogName(column.getRelation().getSchema().getCatalog().getName());
}
if (control.getIncludeSchema()) {
addDefaultValueChange.setSchemaName(column.getRelation().getSchema().getName());
}
addDefaultValueChange.setTableName(column.getRelation().getName());
addDefaultValueChange.setColumnName(column.getName());
addDefaultValueChange.setColumnDataType(columnDataType);

if (defaultValueComputed != null) {
addDefaultValueChange.setDefaultValueComputed(defaultValueComputed);
} else {
addDefaultValueChange.setDefaultValue(defaultValue);
}
addDefaultValueChange.setDefaultValueConstraintName(column.getDefaultValueConstraintName());
return addDefaultValueChange;
}

private AddNotNullConstraintChange handleNotNull(Column column, DiffOutputControl control, AddColumnChange addColumnChange) {
AddColumnConfig addColumnConfig = addColumnChange.getColumns().get(0);
ConstraintsConfig constraints = addColumnConfig.getConstraints();
constraints.setNullable((Boolean) null);
constraints.setNullable((String) null);
constraints.setNotNullConstraintName(null);
AddNotNullConstraintChange addNotNullConstraintChange = createAddNotNullConstraintChange(addColumnConfig, constraints);
if (control.getIncludeCatalog()) {
addNotNullConstraintChange.setCatalogName(column.getRelation().getSchema().getCatalog().getName());
}
if (control.getIncludeSchema()) {
addNotNullConstraintChange.setSchemaName(column.getRelation().getSchema().getName());
}
addNotNullConstraintChange.setTableName(column.getRelation().getName());
return addNotNullConstraintChange;
}

private AddNotNullConstraintChange createAddNotNullConstraintChange(AddColumnConfig column, ConstraintsConfig constraints) {
AddNotNullConstraintChange addNotNullConstraintChange = new AddNotNullConstraintChange();
addNotNullConstraintChange.setColumnName(column.getName());
addNotNullConstraintChange.setColumnDataType(column.getType());
addNotNullConstraintChange.setValidate(constraints.getValidateNullable());
addNotNullConstraintChange.setConstraintName(constraints.getNotNullConstraintName());
return addNotNullConstraintChange;
}

/**
* We perform reversed checks that were used in the
* {@link liquibase.change.core.AddColumnChange#generateStatements(Database)}
* to make sure there won't be empty constraints generated in generated changelog files.
* */
boolean constraintsAreEmpty(AddColumnConfig column, ConstraintsConfig constraints) {
if(constraints != null) {
return ObjectUtils.allNull(constraints.isNullable(), constraints.isUnique(), constraints.isPrimaryKey(),
column.isAutoIncrement(), constraints.getReferences(), constraints.getReferencedColumnNames(),
constraints.getReferencedTableName());
}
return column.isAutoIncrement() != null && !column.isAutoIncrement();
}

private boolean isCurrentColumnChange(Change change, Column currentColumn, DiffOutputControl control) {
if(change instanceof AddColumnChange) {
AddColumnChange addColumnChange = ((AddColumnChange) change);
AddColumnConfig addColumnConfig = addColumnChange.getColumns().get(0);
boolean columnNameEqual = addColumnConfig.getName().equals(currentColumn.getName());
boolean tableNameEqual = addColumnChange.getTableName().equals(currentColumn.getRelation().getName());
boolean schemaNameEqual = !control.getIncludeSchema() ||
Objects.equals(addColumnChange.getSchemaName(), currentColumn.getRelation().getSchema().getName());
boolean catalogNameEqual = !control.getIncludeCatalog() ||
Objects.equals(addColumnChange.getCatalogName(), currentColumn.getRelation().getSchema().getCatalogName());
return columnNameEqual && tableNameEqual && schemaNameEqual && catalogNameEqual;
}
return false;
}

}
10 changes: 5 additions & 5 deletions src/main/java/liquibase/ext/databricks/snapshot/jvm/ColumnSnapshotGeneratorDatabricks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
public class ColumnSnapshotGeneratorDatabricks extends ColumnSnapshotGenerator {

private static final String ALL_DATA_TYPES = " BIGINT | BINARY | BOOLEAN | DATE | DECIMAL| DECIMAL\\(| DOUBLE | FLOAT | INT | INTERVAL | VOID | SMALLINT | STRING | VARCHAR\\(\\d+\\) | TIMESTAMP | TIMESTAMP_NTZ | TINYINT | ARRAY<| MAP<| STRUCT<| VARIANT| OBJECT<";
private static final String DEFAULT_CLAUSE_TERMINATORS = "(?i)(\\s+COMMENT\\s+'| PRIMARY\\s+KEY | FOREIGN\\s+KEY | MASK\\s+\\w+|$|,(\\s+\\w+\\s+" + ALL_DATA_TYPES + "|\\)$)";
private static final String DEFAULT_CLAUSE_TERMINATORS = "(?i)(\\s+COMMENT\\s+'| PRIMARY\\s+KEY | FOREIGN\\s+KEY | MASK\\s+\\w+|$|,(\\s+\\w+\\s+" + ALL_DATA_TYPES + ")?|\\)$";
private static final String GENERATED_BY_DEFAULT_REGEX = "(?i)\\s+GENERATED\\s+(BY\\s+DEFAULT|ALWAYS)\\s+AS\\s+IDENTITY";
private static final String GENERIC_DEFAULT_VALUE_REGEX = "DEFAULT\\s+(.*?)(" + DEFAULT_CLAUSE_TERMINATORS + "?))";
private static final String GENERIC_DEFAULT_VALUE_REGEX = "DEFAULT\\s+(.*?)(" + DEFAULT_CLAUSE_TERMINATORS + "))";
private static final String SANITIZE_TABLE_SPECIFICATION_REGEX = "(\\(.*?\\))\\s*(?i)(USING|OPTIONS|PARTITIONED BY|CLUSTER BY|LOCATION|TBLPROPERTIES|WITH|$|;$)";
private static final Pattern DEFAULT_VALUE_PATTERN = Pattern.compile(GENERIC_DEFAULT_VALUE_REGEX);
private static final Pattern SANITIZE_TABLE_SPECIFICATION_PATTERN = Pattern.compile(SANITIZE_TABLE_SPECIFICATION_REGEX);
Expand Down Expand Up @@ -71,12 +71,11 @@ protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot
String showCreateTableStatement = (String) snapshot.getScratchData(showCreateRelatedTableQuery);
String defaultValue = extractDefaultValue(showCreateTableStatement, column.getName());
column.setAutoIncrementInformation(parseAutoIncrementInfo(showCreateTableStatement, column.getName()));
if (defaultValue != null) {
if (defaultValue != null && !defaultValue.equalsIgnoreCase("null")) {
Matcher functionMatcher = FUNCTION_PATTERN.matcher(defaultValue);
if (functionMatcher.find()) {
DatabaseFunction function = new DatabaseFunction(defaultValue);
column.setDefaultValue(function);
column.setComputed(true);
} else {
column.setDefaultValue(defaultValue);
}
Expand Down Expand Up @@ -104,7 +103,8 @@ private String extractDefaultValue(String createTableStatement, String columnNam
Matcher defaultValueMatcher = DEFAULT_VALUE_PATTERN.matcher(columnWithPotentialDefault);
if (defaultValueMatcher.find()) {
defaultValue = defaultValueMatcher.group(1);
if (stringColumnTypeMatcher.find() && defaultStringValueMatcher.find()) {
if (stringColumnTypeMatcher.find() && defaultStringValueMatcher.find()
&& (defaultValue.startsWith("'") || defaultValue.startsWith("\""))) {
defaultValue = defaultStringValueMatcher.group(2);
}
}
Expand Down
16 changes: 11 additions & 5 deletions src/main/java/liquibase/ext/databricks/sqlgenerator/AddDefaultValueGeneratorDatabricks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@
import liquibase.statement.core.AddDefaultValueStatement;
import liquibase.sqlgenerator.core.AddDefaultValueGenerator;

import java.util.Arrays;
import java.util.List;

public class AddDefaultValueGeneratorDatabricks extends AddDefaultValueGenerator {
private static final List<String> NUMERIC_TYPES = Arrays.asList("TINYINT", "SMALLINT", "INT", "BIGINT", "FLOAT", "DOUBLE", "DECIMAL");
@Override
public int getPriority() {
return PRIORITY_DATABASE;
Expand All @@ -40,14 +44,16 @@ public ValidationErrors validate(AddDefaultValueStatement addDefaultValueStateme
@Override
public Sql[] generateSql(AddDefaultValueStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) {
Object defaultValue = statement.getDefaultValue();
String columnDataType = statement.getColumnDataType();
String finalDefaultValue;
if (defaultValue instanceof DatabaseFunction) {
finalDefaultValue = "("+defaultValue+")";
if (finalDefaultValue.startsWith("((")) {
finalDefaultValue = defaultValue.toString();
}
finalDefaultValue = defaultValue.toString();
} else {
finalDefaultValue = DataTypeFactory.getInstance().fromObject(defaultValue, database).objectToSql(defaultValue, database);
if(NUMERIC_TYPES.contains(columnDataType) && defaultValue instanceof String) {
finalDefaultValue = defaultValue.toString().replace("'", "").trim();
} else {
finalDefaultValue = DataTypeFactory.getInstance().fromObject(defaultValue, database).objectToSql(defaultValue, database);
}
}
return new Sql[]{
new UnparsedSql("ALTER TABLE " + database.escapeTableName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName()) + " ALTER COLUMN " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getColumnName()) + " SET DEFAULT " + finalDefaultValue,
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/META-INF/services/liquibase.diff.output.changelog.ChangeGenerator
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
liquibase.ext.databricks.diff.output.changelog.MissingTableChangeGeneratorDatabricks
liquibase.ext.databricks.diff.output.changelog.MissingViewChangeGeneratorDatabricks
liquibase.ext.databricks.diff.output.changelog.MissingColumnChangeGeneratorDatabricks
liquibase.ext.databricks.diff.output.changelog.ChangedTableChangeGeneratorDatabricks
liquibase.ext.databricks.diff.output.changelog.ChangedViewChangeGeneratorDatabricks
7 changes: 1 addition & 6 deletions ...est/java/liquibase/ext/databricks/snapshot/jvm/ColumnSnapshotGeneratorDatabricksTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package liquibase.ext.databricks.snapshot.jvm;

import liquibase.database.jvm.JdbcConnection;
import liquibase.exception.DatabaseException;
import liquibase.ext.databricks.database.DatabricksDatabase;
import liquibase.snapshot.JdbcDatabaseSnapshot;
import liquibase.statement.DatabaseFunction;
import liquibase.structure.DatabaseObject;
Expand All @@ -17,9 +15,6 @@
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -90,7 +85,7 @@ void snapshotObjectTest() throws DatabaseException, SQLException {
testedColumn.setAttribute("liquibase-complete", true);
DatabaseObject databaseObject = snapshotGenerator.snapshotObject(testedColumn, snapshot);
assertTrue(databaseObject instanceof Column);
assertTrue(((Column) databaseObject).getComputed());
assertNull(((Column) databaseObject).getComputed());
assertNotNull(((Column) databaseObject).getDefaultValue());
assertEquals(columnWithDefaultComputed.getValue(), ((Column) databaseObject).getDefaultValue());
}
Expand Down
Loading