Merge branch 'DAT-10419' into DAT-10576

liquibase-core/src/main/java/liquibase/change/core/AddColumnChange.java

@@ -157,14 +157,9 @@ public SqlStatement[] generateStatements(Database database) {
             addColumnStatement.setDefaultValueConstraintName(column.getDefaultValueConstraintName());
             addColumnStatement.setComputed(column.getComputed());
 
-            if ((database instanceof MySQLDatabase) && (column.getAfterColumn() != null)) {
-                addColumnStatement.setAddAfterColumn(column.getAfterColumn());
-            } else if (((database instanceof HsqlDatabase) || (database instanceof H2Database))
-                       && (column.getBeforeColumn() != null)) {
-                addColumnStatement.setAddBeforeColumn(column.getBeforeColumn());
-            } else if ((database instanceof FirebirdDatabase) && (column.getPosition() != null)) {
-                addColumnStatement.setAddAtPosition(column.getPosition());
-            }
+            addColumnStatement.setAddAfterColumn(column.getAfterColumn());
+            addColumnStatement.setAddBeforeColumn(column.getBeforeColumn());
+            addColumnStatement.setAddAtPosition(column.getPosition());
 
             addColumnStatements.add(addColumnStatement);
 

liquibase-core/src/main/java/liquibase/changelog/ExpressionExpander.java

@@ -32,16 +32,15 @@ private String expandExpressions(StringReader reader, DatabaseChangeLog changeLo
                 if (nextChar == '$') {
                     reader.mark(1);
                     if (reader.read() == '{') {
-                        reader.mark(1);
-                        if (enableEscaping && reader.read() == ':') {
-                            stringBuilder.append("${");
-                            stringBuilder.append(expandExpressions(reader, changeLog, true).trim());
-                            stringBuilder.append("}");
+                        String paramExpression = expandExpressions(reader, changeLog, true);
+
+                        Object paramValue;
+                        if (paramExpression.startsWith("${")) {
+                            paramValue = paramExpression; //was not actually a valid expression
+                        } else if (paramExpression.startsWith(":") && enableEscaping) {
+                            paramValue = "${" + paramExpression.substring(1).trim() + "}";
                         } else {
-                            inExpression = true;
-                            reader.reset();
-                            String paramExpression = expandExpressions(reader, changeLog, true);
-                            Object paramValue = parameters.getValue(paramExpression.trim(), changeLog);
+                            paramValue = parameters.getValue(paramExpression.trim(), changeLog);
 
                             if (paramValue == null) {
                                 final ChangeLogParserConfiguration.MissingPropertyMode missingPropertyMode = ChangeLogParserConfiguration.MISSING_PROPERTY_MODE.getCurrentValue();
@@ -62,11 +61,14 @@ private String expandExpressions(StringReader reader, DatabaseChangeLog changeLo
                                     paramValue = expandExpressions((String) paramValue, changeLog);
                                 }
                             }
-                            stringBuilder.append(paramValue);
                         }
+
+                        stringBuilder.append(paramValue);
                     } else {
-                        reader.reset();
                         stringBuilder.append("$");
+                        reader.reset();
+                        nextChar = reader.read();
+                        continue;
                     }
                 } else {
                     if (nextChar == '}' && inExpression) {
@@ -82,6 +84,11 @@ private String expandExpressions(StringReader reader, DatabaseChangeLog changeLo
             // Unreachable as we initialize the StringReader with a non-null string
         }
 
-        return stringBuilder.toString();
+        if (inExpression) {
+            //never got to the trailing `}`, return the string as-is
+            return "${"+stringBuilder;
+        } else {
+            return stringBuilder.toString();
+        }
     }
 }

liquibase-core/src/main/java/liquibase/snapshot/JdbcDatabaseSnapshot.java

@@ -392,8 +392,8 @@ public List<CachedRow> fastFetchQuery() throws SQLException, DatabaseException {
                        extract(
                             databaseMetaData.getColumns(
                                     ((AbstractJdbcDatabase) database).getJdbcCatalogName(catalogAndSchema),
-                                    ((AbstractJdbcDatabase) database).getJdbcSchemaName(catalogAndSchema),
-                                    tableName,
+                                    escapeForLike(((AbstractJdbcDatabase) database).getJdbcSchemaName(catalogAndSchema), database),
+                                    escapeForLike(tableName, database),
                                     SQL_FILTER_MATCH_ALL)
                     );
                     //
@@ -424,8 +424,9 @@ public List<CachedRow> bulkFetchQuery() throws SQLException, DatabaseException {
                 try {
                     List<CachedRow> returnList =
                         extract(databaseMetaData.getColumns(((AbstractJdbcDatabase) database)
-                            .getJdbcCatalogName(catalogAndSchema), ((AbstractJdbcDatabase) database)
-                            .getJdbcSchemaName(catalogAndSchema), SQL_FILTER_MATCH_ALL, SQL_FILTER_MATCH_ALL));
+                                    .getJdbcCatalogName(catalogAndSchema),
+                            escapeForLike(((AbstractJdbcDatabase) database).getJdbcSchemaName(catalogAndSchema), database),
+                            SQL_FILTER_MATCH_ALL, SQL_FILTER_MATCH_ALL));
                     //
                     // IF MARIADB
                     // Query to get actual data types and then map each column to its CachedRow

liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGenerator.java

@@ -69,17 +69,17 @@ private ValidationErrors validateSingleColumn(AddColumnStatement statement, Data
             validationErrors.addError("Cannot add a non-primary key identity column");
         }
 
-        // TODO is this feature valid for other databases?
-        if ((statement.getAddAfterColumn() != null) && !(database instanceof MySQLDatabase)) {
-            validationErrors.addError("Cannot add column on specific position");
+        if (!(database instanceof MySQLDatabase || database instanceof H2Database)) {
+            validationErrors.checkDisallowedField("addAfterColumn", statement.getAddAfterColumn(), database, database.getClass());
         }
-        if ((statement.getAddBeforeColumn() != null) && !((database instanceof H2Database) || (database instanceof HsqlDatabase))) {
-            validationErrors.addError("Cannot add column on specific position");
-        }
-        if ((statement.getAddAtPosition() != null) && !(database instanceof FirebirdDatabase)) {
-            validationErrors.addError("Cannot add column on specific position");
+
+        if (!((database instanceof H2Database || database instanceof HsqlDatabase))) {
+            validationErrors.checkDisallowedField("addBeforeColumn", statement.getAddBeforeColumn(), database, database.getClass());
         }
 
+        //no databases liquibase supports currently supports adding columns at a given position. Firebird only allows position on alters
+        validationErrors.checkDisallowedField("addAtPosition", statement.getAddAtPosition(), database, database.getClass());
+
         return validationErrors;
     }
 
@@ -186,8 +186,12 @@ protected String generateSingleColumnSQL(AddColumnStatement statement, Database
             alterTable += " COMMENT '" + database.escapeStringForDatabase(StringUtil.trimToEmpty(statement.getRemarks())) + "' ";
         }
 
+        if ((statement.getAddBeforeColumn() != null) && !statement.getAddBeforeColumn().isEmpty()) {
+            alterTable += " BEFORE " + database.escapeColumnName(statement.getSchemaName(), statement.getSchemaName(), statement.getTableName(), statement.getAddBeforeColumn()) + " ";
+        }
+
         if ((statement.getAddAfterColumn() != null) && !statement.getAddAfterColumn().isEmpty()) {
-            alterTable += " AFTER `" + statement.getAddAfterColumn() + "` ";
+            alterTable += " AFTER " + database.escapeColumnName(statement.getSchemaName(), statement.getSchemaName(), statement.getTableName(), statement.getAddAfterColumn());
         }
 
         return alterTable;

liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddColumnGeneratorDefaultClauseBeforeNotNull.java

@@ -15,6 +15,7 @@
 import liquibase.sqlgenerator.SqlGeneratorChain;
 import liquibase.statement.AutoIncrementConstraint;
 import liquibase.statement.core.AddColumnStatement;
+import liquibase.util.StringUtil;
 
 public class AddColumnGeneratorDefaultClauseBeforeNotNull extends AddColumnGenerator {
     @Override
@@ -79,6 +80,15 @@ protected String generateSingleColumnSQL(AddColumnStatement statement,
                 alterTable += " PRIMARY KEY";
             }
         }
+
+        if (!StringUtil.isEmpty(statement.getAddBeforeColumn())) {
+            alterTable += " BEFORE " + database.escapeColumnName(statement.getSchemaName(), statement.getSchemaName(), statement.getTableName(), statement.getAddBeforeColumn()) + " ";
+        }
+
+        if (!StringUtil.isEmpty(statement.getAddAfterColumn())) {
+            alterTable += " AFTER " + database.escapeColumnName(statement.getSchemaName(), statement.getSchemaName(), statement.getTableName(), statement.getAddAfterColumn());
+        }
+
         return alterTable;
     }
 

liquibase-core/src/main/resources/liquibase/examples/sql/liquibase.psql.conf

@@ -0,0 +1,46 @@
+####     _     _             _ _                      _____
+##      | |   (_)           (_) |                    |  __ \
+##      | |    _  __ _ _   _ _| |__   __ _ ___  ___  | |__) | __ ___
+##      | |   | |/ _` | | | | | '_ \ / _` / __|/ _ \ |  ___/ '__/ _ \
+##      | |___| | (_| | |_| | | |_) | (_| \__ \  __/ | |   | | | (_) |
+##      \_____/_|\__, |\__,_|_|_.__/ \__,_|___/\___| |_|   |_|  \___/
+##                  | |
+##                  |_|
+##
+##      The liquibase.psql.conf file stores properties which are used during the
+##      execution of the PostgreSQL psql tool.
+##      Learn more: https://www.liquibase.org/documentation/config_properties.html
+####
+
+# The full path to the psql executable.
+# Sample Linux path
+# liquibase.psql.path=/usr/local/bin/psql
+# Sample Windows path
+# liquibase.psql.path="C:\\Program Files\\PostgreSQL\\<version>\\bin\\psql.exe"
+
+# Timeout value for the execution of the psql tool
+# Measured in seconds. -1 to disable.
+liquibase.psql.timeout=-1
+
+# Flag to indicate whether or not to keep the temporary SQL file after execution of psql.
+# True = keep False = delete (default)
+liquibase.psql.keep.temp=false
+
+# OPTIONAL Flag to designate the location to store temporary SQL file after execution of psql.
+# Liquibase will attempt to use path exactly as entered, so please ensure it complies with your OS requirements.
+# liquibase.psql.keep.temp.path=
+
+# OPTIONAL Flag to designate the name of temporary SQL file after execution of psql.
+# Liquibase will attempt to use the name exactly as entered, so please ensure it complies with your OS requirements.
+# liquibase.psql.keep.temp.name=
+
+# OPTIONAL Args to pass directly to psql.
+# Note: The delimiter for args is a space eg:" " and not "," or ";" separated.
+# liquibase.psql.args=
+
+# OPTIONAL Path to a log file for the psql output
+# liquibase.psql.logFile=
+
+# OPTIONAL Name of a custom executor to use instead of psql
+# The Executor must be on the Liquibase classpath
+# liquibase.psql.executor=

liquibase-core/src/main/resources/liquibase/examples/xml/liquibase.psql.conf

@@ -0,0 +1,46 @@
+####     _     _             _ _                      _____
+##      | |   (_)           (_) |                    |  __ \
+##      | |    _  __ _ _   _ _| |__   __ _ ___  ___  | |__) | __ ___
+##      | |   | |/ _` | | | | | '_ \ / _` / __|/ _ \ |  ___/ '__/ _ \
+##      | |___| | (_| | |_| | | |_) | (_| \__ \  __/ | |   | | | (_) |
+##      \_____/_|\__, |\__,_|_|_.__/ \__,_|___/\___| |_|   |_|  \___/
+##                  | |
+##                  |_|
+##
+##      The liquibase.psql.conf file stores properties which are used during the
+##      execution of the PostgreSQL psql tool.
+##      Learn more: https://www.liquibase.org/documentation/config_properties.html
+####
+
+# The full path to the psql executable.
+# Sample Linux path
+# liquibase.psql.path=/usr/local/bin/psql
+# Sample Windows path
+# liquibase.psql.path="C:\\Program Files\\PostgreSQL\\<version>\\bin\\psql.exe"
+
+# Timeout value for the execution of the psql tool
+# Measured in seconds. -1 to disable.
+liquibase.psql.timeout=-1
+
+# Flag to indicate whether or not to keep the temporary SQL file after execution of psql.
+# True = keep False = delete (default)
+liquibase.psql.keep.temp=false
+
+# OPTIONAL Flag to designate the location to store temporary SQL file after execution of psql.
+# Liquibase will attempt to use path exactly as entered, so please ensure it complies with your OS requirements.
+# liquibase.psql.keep.temp.path=
+
+# OPTIONAL Flag to designate the name of temporary SQL file after execution of psql.
+# Liquibase will attempt to use the name exactly as entered, so please ensure it complies with your OS requirements.
+# liquibase.psql.keep.temp.name=
+
+# OPTIONAL Args to pass directly to psql.
+# Note: The delimiter for args is a space eg:" " and not "," or ";" separated.
+# liquibase.psql.args=
+
+# OPTIONAL Path to a log file for the psql output
+# liquibase.psql.logFile=
+
+# OPTIONAL Name of a custom executor to use instead of psql
+# The Executor must be on the Liquibase classpath
+# liquibase.psql.executor=

liquibase-core/src/test/groovy/liquibase/changelog/ExpressionExpanderTest.groovy

@@ -44,6 +44,7 @@ class ExpressionExpanderTest extends Specification {
         PRESERVE | ""                                                                                                | ""
         PRESERVE | "A Simple String"                                                                                 | "A Simple String"
         PRESERVE | "With { brackets } in it"                                                                         | "With { brackets } in it"
+        PRESERVE | "With \${ unmatched"                                                                              | "With \${ unmatched"
         PRESERVE | "A string with one expression \${param1} set"                                                     | "A string with one expression value1 set"
         PRESERVE | "A string with two expressions \${param1} and \${param2} set"                                     | "A string with two expressions value1 and value2 set"
         EMPTY    | "A string with two expressions \${param1} and \${param2} set"                                     | "A string with two expressions value1 and value2 set"
@@ -69,6 +70,12 @@ class ExpressionExpanderTest extends Specification {
         PRESERVE | "valid '\${double.nested.value.valid}' double nested"                                             | "valid 'double a param with param1=value1' double nested"
         PRESERVE | "invalid '\${double.nested.value.invalid}' double nested"                                         | "invalid 'double \${nested.value.invalid}' double nested"
         EMPTY    | "invalid '\${double.nested.value.invalid}' double nested"                                         | "invalid 'double ' double nested"
+        PRESERVE | "INSERT INTO script (id, script) VALUES (5, '{test} \${test} {test} \${test} {test} ');"          | "INSERT INTO script (id, script) VALUES (5, '{test} \${test} {test} \${test} {test} ');"
+        PRESERVE | "test \$\$ here"                                                                                  | "test \$\$ here"
+        PRESERVE | "test \$1.30 here"                                                                                | "test \$1.30 here"
+        PRESERVE | "test \$1.30 here \$"                                                                             | "test \$1.30 here \$"
+        PRESERVE | "ending \${"                                                                                      | "ending \${"
+        PRESERVE | "ending \$"                                                                                       | "ending \$"
     }
 
 

liquibase-dist/pom.xml

@@ -24,8 +24,8 @@
         <postgresql.version>42.3.4</postgresql.version>
         <mssql.version>10.2.1.jre8</mssql.version>
         <mysql.version>8.0.21</mysql.version>
-        <mariadb.version>3.0.4</mariadb.version>
-        <oracle.version>21.5.0.0</oracle.version>
+        <mariadb.version>3.0.5</mariadb.version>
+        <oracle.version>21.6.0.0.1</oracle.version>
         <sqlite.version>3.36.0.3</sqlite.version>
         <db2.version>11.5.7.0</db2.version>
         <firebird.version>4.0.6.java8</firebird.version>
@@ -79,7 +79,7 @@
         <dependency>
             <groupId>net.snowflake</groupId>
             <artifactId>snowflake-jdbc</artifactId>
-            <version>3.13.19</version>
+            <version>3.13.20</version>
         </dependency>
 
         <!--        CANNOT INCLUDE MYSQL FOR LICENSING REASONS -->

liquibase-integration-tests/src/test/groovy/liquibase/snapshot/JdbcDatabaseSnapshotTest.groovy

@@ -7,22 +7,20 @@ import liquibase.extension.testing.testsystem.DatabaseTestSystem
 import liquibase.extension.testing.testsystem.TestSystemFactory
 import liquibase.statement.SqlStatement
 import liquibase.statement.core.RawSqlStatement
+import liquibase.structure.core.Column
 import liquibase.structure.core.Table
 import liquibase.structure.core.View
 import org.junit.Rule
-import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
 
-import java.sql.Connection
-
 class JdbcDatabaseSnapshotTest extends Specification {
 
     @Rule
     public DatabaseTestSystem h2 = Scope.currentScope.getSingleton(TestSystemFactory).getTestSystem("h2")
 
     @Unroll
-    def "getTables and getViews works with underscores in schema names"() {
+    def "getTables, getColumns and getViews works with underscores in schema names"() {
         when:
         def connection = h2.getConnection()
         def db = DatabaseFactory.instance.findCorrectDatabaseImplementation(new JdbcConnection(connection))
@@ -37,6 +35,9 @@ class JdbcDatabaseSnapshotTest extends Specification {
         SnapshotGeneratorFactory.instance.has(new Table(null, "TEST-SCHEMA", "TEST_TABLE"), db)
         !SnapshotGeneratorFactory.instance.has(new Table(null, "TEST_SCHEMA", "TEST_TABLE"), db)
 
+        SnapshotGeneratorFactory.instance.has(new Column(Table.class,null, "TEST-SCHEMA", "TEST_TABLE", "ID"), db)
+        !SnapshotGeneratorFactory.instance.has(new Column(Table.class, null, "TEST_SCHEMA", "TEST_TABLE","ID"), db)
+
         SnapshotGeneratorFactory.instance.has(new View(null, "TEST-SCHEMA", "TEST_VIEW"), db)
         !SnapshotGeneratorFactory.instance.has(new View(null, "TEST_SCHEMA", "TEST_VIEW"), db)