fixed escaping PK and FK, refactoring

liquibase · Dec 3, 2024 · 429236b · 429236b
1 parent 2b64fff
commit 429236b
Show file tree

Hide file tree

Showing 23 changed files with 58 additions and 145 deletions.
diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml
@@ -1,5 +1,5 @@
 
-name-template: 'Support for Liquibase BigQuery Extension v$NEXT_MINOR_VERSION'
+name-template: 'Support for Liquibase BigQuery Extension v$RESOLVED_VERSION'
 tag-template: 'v$RESOLVED_VERSION'
 exclude-labels:
   - 'skipReleaseNotes'
@@ -49,4 +49,4 @@ template: |
   
   $CHANGES
 
-  **Full Changelog**: https://github.com/liquibase/liquibase-hibernate/compare/$PREVIOUS_TAG...$RESOLVED_VERSION
+  **Full Changelog**: https://github.com/liquibase/liquibase-bigquery/compare/$PREVIOUS_TAG...v$RESOLVED_VERSION
diff --git a/src/main/java/liquibase/ext/bigquery/change/BigQueryMergeColumnChange.java b/src/main/java/liquibase/ext/bigquery/change/BigQueryMergeColumnChange.java
@@ -9,7 +9,7 @@
 import liquibase.ext.bigquery.database.BigQueryDatabase;
 import liquibase.servicelocator.PrioritizedService;
 import liquibase.statement.SqlStatement;
-import liquibase.statement.core.RawSqlStatement;
+import liquibase.statement.core.RawParameterizedSqlStatement;
 import liquibase.structure.core.Column;
 
 import java.util.ArrayList;
