From 17ddd25be69de388d2cd617c69d55546c2473846 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Fri, 25 Apr 2025 13:01:30 +0200 Subject: [PATCH] [jOOQ/jOOQ#18327] Meta.migrateTo() should defer adding FOREIGN KEY constraints after CREATE TABLE --- jOOQ/src/main/java/org/jooq/impl/DDL.java | 48 ++++++++++----- jOOQ/src/main/java/org/jooq/impl/Diff.java | 72 ++++++++++++---------- 2 files changed, 73 insertions(+), 47 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/DDL.java b/jOOQ/src/main/java/org/jooq/impl/DDL.java index 9fef44b906..fc978d2625 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DDL.java +++ b/jOOQ/src/main/java/org/jooq/impl/DDL.java @@ -153,7 +153,7 @@ final class DDL { this.configuration = configuration; } - final List createTableOrView(Table table, Collection constraints) { + private final List createTableOrViewWithInlineConstraints(Table table, Collection constraints) { boolean temporary = table.getTableType() == TableType.TEMPORARY; boolean materialized = table.getTableType() == MATERIALIZED_VIEW; boolean view = table.getTableType().isView(); @@ -323,7 +323,20 @@ final class DDL { } final List createTableOrView(Table table) { - return createTableOrView(table, constraints(table, true)); + return createTableOrView(table, new HashSet>()); + } + + private final List createTableOrView(Table table, Set> tablesWithInlineConstraints) { + List queries = new ArrayList<>(); + List constraints = new ArrayList<>(); + + if (!hasConstraintsUsingIndexes(table)) { + tablesWithInlineConstraints.add(table); + constraints.addAll(constraints(table, inlineForeignKeyDefinitions())); + } + + queries.addAll(createTableOrViewWithInlineConstraints(table, constraints)); + return queries; } final List createIndex(Table table) { @@ -350,6 +363,16 @@ final class DDL { return i.getWhere() != null ? s1.where(i.getWhere()) : s1; } + final List createForeignKey(Table table) { + List result = new ArrayList<>(); + + if (configuration.flags().contains(DDLFlag.FOREIGN_KEY)) + for (Constraint constraint : foreignKeys(table)) + result.add(ctx.alterTable(table).add(constraint)); + + return result; + } + @@ -523,6 +546,9 @@ final class DDL { else queries.addAll(alterTableAddConstraints(table)); + if (!inlineForeignKeyDefinitions()) + queries.addAll(createForeignKey(table)); + queries.addAll(createIndex(table)); queries.addAll(commentOn(table)); @@ -574,20 +600,10 @@ final class DDL { // the constraint. Set> tablesWithInlineConstraints = new HashSet<>(); - if (configuration.flags().contains(TABLE)) { - for (Schema schema : schemas) { - for (Table table : sortTablesIf(schema.getTables(), !configuration.respectTableOrder())) { - List constraints = new ArrayList<>(); - - if (!hasConstraintsUsingIndexes(table)) { - tablesWithInlineConstraints.add(table); - constraints.addAll(constraints(table, inlineForeignKeyDefinitions())); - } - - queries.addAll(createTableOrView(table, constraints)); - } - } - } + if (configuration.flags().contains(TABLE)) + for (Schema schema : schemas) + for (Table table : sortTablesIf(schema.getTables(), !configuration.respectTableOrder())) + queries.addAll(createTableOrView(table, tablesWithInlineConstraints)); if (configuration.flags().contains(INDEX)) for (Schema schema : schemas) diff --git a/jOOQ/src/main/java/org/jooq/impl/Diff.java b/jOOQ/src/main/java/org/jooq/impl/Diff.java index c8d0583feb..e6a03cfa1a 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Diff.java +++ b/jOOQ/src/main/java/org/jooq/impl/Diff.java @@ -106,6 +106,7 @@ import org.jooq.Sequence; import org.jooq.Table; import org.jooq.TableOptions.TableType; import org.jooq.UniqueKey; +import org.jooq.DDLExportConfiguration.InlineForeignKeyConstraints; import org.jooq.impl.QOM.DropDatabase; import org.jooq.impl.QOM.DropIndex; import org.jooq.tools.StringUtils; @@ -370,8 +371,8 @@ final class Diff { } else { - // [#18044] Ensure constraint / column drop / add order - DiffResult temp = new DiffResult(new ArrayList<>(), r.droppedFks); + // [#18044] [#18327] Ensure constraint / column drop / add order + DiffResult temp = new DiffResult(new ArrayList<>(), r.addedFks, r.droppedFks); appendColumns(temp, t1, t2, asList(t1.fields()), asList(t2.fields())); appendPrimaryKey(temp, t1, asList(t1.getPrimaryKey()), asList(t2.getPrimaryKey())); @@ -380,7 +381,7 @@ final class Diff { appendChecks(temp, t1, t1.getChecks(), t2.getChecks()); appendIndexes(temp, t1, t1.getIndexes(), t2.getIndexes()); - temp.queries.sort(this::sortOrder); + temp.queries.sort(Diff::sortOrder); r.addAll(temp); } @@ -396,28 +397,6 @@ final class Diff { r.queries.add(ctx.commentOnTable(t2).is(c2)); } - private int sortOrder(Query q1, Query q2) { - return sortIndex(q1) - sortIndex(q2); - } - - private int sortIndex(Query q) { - - // [#18044] DROP CONSTRAINT / INDEX before everything, ADD CONSTRAINT / INDEX after everything - if (q instanceof AlterTableImpl a) { - return a.$dropConstraint() != null - ? -1 - : a.$addConstraint() != null - ? 1 - : 0; - } - else if (q instanceof QOM.DropIndex) - return -1; - else if (q instanceof QOM.CreateIndex) - return 1; - else - return 0; - } - private void replaceView(DiffResult r, Table v1, Table v2, boolean canReplace) { if (!canReplace || v2.getTableType() == VIEW && !migrateConf.createOrReplaceView() @@ -725,7 +704,10 @@ final class Diff { } private final DiffResult appendForeignKeys(DiffResult result, final Table t1, List> fk1, List> fk2) { - final Create> create = (r, fk) -> r.queries.add(ctx.alterTable(t1).add(fk.constraint())); + final Create> create = (r, fk) -> { + if (r.addedFks.add(fk)) + r.queries.add(ctx.alterTable(t1).add(fk.constraint())); + }; final Drop> drop = (r, fk) -> { if (r.droppedFks.add(fk)) r.queries.add(ctx.alterTable(t1).dropForeignKey(fk.constraint())); @@ -813,9 +795,9 @@ final class Diff { Iterator i1 = sorted(l1, comp); Iterator i2 = sorted(l2, comp); - DiffResult dropped = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.droppedFks) : result; - DiffResult merged = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.droppedFks) : result; - DiffResult created = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.droppedFks) : result; + DiffResult dropped = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.addedFks, result.droppedFks) : result; + DiffResult merged = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.addedFks, result.droppedFks) : result; + DiffResult created = dropMergeCreate ? new DiffResult(new ArrayList<>(), result.addedFks, result.droppedFks) : result; for (;;) { if (s1 == null && i1.hasNext()) @@ -859,6 +841,7 @@ final class Diff { result.addAll(created); } + result.queries.sort(Diff::sortOrder); return result; } @@ -880,13 +863,18 @@ final class Diff { return result.iterator(); } - private static final record DiffResult(List queries, Set> droppedFks) { + private static final record DiffResult( + List queries, + Set> addedFks, + Set> droppedFks + ) { DiffResult() { - this(new ArrayList<>(), new HashSet<>()); + this(new ArrayList<>(), new HashSet<>(), new HashSet<>()); } void addAll(DiffResult other) { queries.addAll(other.queries); + addedFks.addAll(other.addedFks); droppedFks.addAll(other.droppedFks); } @@ -895,4 +883,26 @@ final class Diff { return queries.toString(); } } + + static final int sortOrder(Query q1, Query q2) { + return sortIndex(q1) - sortIndex(q2); + } + + static final int sortIndex(Query q) { + + // [#18044] DROP CONSTRAINT / INDEX before everything, ADD CONSTRAINT / INDEX after everything + if (q instanceof AlterTableImpl a) { + return a.$dropConstraint() != null + ? -1 + : a.$addConstraint() != null + ? 1 + : 0; + } + else if (q instanceof QOM.DropIndex) + return -1; + else if (q instanceof QOM.CreateIndex) + return 1; + else + return 0; + } }