Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed autoIncrement incrementBy/startWith support in MySQL, H2, HSQLDB, and MariaDB #3026

Merged
merged 5 commits into from Jul 20, 2022
Merged

Fixed autoIncrement incrementBy/startWith support in MySQL, H2, HSQLDB, and MariaDB #3026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f26d2c8
2863: autoIncrement issue fixed: MySQL,H2,HSQLDB,MariaDB.
Jun 30, 2022
9215487
2863: refact H2,HSQL auto increment clause, abstract hsql and h2 data…
Jun 30, 2022
17293fc
Merge branch 'master' into 2863
nvoxland Jul 6, 2022
9f80bc0
Fixing autoincrement logic for h2, hsql, and mysql
nvoxland Jul 7, 2022
ee29021
Added validation warning for incrementBy on mysql
nvoxland Jul 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 20 additions & 17 deletions liquibase-core/src/main/java/liquibase/database/AbstractJdbcDatabase.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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