Skip to content

Commit

Permalink
Merge pull request #2340 from liquibase/support-add-computed-column
Browse files Browse the repository at this point in the history 
Support adding computed=true columns with no type set
  • Loading branch information
@nvoxland
nvoxland committed Feb 14, 2022
2 parents c6ed3fe + 67a603a commit d4f80c3
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 33 deletions.
1 change: 1 addition & 0 deletions liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java
View file
Expand Up @@ -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());
Expand Down
16 changes: 13 additions & 3 deletions liquibase-core/src/main/java/liquibase/change/core/CreateTableChange.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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(),
Expand Down Expand Up @@ -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()]);
Expand Down
29 changes: 16 additions & 13 deletions liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java
View file
Expand Up @@ -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;
Expand All @@ -15,16 +11,14 @@
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;
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;
Expand Down Expand Up @@ -60,7 +54,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) ||
Expand Down Expand Up @@ -139,9 +135,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;

String alterTable = " ADD " + database.escapeColumnName(statement.getCatalogName(), statement.getSchemaName(), statement.getTableName(), statement.getColumnName()) + " " + columnType;
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());

if (columnType != null) {
alterTable += " " + columnType;
}

if (statement.isAutoIncrement() && database.supportsAutoIncrement()) {
AutoIncrementConstraint autoIncrementConstraint = statement.getAutoIncrementConstraint();
Expand All @@ -166,8 +170,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";
}
}
Expand Down
21 changes: 15 additions & 6 deletions liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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?
Expand Down
9 changes: 9 additions & 0 deletions liquibase-core/src/main/java/liquibase/statement/core/AddColumnStatement.java
View file
Expand Up @@ -17,6 +17,7 @@ public class AddColumnStatement extends AbstractSqlStatement {
private String addAfterColumn;
private String addBeforeColumn;
private Integer addAtPosition;
private Boolean computed;
private Set<ColumnConstraint> constraints = new HashSet<>();

private List<AddColumnStatement> columns = new ArrayList<>();
Expand Down Expand Up @@ -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;
}
}
10 changes: 10 additions & 0 deletions liquibase-core/src/main/java/liquibase/statement/core/CreateTableStatement.java
View file
@@ -1,5 +1,6 @@
package liquibase.statement.core;

import liquibase.ContextExpression;
import liquibase.datatype.LiquibaseDataType;
import liquibase.statement.*;

Expand Down Expand Up @@ -30,6 +31,7 @@ in line with the column (this implies that a NN constraint can always affects ex
private HashMap<String, NotNullConstraint> notNullColumns = new HashMap<>();

private Set<UniqueConstraint> uniqueConstraints = new LinkedHashSet<>();
private Set<String> computedColumns = new HashSet<>();

public CreateTableStatement(String catalogName, String schemaName, String tableName) {
this.catalogName = catalogName;
Expand Down Expand Up @@ -254,4 +256,12 @@ public Map<String, String> 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);
}
}
51 changes: 47 additions & 4 deletions liquibase-core/src/test/groovy/liquibase/change/core/AddColumnChangeTest.groovy
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
28 changes: 21 additions & 7 deletions liquibase-core/src/test/groovy/liquibase/change/core/CreateTableChangeTest.groovy
View file
@@ -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
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit d4f80c3

Please sign in to comment.