From 300e6f490a0f482f06dea1447735f6fc9981a4d2 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Thu, 6 Jan 2022 16:50:15 -0600 Subject: [PATCH 1/3] Support adding computed=true columns with no type set --- .../change/core/AddColumnChange.java | 1 + .../sqlgenerator/core/AddColumnGenerator.java | 20 ++++++++++++++----- .../statement/core/AddColumnStatement.java | 9 +++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java b/liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java index b702b7fe26e..3938d59c965 100644 --- a/liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java +++ b/liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java @@ -155,6 +155,7 @@ public SqlStatement[] generateStatements(Database database) { column.getRemarks(), constraints.toArray(new ColumnConstraint[constraints.size()])); addColumnStatement.setDefaultValueConstraintName(column.getDefaultValueConstraintName()); + addColumnStatement.setComputed(column.getComputed()); if ((database instanceof MySQLDatabase) && (column.getAfterColumn() != null)) { addColumnStatement.setAddAfterColumn(column.getAfterColumn()); diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java index 3ac49642855..06c2ad4ee21 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java @@ -25,6 +25,7 @@ import liquibase.structure.core.Column; import liquibase.structure.core.Schema; import liquibase.structure.core.Table; +import liquibase.util.ObjectUtil; import liquibase.util.StringUtil; import java.util.ArrayList; @@ -60,7 +61,9 @@ private ValidationErrors validateSingleColumn(AddColumnStatement statement, Data ValidationErrors validationErrors = new ValidationErrors(); validationErrors.checkRequiredField("columnName", statement.getColumnName()); - validationErrors.checkRequiredField("columnType", statement.getColumnType()); + if (!ObjectUtil.defaultIfNull(statement.getComputed(), false)) { + validationErrors.checkRequiredField("columnType", statement.getColumnType()); + } validationErrors.checkRequiredField("tableName", statement.getTableName()); if (statement.isPrimaryKey() && ((database instanceof H2Database) || (database instanceof AbstractDb2Database) || @@ -139,9 +142,17 @@ protected String generateSingleColumBaseSQL(AddColumnStatement statement, Databa } protected String generateSingleColumnSQL(AddColumnStatement statement, Database database) { - DatabaseDataType columnType = DataTypeFactory.getInstance().fromDescription(statement.getColumnType() + (statement.isAutoIncrement() ? "{autoIncrement:true}" : ""), database).toDatabaseDataType(database); + DatabaseDataType columnType = null; + + if (statement.getColumnType() != null) { + columnType = DataTypeFactory.getInstance().fromDescription(statement.getColumnType() + (statement.isAutoIncrement() ? "{autoIncrement:true}" : ""), database).toDatabaseDataType(database); + } - String alterTable = " ADD " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getColumnName()) + " " + columnType; + String alterTable = " ADD " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getColumnName()); + + if (columnType != null) { + alterTable += " " + columnType; + } if (statement.isAutoIncrement() && database.supportsAutoIncrement()) { AutoIncrementConstraint autoIncrementConstraint = statement.getAutoIncrementConstraint(); @@ -166,8 +177,7 @@ protected String generateSingleColumnSQL(AddColumnStatement statement, Database } } else { if ((database instanceof SybaseDatabase) || (database instanceof SybaseASADatabase) || (database - instanceof MySQLDatabase) || ((database instanceof MSSQLDatabase) && "timestamp".equalsIgnoreCase - (columnType.toString()))) { + instanceof MySQLDatabase) || ((database instanceof MSSQLDatabase) && columnType != null && "timestamp".equalsIgnoreCase (columnType.toString()))) { alterTable += " NULL"; } } diff --git a/liquibase-core/src/main/java/liquibase/statement/core/AddColumnStatement.java b/liquibase-core/src/main/java/liquibase/statement/core/AddColumnStatement.java index e0d67d07738..d15daa373ae 100644 --- a/liquibase-core/src/main/java/liquibase/statement/core/AddColumnStatement.java +++ b/liquibase-core/src/main/java/liquibase/statement/core/AddColumnStatement.java @@ -17,6 +17,7 @@ public class AddColumnStatement extends AbstractSqlStatement { private String addAfterColumn; private String addBeforeColumn; private Integer addAtPosition; + private Boolean computed; private Set constraints = new HashSet<>(); private List columns = new ArrayList<>(); @@ -215,4 +216,12 @@ public String getDefaultValueConstraintName() { public void setDefaultValueConstraintName(String defaultValueConstraintName) { this.defaultValueConstraintName = defaultValueConstraintName; } + + public Boolean getComputed() { + return computed; + } + + public void setComputed(Boolean computed) { + this.computed = computed; + } } From b4ebb1fdfa78331a69b8fd07787f638f634cdff4 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Tue, 8 Feb 2022 16:43:02 -0600 Subject: [PATCH 2/3] Added test for AddColumnChange using computed columns --- .../sqlgenerator/core/AddColumnGenerator.java | 9 +--- .../change/core/AddColumnChangeTest.groovy | 51 +++++++++++++++++-- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java index 06c2ad4ee21..24329d77713 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java @@ -2,10 +2,6 @@ import liquibase.change.ColumnConfig; import liquibase.database.Database; -import liquibase.datatype.DatabaseDataType; -import liquibase.statement.NotNullConstraint; -import liquibase.statement.core.AddUniqueConstraintStatement; -import liquibase.structure.core.Schema; import liquibase.database.core.*; import liquibase.datatype.DataTypeFactory; import liquibase.datatype.DatabaseDataType; @@ -15,10 +11,7 @@ import liquibase.sql.UnparsedSql; import liquibase.sqlgenerator.SqlGeneratorChain; import liquibase.sqlgenerator.SqlGeneratorFactory; -import liquibase.statement.AutoIncrementConstraint; -import liquibase.statement.ColumnConstraint; -import liquibase.statement.DatabaseFunction; -import liquibase.statement.ForeignKeyConstraint; +import liquibase.statement.*; import liquibase.statement.core.AddColumnStatement; import liquibase.statement.core.AddForeignKeyConstraintStatement; import liquibase.statement.core.AddUniqueConstraintStatement; diff --git a/liquibase-core/src/test/groovy/liquibase/change/core/AddColumnChangeTest.groovy b/liquibase-core/src/test/groovy/liquibase/change/core/AddColumnChangeTest.groovy index 4f2a844eb25..d709d3c2bb4 100644 --- a/liquibase-core/src/test/groovy/liquibase/change/core/AddColumnChangeTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/change/core/AddColumnChangeTest.groovy @@ -4,19 +4,62 @@ import liquibase.change.AddColumnConfig import liquibase.change.Change import liquibase.change.ChangeStatus import liquibase.change.StandardChangeTest +import liquibase.database.core.H2Database +import liquibase.database.core.MSSQLDatabase import liquibase.database.core.MockDatabase import liquibase.exception.SetupException import liquibase.parser.core.ParsedNode import liquibase.parser.core.ParsedNodeException import liquibase.snapshot.MockSnapshotGeneratorFactory import liquibase.snapshot.SnapshotGeneratorFactory +import liquibase.sqlgenerator.SqlGeneratorFactory +import liquibase.statement.SqlStatement import liquibase.structure.core.Column import liquibase.structure.core.PrimaryKey import liquibase.structure.core.Table +import spock.lang.Unroll -public class AddColumnChangeTest extends StandardChangeTest { +class AddColumnChangeTest extends StandardChangeTest { + @Unroll + def "valid setups are allowed"() { + when: + def change = new AddColumnChange() + change.setTableName("test_table") + change.addColumn(columnConfig) + + then: + !change.validate(new H2Database()).hasErrors() + + where: + columnConfig << [ + new AddColumnConfig() + .setType("int") + .setName("test_col"), + new AddColumnConfig() + .setName("payload_id AS JSON_VALUE(payload,'\$.id')") + .setComputed(true) + ] + } + + def "computed columns generate expected sql"() { + when: + def db = new MSSQLDatabase() + + def change = new AddColumnChange() + change.setTableName("test_table") + change.addColumn(new AddColumnConfig() + .setName("payload_id AS JSON_VALUE(payload,'\$.id')") + .setComputed(true)) + + def statements = change.generateStatements(db) + + then: + SqlGeneratorFactory.getInstance().generateSql(statements, db)*.toString() == ["ALTER TABLE test_table ADD payload_id AS JSON_VALUE(payload,'\$.id');"] + + } + def "add and remove column methods"() throws Exception { when: def columnA = new AddColumnConfig(); @@ -111,8 +154,8 @@ public class AddColumnChangeTest extends StandardChangeTest { when: def node = new ParsedNode(null, "addColumn") .addChildren([tableName: "table_name"]) - .addChild(new ParsedNode(null, "column").addChildren([name: "col_1", type:"int", beforeColumn: "before_col"])) - .addChild(new ParsedNode(null, "column").addChildren([name: "col_2", type:"int", position: "3"])) + .addChild(new ParsedNode(null, "column").addChildren([name: "col_1", type: "int", beforeColumn: "before_col"])) + .addChild(new ParsedNode(null, "column").addChildren([name: "col_2", type: "int", position: "3"])) def change = new AddColumnChange() try { change.load(node, resourceSupplier.simpleResourceAccessor) @@ -131,7 +174,7 @@ public class AddColumnChangeTest extends StandardChangeTest { change.columns[1].name == "col_2" change.columns[1].type == "int" - change.columns[1].position== 3 + change.columns[1].position == 3 } protected void addColumnsToSnapshot(Table table, Change change, MockSnapshotGeneratorFactory snapshotFactory) { From 67a603ac3bd5e800b7dd7f8032e699c2f90d472f Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Thu, 10 Feb 2022 10:31:00 -0600 Subject: [PATCH 3/3] Support creating a table with computed=true and no type set --- .../change/core/CreateTableChange.java | 16 +++++++++-- .../core/CreateTableGenerator.java | 21 ++++++++++---- .../statement/core/CreateTableStatement.java | 10 +++++++ .../change/core/CreateTableChangeTest.groovy | 28 ++++++++++++++----- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/change/core/CreateTableChange.java b/liquibase-core/src/main/java/liquibase/change/core/CreateTableChange.java index beff2c0b1e5..11f82bc37f9 100644 --- a/liquibase-core/src/main/java/liquibase/change/core/CreateTableChange.java +++ b/liquibase-core/src/main/java/liquibase/change/core/CreateTableChange.java @@ -16,6 +16,7 @@ import liquibase.structure.core.Column; import liquibase.structure.core.PrimaryKey; import liquibase.structure.core.Table; +import liquibase.util.ObjectUtil; import liquibase.util.StringUtil; import java.util.ArrayList; @@ -46,7 +47,7 @@ public ValidationErrors validate(Database database) { if (columns != null) { for (ColumnConfig columnConfig : columns) { - if (columnConfig.getType() == null) { + if (columnConfig.getType() == null && !ObjectUtil.defaultIfNull(columnConfig.getComputed(), false)) { validationErrors.addError("column 'type' is required for all columns"); } if (columnConfig.getName() == null) { @@ -67,8 +68,12 @@ public SqlStatement[] generateStatements(Database database) { Object defaultValue = column.getDefaultValueObject(); - LiquibaseDataType columnType = DataTypeFactory.getInstance().fromDescription(column.getType() + (isAutoIncrement ? "{autoIncrement:true}" : ""), database); - isAutoIncrement |= columnType.isAutoIncrement(); + LiquibaseDataType columnType = null; + if (column.getType() != null) { + columnType = DataTypeFactory.getInstance().fromDescription(column.getType() + (isAutoIncrement ? "{autoIncrement:true}" : ""), database); + isAutoIncrement |= columnType.isAutoIncrement(); + } + if ((constraints != null) && (constraints.isPrimaryKey() != null) && constraints.isPrimaryKey()) { statement.addPrimaryKeyColumn(column.getName(), columnType, defaultValue, constraints.getValidatePrimaryKey(), constraints.isDeferrable() != null && constraints.isDeferrable(), @@ -146,6 +151,11 @@ public SqlStatement[] generateStatements(Database database) { statements.add(remarksStatement); } } + + final Boolean computed = column.getComputed(); + if (computed != null && computed) { + statement.setComputed(column.getName()); + } } return statements.toArray(new SqlStatement[statements.size()]); diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java index 1ae4fb294ca..42f023382cc 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java @@ -13,6 +13,7 @@ import liquibase.database.core.SybaseASADatabase; import liquibase.database.core.SybaseDatabase; import liquibase.datatype.DatabaseDataType; +import liquibase.datatype.LiquibaseDataType; import liquibase.exception.DatabaseException; import liquibase.exception.ValidationErrors; import liquibase.sql.Sql; @@ -67,10 +68,18 @@ public Sql[] generateSql(CreateTableStatement statement, Database database, SqlG /* We have reached the point after "CREATE TABLE ... (" and will now iterate through the column list. */ while (columnIterator.hasNext()) { String column = columnIterator.next(); - DatabaseDataType columnType = statement.getColumnTypes().get(column).toDatabaseDataType(database); - buffer.append(database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), column, true)); + DatabaseDataType columnType = null; + if (statement.getColumnTypes().get(column) != null) { + columnType = statement.getColumnTypes().get(column).toDatabaseDataType(database); + } + + if (columnType == null) { + buffer.append(database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), column, false)); + } else { + buffer.append(database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), column, !statement.isComputed(column))); + buffer.append(" ").append(columnType); + } - buffer.append(" ").append(columnType); AutoIncrementConstraint autoIncrementConstraint = null; @@ -102,7 +111,7 @@ public Sql[] generateSql(CreateTableStatement statement, Database database, SqlG } // for the serial data type in postgres, there should be no default value - if (!columnType.isAutoIncrement() && (statement.getDefaultValue(column) != null)) { + if (columnType != null && !columnType.isAutoIncrement() && (statement.getDefaultValue(column) != null)) { Object defaultValue = statement.getDefaultValue(column); if (database instanceof MSSQLDatabase) { String constraintName = statement.getDefaultValueConstraintName(column); @@ -196,9 +205,9 @@ public Sql[] generateSql(CreateTableStatement statement, Database database, SqlG } } // does the DB support constraint names? } else { - if ((database instanceof SybaseDatabase) || (database instanceof SybaseASADatabase) || (database + if (columnType != null && ((database instanceof SybaseDatabase) || (database instanceof SybaseASADatabase) || (database instanceof MySQLDatabase) || ((database instanceof MSSQLDatabase) && columnType.toString() - .toLowerCase().contains("timestamp"))) { + .toLowerCase().contains("timestamp")))) { buffer.append(" NULL"); } // Do we need to specify NULL explicitly? } // Do we have a NOT NULL constraint for this column? diff --git a/liquibase-core/src/main/java/liquibase/statement/core/CreateTableStatement.java b/liquibase-core/src/main/java/liquibase/statement/core/CreateTableStatement.java index 0b0767e37e5..335988437f0 100644 --- a/liquibase-core/src/main/java/liquibase/statement/core/CreateTableStatement.java +++ b/liquibase-core/src/main/java/liquibase/statement/core/CreateTableStatement.java @@ -1,5 +1,6 @@ package liquibase.statement.core; +import liquibase.ContextExpression; import liquibase.datatype.LiquibaseDataType; import liquibase.statement.*; @@ -30,6 +31,7 @@ in line with the column (this implies that a NN constraint can always affects ex private HashMap notNullColumns = new HashMap<>(); private Set uniqueConstraints = new LinkedHashSet<>(); + private Set computedColumns = new HashSet<>(); public CreateTableStatement(String catalogName, String schemaName, String tableName) { this.catalogName = catalogName; @@ -254,4 +256,12 @@ public Map getDefaultValueConstraintNames() { public void setSchemaName(String schemaName) { this.schemaName = schemaName; } + + public void setComputed(String columnName) { + this.computedColumns.add(columnName); + } + + public boolean isComputed(String columnName) { + return this.computedColumns.contains(columnName); + } } diff --git a/liquibase-core/src/test/groovy/liquibase/change/core/CreateTableChangeTest.groovy b/liquibase-core/src/test/groovy/liquibase/change/core/CreateTableChangeTest.groovy index 9c636468379..7e7ed721661 100644 --- a/liquibase-core/src/test/groovy/liquibase/change/core/CreateTableChangeTest.groovy +++ b/liquibase-core/src/test/groovy/liquibase/change/core/CreateTableChangeTest.groovy @@ -1,16 +1,17 @@ package liquibase.change.core + import liquibase.change.ChangeStatus import liquibase.change.ColumnConfig import liquibase.change.ConstraintsConfig import liquibase.change.StandardChangeTest -import liquibase.database.core.PostgresDatabase +import liquibase.database.core.MSSQLDatabase import liquibase.database.core.MockDatabase +import liquibase.database.core.PostgresDatabase import liquibase.parser.core.ParsedNode import liquibase.parser.core.ParsedNodeException import liquibase.snapshot.MockSnapshotGeneratorFactory import liquibase.snapshot.SnapshotGeneratorFactory -import liquibase.sql.Sql import liquibase.sqlgenerator.SqlGeneratorFactory import liquibase.statement.DatabaseFunction import liquibase.statement.ForeignKeyConstraint @@ -289,11 +290,24 @@ public class CreateTableChangeTest extends StandardChangeTest { where: - type | autoinc | database | expected - "int" | true | new MockDatabase() | "CREATE TABLE test_table (id INT null)" - "SERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id INTEGER GENERATED BY DEFAULT AS IDENTITY)" - "SMALLSERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id SMALLSERIAL)" - "BIGSERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id BIGINT GENERATED BY DEFAULT AS IDENTITY)" + type | autoinc | database | expected + "int" | true | new MockDatabase() | "CREATE TABLE test_table (id INT null)" + "SERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id INTEGER GENERATED BY DEFAULT AS IDENTITY)" + "SMALLSERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id SMALLSERIAL)" + "BIGSERIAL" | false | new PostgresDatabase() | "CREATE TABLE test_table (id BIGINT GENERATED BY DEFAULT AS IDENTITY)" + } + + def "computed columns can be created"() throws Exception { + when: + def change = new CreateTableChange() + change.setTableName("test_table") + change.addColumn(new ColumnConfig().setName("regular_col").setType("int")) + change.addColumn(new ColumnConfig().setName("value_id AS JSON_VALUE(value,'\$.id')", true)) + + then: + !change.validate(new MSSQLDatabase()).hasErrors() + SqlGeneratorFactory.instance.generateSql(change, new MSSQLDatabase())*.toSql() == ["CREATE TABLE test_table (regular_col int, value_id AS JSON_VALUE(value,'\$.id'))"] + } def "column remarks can be set with createTable"() throws Exception {