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

Handle java.lang.StackOverflowError when running generate-changelog (DAT-16812) #5545

Merged
merged 4 commits into from
Feb 14, 2024
Merged

Handle java.lang.StackOverflowError when running generate-changelog (DAT-16812) #5545

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c219463
fix: Validate the error code and not the whole error message as peopl…
filipelautert Feb 1, 2024
86e4442
feat: Implements skipObjectSorting option in GenerateChangelog/DiffCh…
filipelautert Feb 1, 2024
3b73111
fix: fix command tests
filipelautert Feb 1, 2024
7144b2c
fix(lang): save, not gain
filipelautert Feb 5, 2024
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
2 changes: 2 additions & 0 deletions ...on-tests/src/test/resources/liquibase/extension/testing/command/diffChangelog.test.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ Optional Args:
Default: none
schemas (String) Schemas to include in diff
Default: null
skipObjectSorting (Boolean) When true will skip object sorting. This can be useful on databases that have a lot of packages/procedures that are linked to each other
Default: false
useOrReplaceOption (Boolean) If true, will add 'OR REPLACE' option to the create view change object
Default: false
username (String) Username to use to connect to the database
Expand Down
2 changes: 2 additions & 0 deletions ...ests/src/test/resources/liquibase/extension/testing/command/generateChangelog.test.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ Optional Args:
Default: none
schemas (String) Schemas to include in diff
Default: null
skipObjectSorting (Boolean) When true will skip object sorting. This can be useful on databases that have a lot of packages/procedures that are linked to each other
Default: false
useOrReplaceOption (Boolean) If true, will add 'OR REPLACE' option to the create view change object
Default: false
username (String) Username to use to connect to the database
Expand Down
8 changes: 4 additions & 4 deletions liquibase-standard/src/main/java/liquibase/command/core/DiffChangelogCommandStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ public class DiffChangelogCommandStep extends AbstractChangelogCommandStep {
.setValueHandler(ValueHandlerUtil::booleanValueHandler)
.build();
builder.addArgument(AbstractChangelogCommandStep.RUN_ON_CHANGE_TYPES_ARG).build();

builder.addArgument(AbstractChangelogCommandStep.REPLACE_IF_EXISTS_TYPES_ARG).build();
builder.addArgument(AbstractChangelogCommandStep.SKIP_OBJECT_SORTING).build();
}

@Override
Expand Down Expand Up @@ -96,7 +96,7 @@ public void run(CommandResultsBuilder resultsBuilder) throws Exception {
Scope.getCurrentScope().addMdcValue(MdcKey.DIFF_CHANGELOG_FILE, changeLogFile);
referenceDatabase.setObjectQuotingStrategy(ObjectQuotingStrategy.QUOTE_ALL_OBJECTS);

DiffToChangeLog changeLogWriter = createDiffToChangeLogObject(diffResult, diffOutputControl);
DiffToChangeLog changeLogWriter = createDiffToChangeLogObject(diffResult, diffOutputControl, commandScope.getArgumentValue(AbstractChangelogCommandStep.SKIP_OBJECT_SORTING));
changeLogWriter.setChangeSetContext(commandScope.getArgumentValue(CONTEXTS_ARG));
changeLogWriter.setChangeSetLabels(commandScope.getArgumentValue(LABEL_FILTER_ARG));
changeLogWriter.setChangeSetAuthor(commandScope.getArgumentValue(AUTHOR_ARG));
Expand Down Expand Up @@ -130,8 +130,8 @@ public void validate(CommandScope commandScope) throws CommandValidationExceptio
validateRunOnChangeTypes(commandScope);
}

