Skip to content

Commit

Permalink
Output "may loose settings" warning on mysql/mariadb for more impacte…
Browse files Browse the repository at this point in the history
…d change types (#3045)

* Add "may lose column details" warning for addComment on mysql
* Keep warning messages from being duplicated
  • Loading branch information
nvoxland committed Jul 28, 2022
1 parent 28631d9 commit a36a972
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 8 deletions.
Expand Up @@ -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;

Expand Down Expand Up @@ -299,8 +300,9 @@ 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());
for (String message : validatingVisitor.getWarnings().getMessages()) {
Scope.getCurrentScope().getLog(getClass()).warning(message);
log.warning(message);
}

if (!validatingVisitor.validationPassed()) {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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 <sql> or <modifySql> to re-specify all configuration if this is the case");
}
}
@@ -1,11 +1,11 @@
package liquibase.exception;

import java.util.ArrayList;
import java.util.List;
import java.util.*;

public class Warnings {

private List<String> messages = new ArrayList<>();
//use linkedHashSet to keep them in order, but don't duplicate warnings that have already been logged
private final LinkedHashSet<String> messages = new LinkedHashSet<>();

public Warnings addWarning(String warning) {
messages.add(warning);
Expand All @@ -20,7 +20,7 @@ public Warnings addAll(Warnings warnings) {
}

public List<String> getMessages() {
return messages;
return new ArrayList<>(messages);
}

public boolean hasWarnings() {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Expand Up @@ -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 <sql> and re-specify all configuration if this is the case");
if (database instanceof MySQLDatabase) {
((MySQLDatabase) database).warnAboutAlterColumn("modifyDataType", warnings);
}

return warnings;
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -37,6 +38,16 @@ public ValidationErrors validate(SetColumnRemarksStatement setColumnRemarksState
return validationErrors;
}

@Override
public Warnings warn(SetColumnRemarksStatement statementType, Database database, SqlGeneratorChain<SetColumnRemarksStatement> sqlGeneratorChain) {
final Warnings warnings = super.warn(statementType, database, sqlGeneratorChain);
if (database instanceof MySQLDatabase) {
((MySQLDatabase) database).warnAboutAlterColumn("setColumnRemarks", warnings);
}

return warnings;
}

@Override
public Sql[] generateSql(SetColumnRemarksStatement statement, Database database, SqlGeneratorChain sqlGeneratorChain) {

Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit a36a972

Please sign in to comment.