Skip to content

Commit

Permalink
Merge pull request #3026 from liquibase/2863
Browse files Browse the repository at this point in the history 
Fixed autoIncrement incrementBy/startWith support in MySQL, H2, HSQLDB, and MariaDB
  • Loading branch information
@nvoxland
nvoxland committed Jul 20, 2022
2 parents a565aaf + ee29021 commit 9b4f534
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 27 deletions.
37 changes: 20 additions & 17 deletions liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java
View file
Expand Up @@ -310,7 +310,7 @@ public String correctObjectName(final String objectName, final Class<? extends D
if (isCatalogOrSchemaType(objectType) && preserveCaseIfRequested() == CatalogAndSchema.CatalogAndSchemaCase.ORIGINAL_CASE) {
return objectName;
} else if ((getObjectQuotingStrategy() == ObjectQuotingStrategy.QUOTE_ALL_OBJECTS) || (unquotedObjectsAreUppercased == null) ||
( objectName == null) || (objectName.startsWith(getQuotingStartCharacter()) && objectName.endsWith(getQuotingEndCharacter()))) {
(objectName == null) || (objectName.startsWith(getQuotingStartCharacter()) && objectName.endsWith(getQuotingEndCharacter()))) {
return objectName;
} else if (Boolean.TRUE.equals(unquotedObjectsAreUppercased)) {
return objectName.toUpperCase(Locale.US);
Expand All @@ -322,9 +322,10 @@ public String correctObjectName(final String objectName, final Class<? extends D
private boolean isCatalogOrSchemaType(Class<? extends DatabaseObject> objectType) {
return objectType.equals(Catalog.class) || objectType.equals(Schema.class);
}

private CatalogAndSchema.CatalogAndSchemaCase preserveCaseIfRequested() {
if (Boolean.TRUE.equals(GlobalConfiguration.PRESERVE_SCHEMA_CASE.getCurrentValue())) {
return CatalogAndSchema.CatalogAndSchemaCase.ORIGINAL_CASE;
return CatalogAndSchema.CatalogAndSchemaCase.ORIGINAL_CASE;
}
return getSchemaAndCatalogCase();
}
Expand Down Expand Up @@ -368,6 +369,7 @@ public void setDefaultSchemaName(final String schemaName) {
/**
* Overwrite this method to get the default schema name for the connection.
* If you only need to change the statement that obtains the current schema then override
*
* @see AbstractJdbcDatabase#getConnectionSchemaNameCallStatement()
*/
protected String getConnectionSchemaName() {
Expand Down Expand Up @@ -395,9 +397,10 @@ protected String getConnectionSchemaName() {
* Used to obtain the connection schema name through a statement
* Override this method to change the statement.
* Only override this if getConnectionSchemaName is left unchanges or is using this method.
*
* @see AbstractJdbcDatabase#getConnectionSchemaName()
*/
protected SqlStatement getConnectionSchemaNameCallStatement(){
protected SqlStatement getConnectionSchemaNameCallStatement() {
return new RawCallStatement("call current_schema");
}

Expand Down Expand Up @@ -485,7 +488,7 @@ public String getDateLiteral(final Date date) {
return getTimeLiteral(((java.sql.Time) date));
} else if (date instanceof java.sql.Timestamp) {
return getDateTimeLiteral(((java.sql.Timestamp) date));
} else if(date instanceof java.util.Date) {
} else if (date instanceof java.util.Date) {
return getDateTimeLiteral(new java.sql.Timestamp(date.getTime()));
} else {
throw new RuntimeException("Unexpected type: " + date.getClass().getName());
Expand Down Expand Up @@ -581,14 +584,14 @@ public String getAutoIncrementClause(final BigInteger startWith, final BigIntege

if (generateStartWith || generateIncrementBy) {
autoIncrementClause += getAutoIncrementOpening();

if (generateStartWith) {
autoIncrementClause += String.format(getAutoIncrementStartWithClause(), (startWith == null) ? defaultAutoIncrementStartWith : startWith);
}

if (generateIncrementBy) {
if (generateStartWith) {
autoIncrementClause += ", ";

}

autoIncrementClause += String.format(getAutoIncrementByClause(), (incrementBy == null) ? defaultAutoIncrementBy : incrementBy);
Expand Down Expand Up @@ -758,8 +761,8 @@ public boolean isReservedWord(final String string) {
}

/*
* Check if given string starts with numeric values that may cause problems and should be escaped.
*/
* Check if given string starts with numeric values that may cause problems and should be escaped.
*/
protected boolean startsWithNumeric(final String objectName) {
return STARTS_WITH_NUMBER_PATTERN.matcher(objectName).matches();
}
Expand Down Expand Up @@ -797,7 +800,7 @@ public void dropDatabaseObjects(final CatalogAndSchema schemaToDrop) throws Liqu

final long changeSetStarted = System.currentTimeMillis();
CompareControl compareControl = new CompareControl(
new CompareControl.SchemaComparison[] {
new CompareControl.SchemaComparison[]{
new CompareControl.SchemaComparison(
CatalogAndSchema.DEFAULT,
schemaToDrop)},
Expand Down Expand Up @@ -849,7 +852,7 @@ public void dropDatabaseObjects(final CatalogAndSchema schemaToDrop) throws Liqu
@Override
public boolean supportsDropTableCascadeConstraints() {
return ((this instanceof SQLiteDatabase) || (this instanceof SybaseDatabase) || (this instanceof
SybaseASADatabase) || (this instanceof PostgresDatabase) || (this instanceof OracleDatabase));
SybaseASADatabase) || (this instanceof PostgresDatabase) || (this instanceof OracleDatabase));
}

@Override
Expand All @@ -858,7 +861,7 @@ public boolean isSystemObject(final DatabaseObject example) {
return false;
}
if ((example.getSchema() != null) && (example.getSchema().getName() != null) && "information_schema"
.equalsIgnoreCase(example.getSchema().getName())) {
.equalsIgnoreCase(example.getSchema().getName())) {
return true;
}
if ((example instanceof Table) && getSystemTables().contains(example.getName())) {
Expand Down Expand Up @@ -1264,7 +1267,6 @@ public void setAutoCommit(final boolean b) throws DatabaseException {
* Default implementation, just look for "local" IPs. If the database returns a null URL we return false since we don't know it's safe to run the update.
*
* @throws liquibase.exception.DatabaseException
*
*/
@Override
public boolean isSafeToRunUpdate() throws DatabaseException {
Expand Down Expand Up @@ -1304,7 +1306,7 @@ public void execute(final SqlStatement[] statements, final List<SqlVisitor> sqlV
Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", this).execute(statement, sqlVisitors);
} catch (DatabaseException e) {
if (statement.continueOnError()) {
Scope.getCurrentScope().getLog(getClass()).severe("Error executing statement '"+statement.toString()+"', but continuing", e);
Scope.getCurrentScope().getLog(getClass()).severe("Error executing statement '" + statement.toString() + "', but continuing", e);
} else {
throw e;
}
Expand All @@ -1314,7 +1316,7 @@ public void execute(final SqlStatement[] statements, final List<SqlVisitor> sqlV

@Override
public void saveStatements(final Change change, final List<SqlVisitor> sqlVisitors, final Writer writer) throws
IOException {
IOException {
SqlStatement[] statements = change.generateStatements(this);
for (SqlStatement statement : statements) {
for (Sql sql : SqlGeneratorFactory.getInstance().generateSql(statement, this)) {
Expand Down Expand Up @@ -1471,7 +1473,7 @@ public String generateDatabaseFunctionValue(final DatabaseFunction databaseFunct
if (sequenceNextValueFunction.contains("'")) {
/* For PostgreSQL, the quotes around dangerous identifiers (e.g. mixed-case) need to stay in place,
* or else PostgreSQL will not be able to find the sequence. */
if (! (this instanceof PostgresDatabase)) {
if (!(this instanceof PostgresDatabase)) {
sequenceName = sequenceName.replace("\"", "");
}
}
Expand All @@ -1490,7 +1492,7 @@ public String generateDatabaseFunctionValue(final DatabaseFunction databaseFunct
if (sequenceCurrentValueFunction.contains("'")) {
/* For PostgreSQL, the quotes around dangerous identifiers (e.g. mixed-case) need to stay in place,
* or else PostgreSQL will not be able to find the sequence. */
if (! (this instanceof PostgresDatabase)) {
if (!(this instanceof PostgresDatabase)) {
sequenceName = sequenceName.replace("\"", "");
}
}
Expand Down Expand Up @@ -1572,7 +1574,7 @@ public boolean supportsPrimaryKeyNames() {
}

@Override
public String getSystemSchema(){
public String getSystemSchema() {
return "information_schema";
}

Expand Down Expand Up @@ -1643,7 +1645,7 @@ public boolean supportsBatchUpdates() throws DatabaseException {
if (connection instanceof OfflineConnection) {
return false;
} else if (connection instanceof JdbcConnection) {
return ((JdbcConnection)getConnection()).supportsBatchUpdates();
return ((JdbcConnection) getConnection()).supportsBatchUpdates();
} else {
// Normally, the connection can only be one of the two above types. But if, for whatever reason, it is
// not, let's err on the safe side.
Expand All @@ -1664,6 +1666,7 @@ public boolean requiresExplicitNullForColumns() {

/**
* This logic is used when db support catalogs
*
* @return UPPER_CASE by default
*/
@Override
Expand Down
13 changes: 7 additions & 6 deletions liquibase-core/src/main/java/liquibase/database/core/H2Database.java
View file
Expand Up @@ -14,6 +14,7 @@
import liquibase.util.JdbcUtil;

import java.lang.reflect.Method;
import java.math.BigInteger;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
Expand Down Expand Up @@ -303,13 +304,13 @@ protected String getAutoIncrementClause() {
}

@Override
protected String getAutoIncrementStartWithClause() {
return "%d";
}
public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) {
final String clause = super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull);
if (clause.startsWith("AUTO_INCREMENT")) {
return clause;
}

@Override
protected String getAutoIncrementByClause() {
return "%d";
return clause.replace(",", ""); //h2 doesn't use commas between the values
}

@Override
Expand Down
7 changes: 7 additions & 0 deletions liquibase-core/src/main/java/liquibase/database/core/HsqlDatabase.java
View file
Expand Up @@ -17,6 +17,7 @@
import static liquibase.statement.DatabaseFunction.CURRENT_DATE_TIME_PLACE_HOLDER;

public class HsqlDatabase extends AbstractJdbcDatabase {

private static final Map<String, HashSet<String>> SUPPORTED_DEFAULT_VALUE_COMPUTED_MAP;
private static String START_CONCAT = "CONCAT(";
private static String END_CONCAT = ")";
Expand Down Expand Up @@ -512,4 +513,10 @@ public int getMaxFractionalDigitsForTimestamp() {
// this value is derived from tests.
return 9;
}

@Override
public String getAutoIncrementClause(BigInteger startWith, BigInteger incrementBy, String generationType, Boolean defaultOnNull) {
final String clause = super.getAutoIncrementClause(startWith, incrementBy, generationType, defaultOnNull);
return clause.replace(",", ""); //sql doesn't use commas between the values
}
}
10 changes: 9 additions & 1 deletion liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddAutoIncrementGeneratorMySQL.java
View file
Expand Up @@ -2,6 +2,7 @@

import liquibase.database.Database;
import liquibase.database.core.MySQLDatabase;
import liquibase.exception.ValidationErrors;
import liquibase.sql.Sql;
import liquibase.sql.UnparsedSql;
import liquibase.sqlgenerator.SqlGeneratorChain;
Expand Down Expand Up @@ -40,6 +41,13 @@ public Sql[] generateSql(final AddAutoIncrementStatement statement, Database dat
return sql;
}

@Override
public ValidationErrors validate(AddAutoIncrementStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) {
ValidationErrors validationErrors = super.validate(statement, database, sqlGeneratorChain);
validationErrors.checkDisallowedField("incrementBy", statement.getIncrementBy(), database, MySQLDatabase.class);
return validationErrors;
}

private Sql[] concact(Sql[] origSql, UnparsedSql unparsedSql) {
Sql[] changedSql = new Sql[origSql.length+1];
System.arraycopy(origSql, 0, changedSql, 0, origSql.length);
Expand All @@ -51,4 +59,4 @@ private Sql[] concact(Sql[] origSql, UnparsedSql unparsedSql) {
private DatabaseObject getAffectedTable(AddAutoIncrementStatement statement) {
return new Table().setName(statement.getTableName()).setSchema(new Schema(statement.getCatalogName(), statement.getSchemaName()));
}
}
}
6 changes: 6 additions & 0 deletions liquibase-core/src/main/java/liquibase/sqlgenerator/core/CreateTableGenerator.java
View file
Expand Up @@ -43,6 +43,12 @@ public ValidationErrors validate(CreateTableStatement createTableStatement, Data
ValidationErrors validationErrors = new ValidationErrors();
validationErrors.checkRequiredField("tableName", createTableStatement.getTableName());
validationErrors.checkRequiredField("columns", createTableStatement.getColumns());

if (createTableStatement.getAutoIncrementConstraints() != null) {
for (AutoIncrementConstraint constraint : createTableStatement.getAutoIncrementConstraints()) {
validationErrors.checkDisallowedField("incrementBy", constraint.getIncrementBy(), database, MySQLDatabase.class);
}
}
return validationErrors;
}

Expand Down
6 changes: 3 additions & 3 deletions liquibase-core/src/test/java/liquibase/sqlgenerator/core/CreateTableGeneratorTest.java
View file
Expand Up @@ -540,7 +540,7 @@ public void testAutoIncrementStartWithH2Database() throws Exception {

Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null);

assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (0))", generatedSql[0].toSql());
assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 0))", generatedSql[0].toSql());
}
}
}
Expand All @@ -558,7 +558,7 @@ public void testAutoIncrementStartWithIncrementByH2Database() throws Exception {

Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null);

assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (0, 10))", generatedSql[0].toSql());
assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 0 INCREMENT BY 10))", generatedSql[0].toSql());
}
}
}
Expand Down Expand Up @@ -612,7 +612,7 @@ public void testAutoIncrementStartWithIncrementByHsqlDatabase() throws Exception

Sql[] generatedSql = this.generatorUnderTest.generateSql(statement, database, null);

assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1, INCREMENT BY 10))", generatedSql[0].toSql());
assertEquals("CREATE TABLE SCHEMA_NAME.TABLE_NAME (COLUMN1_NAME BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1 INCREMENT BY 10))", generatedSql[0].toSql());
}
}
}
Expand Down

0 comments on commit 9b4f534

Please sign in to comment.