From 77ba62405338a7a558de58ab59d47d2e66df39bf Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Wed, 6 Jul 2022 12:20:27 -0500 Subject: [PATCH 1/4] Add "may lose column details" warning for addComment on mysql --- .../java/liquibase/changelog/DatabaseChangeLog.java | 6 +++++- .../sqlgenerator/core/SetColumnRemarksGenerator.java | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java index d51e3512483..612f050a997 100644 --- a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java +++ b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java @@ -19,6 +19,7 @@ import liquibase.precondition.core.PreconditionContainer; import liquibase.resource.ResourceAccessor; import liquibase.servicelocator.LiquibaseService; +import liquibase.ui.UIService; import liquibase.util.StringUtil; import liquibase.util.FilenameUtil; @@ -299,8 +300,11 @@ public void validate(Database database, Contexts contexts, LabelExpression label validatingVisitor.validate(database, this); logIterator.run(validatingVisitor, new RuntimeEnvironment(database, contexts, labelExpression)); + final Logger log = Scope.getCurrentScope().getLog(getClass()); + final UIService ui = Scope.getCurrentScope().getUI(); for (String message : validatingVisitor.getWarnings().getMessages()) { - Scope.getCurrentScope().getLog(getClass()).warning(message); + ui.sendMessage("WARNING: "+message); + log.warning(message); } if (!validatingVisitor.validationPassed()) { diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java index 5d48bbaa040..57558b73a62 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java @@ -4,6 +4,7 @@ import liquibase.database.core.*; import liquibase.datatype.DataTypeFactory; import liquibase.exception.ValidationErrors; +import liquibase.exception.Warnings; import liquibase.sql.Sql; import liquibase.sql.UnparsedSql; import liquibase.sqlgenerator.SqlGeneratorChain; @@ -37,6 +38,16 @@ public ValidationErrors validate(SetColumnRemarksStatement setColumnRemarksState return validationErrors; } + @Override + public Warnings warn(SetColumnRemarksStatement statementType, Database database, SqlGeneratorChain sqlGeneratorChain) { + final Warnings warnings = super.warn(statementType, database, sqlGeneratorChain); + if (database instanceof MySQLDatabase) { + warnings.addWarning("setColumnRemarks will lose primary key/autoincrement/not null settings for "+database.getShortName()+". Use or and re-specify all configuration if this is the case"); + } + + return warnings; + } + @Override public Sql[] generateSql(SetColumnRemarksStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { From 9a1e96e134e5d35c5e45eec766dcccc2741ed23a Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Wed, 6 Jul 2022 12:31:14 -0500 Subject: [PATCH 2/4] Add "may lose column details" warning for addComment on mysql --- .../src/main/java/liquibase/changelog/DatabaseChangeLog.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java index 612f050a997..03184c455c8 100644 --- a/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java +++ b/liquibase-core/src/main/java/liquibase/changelog/DatabaseChangeLog.java @@ -301,9 +301,7 @@ public void validate(Database database, Contexts contexts, LabelExpression label logIterator.run(validatingVisitor, new RuntimeEnvironment(database, contexts, labelExpression)); final Logger log = Scope.getCurrentScope().getLog(getClass()); - final UIService ui = Scope.getCurrentScope().getUI(); for (String message : validatingVisitor.getWarnings().getMessages()) { - ui.sendMessage("WARNING: "+message); log.warning(message); } From 7d4fae4a8d4b16532c206c3a6ad84e0484a7726d Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Wed, 6 Jul 2022 12:53:30 -0500 Subject: [PATCH 3/4] Centralized "may lose column details" warning for mysql and added it to more SqlGenerators --- .../liquibase/database/core/MySQLDatabase.java | 6 ++++++ .../core/AddDefaultValueGenerator.java | 14 ++++++++++++++ .../sqlgenerator/core/ModifyDataTypeGenerator.java | 5 ++--- .../core/SetColumnRemarksGenerator.java | 2 +- .../sqlgenerator/core/SetNullableGenerator.java | 13 +++++++++++++ 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/database/core/MySQLDatabase.java b/liquibase-core/src/main/java/liquibase/database/core/MySQLDatabase.java index 6f1f26adf23..9b069ca572e 100644 --- a/liquibase-core/src/main/java/liquibase/database/core/MySQLDatabase.java +++ b/liquibase-core/src/main/java/liquibase/database/core/MySQLDatabase.java @@ -2,9 +2,11 @@ import liquibase.CatalogAndSchema; import liquibase.Scope; +import liquibase.change.Change; import liquibase.database.AbstractJdbcDatabase; import liquibase.database.DatabaseConnection; import liquibase.exception.DatabaseException; +import liquibase.exception.Warnings; import liquibase.executor.ExecutorService; import liquibase.statement.DatabaseFunction; import liquibase.statement.core.RawSqlStatement; @@ -661,4 +663,8 @@ private int extractPrecision(DatabaseFunction databaseFunction) { } return precision; } + + public void warnAboutAlterColumn(String changeName, Warnings warnings ) { + warnings.addWarning("Due to " + this.getShortName() + " SQL limitations, " + changeName + " will lose primary key/autoincrement/not null/comment settings explicitly redefined in the change. Use or to re-specify all configuration if this is the case"); + } } diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddDefaultValueGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddDefaultValueGenerator.java index d058b185eea..59e95c02730 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddDefaultValueGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/AddDefaultValueGenerator.java @@ -2,17 +2,20 @@ import liquibase.database.Database; import liquibase.database.core.HsqlDatabase; +import liquibase.database.core.MySQLDatabase; import liquibase.datatype.DataTypeFactory; import liquibase.datatype.LiquibaseDataType; import liquibase.datatype.core.BooleanType; import liquibase.datatype.core.CharType; import liquibase.exception.ValidationErrors; +import liquibase.exception.Warnings; import liquibase.sql.Sql; import liquibase.sql.UnparsedSql; import liquibase.sqlgenerator.SqlGeneratorChain; import liquibase.statement.DatabaseFunction; import liquibase.statement.SequenceNextValueFunction; import liquibase.statement.core.AddDefaultValueStatement; +import liquibase.statement.core.ModifyDataTypeStatement; import liquibase.structure.core.Column; import liquibase.structure.core.Schema; import liquibase.structure.core.Table; @@ -61,6 +64,17 @@ public ValidationErrors validate(AddDefaultValueStatement addDefaultValueStateme return validationErrors; } + @Override + public Warnings warn(AddDefaultValueStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { + Warnings warnings = super.warn(statement, database, sqlGeneratorChain); + + if (database instanceof MySQLDatabase) { + ((MySQLDatabase) database).warnAboutAlterColumn("addDefaultValue", warnings); + } + + return warnings; + } + @Override public Sql[] generateSql(AddDefaultValueStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { Object defaultValue = statement.getDefaultValue(); diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/ModifyDataTypeGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/ModifyDataTypeGenerator.java index 0c18c606699..1cefa52fa75 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/ModifyDataTypeGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/ModifyDataTypeGenerator.java @@ -27,9 +27,8 @@ public boolean supports(ModifyDataTypeStatement statement, Database database) { public Warnings warn(ModifyDataTypeStatement modifyDataTypeStatement, Database database, SqlGeneratorChain sqlGeneratorChain) { Warnings warnings = super.warn(modifyDataTypeStatement, database, sqlGeneratorChain); - if ((database instanceof MySQLDatabase) && !modifyDataTypeStatement.getNewDataType().toLowerCase().contains - ("varchar")) { - warnings.addWarning("modifyDataType will lose primary key/autoincrement/not null settings for mysql. Use and re-specify all configuration if this is the case"); + if (database instanceof MySQLDatabase) { + ((MySQLDatabase) database).warnAboutAlterColumn("modifyDataType", warnings); } return warnings; diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java index 57558b73a62..72901968160 100644 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetColumnRemarksGenerator.java @@ -42,7 +42,7 @@ public ValidationErrors validate(SetColumnRemarksStatement setColumnRemarksState public Warnings warn(SetColumnRemarksStatement statementType, Database database, SqlGeneratorChain sqlGeneratorChain) { final Warnings warnings = super.warn(statementType, database, sqlGeneratorChain); if (database instanceof MySQLDatabase) { - warnings.addWarning("setColumnRemarks will lose primary key/autoincrement/not null settings for "+database.getShortName()+". Use or and re-specify all configuration if this is the case"); + ((MySQLDatabase) database).warnAboutAlterColumn("setColumnRemarks", warnings); } return warnings; diff --git a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetNullableGenerator.java b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetNullableGenerator.java index 7ff3266a1cd..178ad61d5cd 100755 --- a/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetNullableGenerator.java +++ b/liquibase-core/src/main/java/liquibase/sqlgenerator/core/SetNullableGenerator.java @@ -5,10 +5,12 @@ import liquibase.datatype.DataTypeFactory; import liquibase.exception.DatabaseException; import liquibase.exception.ValidationErrors; +import liquibase.exception.Warnings; import liquibase.sql.Sql; import liquibase.sql.UnparsedSql; import liquibase.sqlgenerator.SqlGeneratorChain; import liquibase.sqlgenerator.SqlGeneratorFactory; +import liquibase.statement.core.AddDefaultValueStatement; import liquibase.statement.core.ReorganizeTableStatement; import liquibase.statement.core.SetNullableStatement; import liquibase.structure.core.Column; @@ -58,6 +60,17 @@ public ValidationErrors validate(SetNullableStatement setNullableStatement, Data return validationErrors; } + @Override + public Warnings warn(SetNullableStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { + Warnings warnings = super.warn(statement, database, sqlGeneratorChain); + + if (database instanceof MySQLDatabase) { + ((MySQLDatabase) database).warnAboutAlterColumn("setNullable", warnings); + } + + return warnings; + } + @Override public Sql[] generateSql(SetNullableStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) { String sql; From 612ca9dfed2a19b11cefc5d061280764b584769c Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Tue, 26 Jul 2022 16:37:05 -0500 Subject: [PATCH 4/4] Keep warning messages from being duplicated --- .../src/main/java/liquibase/exception/Warnings.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/liquibase-core/src/main/java/liquibase/exception/Warnings.java b/liquibase-core/src/main/java/liquibase/exception/Warnings.java index f4f8dea21b8..5cd9cab7db3 100644 --- a/liquibase-core/src/main/java/liquibase/exception/Warnings.java +++ b/liquibase-core/src/main/java/liquibase/exception/Warnings.java @@ -1,11 +1,11 @@ package liquibase.exception; -import java.util.ArrayList; -import java.util.List; +import java.util.*; public class Warnings { - private List messages = new ArrayList<>(); + //use linkedHashSet to keep them in order, but don't duplicate warnings that have already been logged + private final LinkedHashSet messages = new LinkedHashSet<>(); public Warnings addWarning(String warning) { messages.add(warning); @@ -20,7 +20,7 @@ public Warnings addAll(Warnings warnings) { } public List getMessages() { - return messages; + return new ArrayList<>(messages); } public boolean hasWarnings() {