@@ -28,7 +28,6 @@ public boolean supports(Database database) {
 
     @Override
     public SqlStatement[] generateStatements(final Database database) {
-        List<SqlStatement> statements = new ArrayList<>();
 
         AddColumnChange addNewColumnChange = new AddColumnChange();
         addNewColumnChange.setSchemaName(getSchemaName());
@@ -37,17 +36,17 @@ public SqlStatement[] generateStatements(final Database database) {
         columnConfig.setName(getFinalColumnName());
         columnConfig.setType(getFinalColumnType());
         addNewColumnChange.addColumn(columnConfig);
-        statements.addAll(Arrays.asList(addNewColumnChange.generateStatements(database)));
+        List<SqlStatement> statements = new ArrayList<>(Arrays.asList(addNewColumnChange.generateStatements(database)));
 
-        String updateStatement = "";
+        String updateStatement;
 
         updateStatement = "UPDATE " + database.escapeTableName(getCatalogName(), getSchemaName(), getTableName()) +
                 " SET " + database.escapeObjectName(getFinalColumnName(), Column.class)
                 + " = " + database.getConcatSql(database.escapeObjectName(getColumn1Name(), Column.class)
                 , "'" + getJoinString() + "'", database.escapeObjectName(getColumn2Name(), Column.class))
                 + " WHERE 1 = 1 ";
 
-        statements.add(new RawSqlStatement(updateStatement));
+        statements.add(new RawParameterizedSqlStatement(updateStatement));
 
         DropColumnChange dropColumn1Change = new DropColumnChange();
         dropColumn1Change.setSchemaName(getSchemaName());
@@ -61,6 +60,6 @@ public SqlStatement[] generateStatements(final Database database) {
         dropColumn2Change.setColumnName(getColumn2Name());
         statements.addAll(Arrays.asList(dropColumn2Change.generateStatements(database)));
 
-        return statements.toArray(new SqlStatement[statements.size()]);
+        return statements.toArray(new SqlStatement[0]);
     }
 }
diff --git a/src/main/java/liquibase/ext/bigquery/database/BigQueryDatabase.java b/src/main/java/liquibase/ext/bigquery/database/BigQueryDatabase.java
@@ -110,7 +110,7 @@ public boolean isCorrectDatabaseImplementation(DatabaseConnection conn) throws D
     }
     @Override
     public String escapeObjectName(String objectName, Class<? extends DatabaseObject> objectType) {
-        if (objectType.equals(Schema.class) || objectType.equals(Catalog.class)) {
+        if (objectType.equals(Schema.class)) {
             return objectName;
         }
         return super.escapeObjectName(objectName, objectType);
@@ -144,6 +144,7 @@ public boolean supportsNotNullConstraintNames() {
         return false;
     }
 
+    @Override
     public boolean supportsPrimaryKeyNames() {
         return false;
     }

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/BignumericDataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/BignumericDataTypeBigQuery.java
@@ -19,9 +19,6 @@ public class BignumericDataTypeBigQuery extends LiquibaseDataType {
 
     private static final String BIGNUMERIC = "BIGNUMERIC";
 
-    public BignumericDataTypeBigQuery() {
-    }
-
     @Override
     public boolean supports(Database database) {
         return database instanceof BigQueryDatabase;

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/BoolDataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/BoolDataTypeBigQuery.java
@@ -16,8 +16,6 @@
         priority = PrioritizedService.PRIORITY_DATABASE
 )
 public class BoolDataTypeBigQuery extends LiquibaseDataType {
-    public BoolDataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/Float64DataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/Float64DataTypeBigQuery.java
@@ -16,8 +16,6 @@
         priority = PrioritizedService.PRIORITY_DATABASE
 )
 public class Float64DataTypeBigQuery extends LiquibaseDataType {
-    public Float64DataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/GeographyDataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/GeographyDataTypeBigQuery.java
@@ -16,8 +16,6 @@
         priority = PrioritizedService.PRIORITY_DATABASE
 )
 public class GeographyDataTypeBigQuery extends LiquibaseDataType {
-    public GeographyDataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/Int64DataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/Int64DataTypeBigQuery.java
@@ -16,8 +16,6 @@
         priority = PrioritizedService.PRIORITY_DATABASE
 )
 public class Int64DataTypeBigQuery extends LiquibaseDataType {
-    public Int64DataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/NumericDataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/NumericDataTypeBigQuery.java
@@ -16,8 +16,6 @@
         priority = PrioritizedService.PRIORITY_DATABASE
 )
 public class NumericDataTypeBigQuery extends LiquibaseDataType {
-    public NumericDataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {

diff --git a/src/main/java/liquibase/ext/bigquery/datatype/core/StringDataTypeBigQuery.java b/src/main/java/liquibase/ext/bigquery/datatype/core/StringDataTypeBigQuery.java
@@ -17,8 +17,6 @@
         aliases = { "varchar", "clob", "java.lang.String" }
 )
 public class StringDataTypeBigQuery extends VarcharType {
-    public StringDataTypeBigQuery() {
-    }
 
     @Override
     public boolean supports(Database database) {
@@ -28,19 +26,19 @@ public boolean supports(Database database) {
     @Override
     public DatabaseDataType toDatabaseDataType(Database database) {
         if (database instanceof BigQueryDatabase) {
-
-            DatabaseDataType type = new DatabaseDataType("STRING", this.getParameters());
+            String dataTypeString = "STRING";
+            DatabaseDataType type = new DatabaseDataType(dataTypeString, this.getParameters());
             if (this.getParameters().length == 0) {
-                type.setType("STRING");
+                type.setType(dataTypeString);
             } else {
                 String firstParameter = String.valueOf(this.getParameters()[0]);
                 try {
                     int stringSize = Integer.parseInt(firstParameter);
                     if (stringSize == 65535) {
-                     type.setType("STRING");
+                     type.setType(dataTypeString);
                     }
                 } catch (NumberFormatException e) {
-                    type.setType("STRING");
+                    type.setType(dataTypeString);
                 }
             }
             return type;

diff --git a/...iquibase/ext/bigquery/diff/output/changelog/core/BigQueryChangedTableChangeGenerator.java b/...iquibase/ext/bigquery/diff/output/changelog/core/BigQueryChangedTableChangeGenerator.java
@@ -15,9 +15,6 @@
 
 public class BigQueryChangedTableChangeGenerator extends ChangedTableChangeGenerator {
 
-    public BigQueryChangedTableChangeGenerator() {
-    }
-
     @Override
     public int getPriority(Class<? extends DatabaseObject> objectType, Database database) {
         int priority = super.getPriority(objectType, database);
@@ -50,7 +47,7 @@ public Change[] fixChanged(DatabaseObject changedObject, ObjectDifferences diffe
                 Scope.getCurrentScope().getLog(this.getClass()).warning("A change of the tablespace was detected, however, Liquibase does not currently generate statements to move a table between tablespaces.");
             }
 
-            return null;
+            return new Change[0];
         }
     }
 

diff --git a/src/main/java/liquibase/ext/bigquery/executor/BigQueryExecutor.java b/src/main/java/liquibase/ext/bigquery/executor/BigQueryExecutor.java
diff --git a/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryDatasetSnapshotGenerator.java b/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryDatasetSnapshotGenerator.java
@@ -24,35 +24,26 @@ public class BigQueryDatasetSnapshotGenerator extends SchemaSnapshotGenerator {
 
     @Override
     public int getPriority(Class<? extends DatabaseObject> objectType, Database database) {
-        if (!(database instanceof BigQueryDatabase)) {
+        if (database instanceof BigQueryDatabase) {
+            return super.getPriority(objectType, database) + PRIORITY_DATABASE;
+        } else {
             return PRIORITY_NONE;
         }
-        int priority = super.getPriority(objectType, database);
-        if (priority > PRIORITY_NONE && database instanceof BigQueryDatabase) {
-            priority += PRIORITY_DATABASE;
-        }
-        return priority;
     }
 
     @Override
     protected String[] getDatabaseSchemaNames(Database database) throws SQLException, DatabaseException {
         List<String> returnList = new ArrayList<>();
 
-        ResultSet schemas = null;
-        try {
-            schemas = ((JdbcConnection) database.getConnection()).getMetaData()
-                    .getSchemas(database.getDefaultCatalogName(), null);
+        try (ResultSet schemas = ((JdbcConnection) database.getConnection()).getMetaData()
+                .getSchemas(database.getDefaultCatalogName(), null)) {
 
             while (schemas.next()) {
                 returnList.add(JdbcUtil.getValueForColumn(schemas, "TABLE_SCHEM", database));
             }
-        } finally {
-            if (schemas != null) {
-                schemas.close();
-            }
         }
 
-        return returnList.toArray(new String[returnList.size()]);
+        return returnList.toArray(new String[0]);
     }
 
     @Override
@@ -106,8 +97,7 @@ protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot
                 Catalog catalog = example1.getCatalog();
                 String[] dbCatalogNames = this.getDatabaseCatalogNames(database);
 
-                for (int i = 0; i < dbCatalogNames.length; ++i) {
-                    String candidateCatalogName = dbCatalogNames[i];
+                for (String candidateCatalogName : dbCatalogNames) {
                     if (catalog.equals(new Catalog(candidateCatalogName))) {
                         match = new Schema(catalog, catalogName);
                     }

diff --git a/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryForeignKeySnapshotGenerator.java b/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryForeignKeySnapshotGenerator.java
@@ -40,7 +40,6 @@ protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot
         String fkFullName = searchTableName + "." + example.getName();
         String searchCatalog = ((AbstractJdbcDatabase) database).getJdbcCatalogName(fkTable.getSchema());
         String searchSchema = ((AbstractJdbcDatabase) database).getJdbcSchemaName(fkTable.getSchema());
-        String systemSchema = database.getSystemSchema().toUpperCase();
 
         String query = new StringBuilder("SELECT ")
                 .append("TC.CONSTRAINT_NAME as CONSTRAINT_NAME, ")
@@ -52,9 +51,9 @@ protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot
                 .append("CCU.TABLE_SCHEMA as PRIMARY_KEY_TABLE_SCHEMA, ")
                 .append("CCU.TABLE_NAME as PRIMARY_KEY_TABLE, ")
                 .append("CCU.COLUMN_NAME as PRIMARY_KEY_COLUMN ")
-                .append(String.format("FROM %1$s.%2$s.TABLE_CONSTRAINTS as TC JOIN %1$s.%2$s.CONSTRAINT_COLUMN_USAGE as CCU on " +
-                                "TC.CONSTRAINT_NAME=CCU.CONSTRAINT_NAME JOIN %1$s.%2$s.KEY_COLUMN_USAGE as KCU on KCU.CONSTRAINT_NAME=TC.CONSTRAINT_NAME ",
-                        searchSchema, systemSchema))
+                .append(String.format("FROM %1$s.INFORMATION_SCHEMA.TABLE_CONSTRAINTS as TC JOIN %1$s.INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE as CCU on " +
+                                "TC.CONSTRAINT_NAME=CCU.CONSTRAINT_NAME JOIN %1$s.INFORMATION_SCHEMA.KEY_COLUMN_USAGE as KCU on KCU.CONSTRAINT_NAME=TC.CONSTRAINT_NAME ",
+                        database.escapeObjectName(searchSchema, Schema.class)))
                 .append("WHERE TC.TABLE_NAME=? AND TC.TABLE_SCHEMA=? AND TC.TABLE_CATALOG=? AND TC.CONSTRAINT_TYPE='FOREIGN KEY' AND TC.CONSTRAINT_NAME=?")
                 .toString();
         List<Map<String, ?>> results = Scope.getCurrentScope().getSingleton(ExecutorService.class)
@@ -113,13 +112,10 @@ protected void addTo(DatabaseObject foundObject, DatabaseSnapshot snapshot) thro
             Database database = snapshot.getDatabase();
             Schema schema = table.getSchema();
 
-            CatalogAndSchema catalogAndSchema = (new CatalogAndSchema(schema.getCatalogName(), schema.getName())).customize(database);
-            String jdbcSchemaName = database.correctObjectName(((AbstractJdbcDatabase) database).getJdbcSchemaName(catalogAndSchema), Schema.class);
-            String query = String.format("SELECT CONSTRAINT_NAME FROM %s.%s.TABLE_CONSTRAINTS WHERE TABLE_NAME=? AND TABLE_SCHEMA=? AND TABLE_CATALOG=? AND " +
-                    "CONSTRAINT_TYPE='FOREIGN KEY';", jdbcSchemaName, database.getSystemSchema().toUpperCase());
+            String query = String.format("SELECT CONSTRAINT_NAME FROM %s.%s.INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE TABLE_NAME=? AND CONSTRAINT_TYPE='FOREIGN KEY';",
+                    database.escapeObjectName(schema.getCatalogName(), Catalog.class), schema.getName());
             List<Map<String, ?>> tableConstraints = Scope.getCurrentScope().getSingleton(ExecutorService.class)
-                    .getExecutor("jdbc", database).queryForList(new RawParameterizedSqlStatement(query, table.getName(), schema.getName(),
-                            schema.getCatalogName()));
+                    .getExecutor("jdbc", database).queryForList(new RawParameterizedSqlStatement(query, table.getName()));
             for (Map<String, ?> row : tableConstraints) {
                 String foreignKeyName = Objects.toString(row.get("CONSTRAINT_NAME"));
                 ForeignKey fk = new ForeignKey()

diff --git a/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryPrimaryKeySnapshotGenerator.java b/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryPrimaryKeySnapshotGenerator.java
@@ -8,7 +8,6 @@
 import liquibase.executor.ExecutorService;
 import liquibase.ext.bigquery.database.BigQueryDatabase;
 import liquibase.snapshot.DatabaseSnapshot;
-import liquibase.snapshot.InvalidExampleException;
 import liquibase.snapshot.SnapshotGenerator;
 import liquibase.snapshot.jvm.PrimaryKeySnapshotGenerator;
 import liquibase.statement.core.RawParameterizedSqlStatement;
@@ -24,17 +23,14 @@
 import java.util.Objects;
 
 public class BigQueryPrimaryKeySnapshotGenerator extends PrimaryKeySnapshotGenerator {
-
+    private static final String CONSTRAINT_NAME = "CONSTRAINT_NAME";
     @Override
     public int getPriority(Class<? extends DatabaseObject> objectType, Database database) {
-        if (!(database instanceof BigQueryDatabase)) {
+        if (database instanceof BigQueryDatabase) {
+            return super.getPriority(objectType, database) + PRIORITY_DATABASE;
+        } else {
             return PRIORITY_NONE;
         }
-        int priority = super.getPriority(objectType, database);
-        if (priority > PRIORITY_NONE) {
-            priority += PRIORITY_DATABASE;
-        }
-        return priority;
     }
 
     @Override
@@ -43,18 +39,12 @@ public Class<? extends SnapshotGenerator>[] replaces() {
     }
 
     @Override
-    protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot snapshot) throws DatabaseException, InvalidExampleException {
+    protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot snapshot) throws DatabaseException {
         Database database = snapshot.getDatabase();
-        Schema schema = example.getSchema();
-        String searchTableName = null;
-        if (((PrimaryKey) example).getTable() != null) {
-            searchTableName = ((PrimaryKey) example).getTable().getName();
-            searchTableName = database.correctObjectName(searchTableName, Table.class);
-        }
         PrimaryKey returnKey = null;
 
         String keyColumnUsageStatement = String.format("SELECT * FROM %s.INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE CONSTRAINT_NAME = ?",
-                schema.getSchema());
+                example.getSchema().getName());
         Executor executor = Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", database);
         List<Map<String, ?>> maps = executor.queryForList(new RawParameterizedSqlStatement(keyColumnUsageStatement, example.getName()));
         String columnName;
@@ -68,7 +58,7 @@ protected DatabaseObject snapshotObject(DatabaseObject example, DatabaseSnapshot
                 String schemaName = (String) map.get("TABLE_SCHEMA");
                 CatalogAndSchema tableSchema = new CatalogAndSchema(catalogName, schemaName);
                 returnKey.setTable((Table) new Table().setName(Objects.toString(map.get("TABLE_NAME"), null)).setSchema(new Schema(tableSchema.getCatalogName(), tableSchema.getSchemaName())));
-                returnKey.setName(Objects.toString(map.get("CONSTRAINT_NAME"), null));
+                returnKey.setName(Objects.toString(map.get(CONSTRAINT_NAME), null));
             }
 
             returnKey.addColumn(position - 1, new Column(columnName)
@@ -91,16 +81,15 @@ protected void addTo(DatabaseObject foundObject, DatabaseSnapshot snapshot) thro
         if (foundObject instanceof Table) {
             Table table = (Table) foundObject;
             Database database = snapshot.getDatabase();
-            Schema schema = table.getSchema();
 
             Executor executor = Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", database);
             String tableConstraintsStatement = String.format("SELECT * FROM %s.INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE " +
-                    "CONSTRAINT_TYPE = 'PRIMARY KEY' AND table_name = ?", schema.getSchema());
+                    "CONSTRAINT_TYPE = 'PRIMARY KEY' AND table_name = ?", table.getSchema().getName());
             List<Map<String, ?>> maps = executor.queryForList(new RawParameterizedSqlStatement(tableConstraintsStatement, table.getName()));
 
             for (Map<String, ?> map : maps) {
-                if (map.containsKey("CONSTRAINT_NAME")) {
-                    String constraintName = Objects.toString(map.get("CONSTRAINT_NAME"), null);
+                if (map.containsKey(CONSTRAINT_NAME)) {
+                    String constraintName = Objects.toString(map.get(CONSTRAINT_NAME), null);
                     PrimaryKey primaryKey = new PrimaryKey().setName(constraintName);
                     primaryKey.setTable((Table) foundObject);
                     if (!database.isSystemObject(primaryKey)) {