protected DiffToChangeLog createDiffToChangeLogObject(DiffResult diffResult, DiffOutputControl diffOutputControl) {
return new DiffToChangeLog(diffResult, diffOutputControl);
protected DiffToChangeLog createDiffToChangeLogObject(DiffResult diffResult, DiffOutputControl diffOutputControl, boolean skipObjectSorting) {
return new DiffToChangeLog(diffResult, diffOutputControl, skipObjectSorting);
}

}
5 changes: 2 additions & 3 deletions liquibase-standard/src/main/java/liquibase/command/core/GenerateChangelogCommandStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.io.PrintStream;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class GenerateChangelogCommandStep extends AbstractChangelogCommandStep {

Expand Down Expand Up @@ -98,8 +97,8 @@ public class GenerateChangelogCommandStep extends AbstractChangelogCommandStep {
.setValueHandler(ValueHandlerUtil::booleanValueHandler)
.build();
builder.addArgument(AbstractChangelogCommandStep.RUN_ON_CHANGE_TYPES_ARG).build();

builder.addArgument(AbstractChangelogCommandStep.REPLACE_IF_EXISTS_TYPES_ARG).build();
builder.addArgument(AbstractChangelogCommandStep.SKIP_OBJECT_SORTING).build();
}

@Override
Expand Down Expand Up @@ -135,7 +134,7 @@ public void run(CommandResultsBuilder resultsBuilder) throws Exception {

DiffResult diffResult = (DiffResult) resultsBuilder.getResult(DiffCommandStep.DIFF_RESULT.getName());

DiffToChangeLog changeLogWriter = new DiffToChangeLog(diffResult, diffOutputControl);
DiffToChangeLog changeLogWriter = new DiffToChangeLog(diffResult, diffOutputControl, commandScope.getArgumentValue(SKIP_OBJECT_SORTING));

changeLogWriter.setChangeSetAuthor(commandScope.getArgumentValue(AUTHOR_ARG));
if (commandScope.getArgumentValue(CONTEXT_ARG) != null) {
Expand Down
5 changes: 5 additions & 0 deletions ...e-standard/src/main/java/liquibase/command/core/helpers/AbstractChangelogCommandStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public abstract class AbstractChangelogCommandStep extends AbstractCommandStep {
protected static final String[] COMMAND_NAME = {"abstractChangelogCommandStep"};
public static final CommandArgumentDefinition<String> RUN_ON_CHANGE_TYPES_ARG;
public static final CommandArgumentDefinition<String> REPLACE_IF_EXISTS_TYPES_ARG;
public static final CommandArgumentDefinition<Boolean> SKIP_OBJECT_SORTING;

static {
final CommandBuilder builder = new CommandBuilder(COMMAND_NAME);
Expand All @@ -29,6 +30,10 @@ public abstract class AbstractChangelogCommandStep extends AbstractCommandStep {
REPLACE_IF_EXISTS_TYPES_ARG = builder.argument("replaceIfExistsTypes", String.class)
.defaultValue("none")
.description(String.format("Sets replaceIfExists=\"true\" for changes of these types (supported types: %s)", replaceIfExistsTypeNames)).build();
SKIP_OBJECT_SORTING = builder.argument("skipObjectSorting", Boolean.class)
.defaultValue(false)
.description("When true will skip object sorting. This can be useful on databases that have a lot of packages/procedures that are " +
"linked to each other").build();
}

protected static void validateRunOnChangeTypes(final CommandScope commandScope) throws CommandValidationException {
Expand Down
68 changes: 33 additions & 35 deletions liquibase-standard/src/main/java/liquibase/diff/output/changelog/DiffToChangeLog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
import liquibase.changelog.ChangeSet;
import liquibase.changeset.ChangeSetService;
import liquibase.changeset.ChangeSetServiceFactory;
import liquibase.command.CommandScope;
import liquibase.command.core.GenerateChangelogCommandStep;
import liquibase.command.core.helpers.AbstractChangelogCommandStep;
import liquibase.configuration.core.DeprecatedConfigurationValueProvider;
import liquibase.database.*;
import liquibase.database.core.*;
Expand All @@ -35,8 +34,6 @@
import liquibase.structure.DatabaseObject;
import liquibase.structure.core.Column;
import liquibase.structure.core.StoredDatabaseLogic;
import liquibase.structure.core.StoredProcedure;
import liquibase.structure.core.View;
import liquibase.util.DependencyUtil;
import liquibase.util.StreamUtil;
import liquibase.util.StringUtil;
Expand Down Expand Up @@ -73,24 +70,37 @@ public class DiffToChangeLog {
private final DiffOutputControl diffOutputControl;
private boolean tryDbaDependencies = true;

private boolean skipObjectSorting = false;

private static final Set<Class> loggedOrderFor = new HashSet<>();

/**
* Creates a new DiffToChangeLog with the given DiffResult and default DiffOutputControl
* @param diffResult the DiffResult to convert to a ChangeLog
* @param diffOutputControl the DiffOutputControl to use to control the output
* @param skipObjectSorting if true, will skip dependency object sorting. This can be useful on databases that have a lot of packages/procedures that are linked to each other
*/
public DiffToChangeLog(DiffResult diffResult, DiffOutputControl diffOutputControl, boolean skipObjectSorting) {
this(diffResult, diffOutputControl);
this.skipObjectSorting = skipObjectSorting;
}

public DiffToChangeLog(DiffResult diffResult, DiffOutputControl diffOutputControl) {
this.diffResult = diffResult;
this.diffOutputControl = diffOutputControl;
respectSchemaAndCatalogCaseIfNeeded(diffOutputControl);
}

public DiffToChangeLog(DiffOutputControl diffOutputControl) {
this.diffOutputControl = diffOutputControl;
}

private void respectSchemaAndCatalogCaseIfNeeded(DiffOutputControl diffOutputControl) {
if (this.diffResult.getComparisonSnapshot().getDatabase() instanceof AbstractDb2Database) {
diffOutputControl.setRespectSchemaAndCatalogCase(true);
}
}

public DiffToChangeLog(DiffOutputControl diffOutputControl) {
this.diffOutputControl = diffOutputControl;
}

public void setDiffResult(DiffResult diffResult) {
this.diffResult = diffResult;
}
Expand Down Expand Up @@ -367,10 +377,10 @@ private void setReplaceIfExistsTrueIfApplicable(Change[] changes) {
// drop FK goes first
//
private List<ChangeSet> bringDropFKToTop(List<ChangeSet> changeSets) {
List<ChangeSet> dropFk = changeSets.stream().filter(cs -> {
return cs.getChanges().stream().anyMatch(ch -> ch instanceof DropForeignKeyConstraintChange);
}).collect(Collectors.toList());
if (dropFk == null || dropFk.isEmpty()) {
List<ChangeSet> dropFk = changeSets.stream().filter(cs ->
cs.getChanges().stream().anyMatch(DropForeignKeyConstraintChange.class::isInstance)
).collect(Collectors.toList());
if (dropFk.isEmpty()) {
return changeSets;
}
List<ChangeSet> returnList = new ArrayList<>();
Expand Down Expand Up @@ -493,6 +503,10 @@ private List<DatabaseObject> sortObjects(final String type, Collection<DatabaseO
}
} catch (DatabaseException e) {
Scope.getCurrentScope().getLog(getClass()).fine("Cannot get object dependencies: " + e.getMessage());
} catch (StackOverflowError e) {
filipelautert marked this conversation as resolved.
Show resolved Hide resolved
Scope.getCurrentScope().getLog(getClass()).warning("You have too many or recursive database object dependencies! " +
"Liquibase is going to ignore dependency sorting and resume processing. To skip this message " +
"(and gain a lot of processing time) use flag " + AbstractChangelogCommandStep.SKIP_OBJECT_SORTING.getName(), e);
filipelautert marked this conversation as resolved.
Show resolved Hide resolved
}
}
return new ArrayList<>(objects);
Expand All @@ -517,9 +531,7 @@ private List<DatabaseObject> sortObjects(final String type, Collection<DatabaseO
// to stop the recursion
//
String message = dbe.getMessage();
if (!message.contains("ORA-00942: table or view does not exist")) {
throw new DatabaseException(dbe);
} else if (!tryDbaDependencies) {
if (!message.contains("ORA-00942") || !tryDbaDependencies) {
throw new DatabaseException(dbe);
}
Scope.getCurrentScope().getLog(getClass()).warning("Unable to query DBA_DEPENDENCIES table. Switching to USER_DEPENDENCIES");
Expand All @@ -533,6 +545,9 @@ private List<DatabaseObject> sortObjects(final String type, Collection<DatabaseO
* Used by {@link #sortMissingObjects(Collection, Database)} to determine whether to go into the sorting logic.
*/
protected boolean supportsSortingObjects(Database database) {
if (this.skipObjectSorting) {
return false;
}
return (database instanceof AbstractDb2Database) || (database instanceof MSSQLDatabase) || (database instanceof
OracleDatabase) || database instanceof PostgresDatabase;
}
Expand Down Expand Up @@ -766,13 +781,6 @@ protected List<Class<? extends DatabaseObject>> getOrderedOutputTypes(Class<? ex

private void addToChangeSets(Change[] changes, List<ChangeSet> changeSets, ObjectQuotingStrategy quotingStrategy, String created) {
if (changes != null) {
String csContext = this.changeSetContext;

if (diffOutputControl.getContext() != null) {
csContext = diffOutputControl.getContext().toString().replaceFirst("^\\(", "")
.replaceFirst("\\)$", "");
}

ChangeSetService service = ChangeSetServiceFactory.getInstance().createChangeSetService();
if (useSeparateChangeSets(changes)) {
for (Change change : changes) {
Expand All @@ -793,7 +801,7 @@ private void addToChangeSets(Change[] changes, List<ChangeSet> changeSets, Objec
changeSets.add(changeSet);
}
} else {
final boolean runOnChange = Arrays.asList(changes).stream().allMatch(change -> isContainedInRunOnChangeTypes(change));
final boolean runOnChange = Arrays.asList(changes).stream().allMatch(this::isContainedInRunOnChangeTypes);
ChangeSet changeSet = service.createChangeSet(generateId(changes), getChangeSetAuthor(), false, runOnChange, this.changeSetPath, changeSetContext,
null, null, null, true, quotingStrategy, null);
changeSet.setCreated(created);
Expand Down Expand Up @@ -1039,20 +1047,10 @@ private void checkForCycleInDependencies(Class<? extends ChangeGenerator> genera
}
}


private Node getNode(Class<? extends DatabaseObject> type) {
Node node = allNodes.get(type);
if (node == null) {
node = new Node(type);
}
return node;
}


static class Node {
public final Class<? extends DatabaseObject> type;
public final HashSet<Edge> inEdges;
public final HashSet<Edge> outEdges;
public final Set<Edge> inEdges;
public final Set<Edge> outEdges;

public Node(Class<? extends DatabaseObject> type) {
this.type = type;
Expand Down