From d0835cd66557243d81a0cb73500afd8fb5517831 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Mon, 3 Nov 2025 11:09:55 +0100 Subject: [PATCH] [jOOQ/jOOQ#19320] Diff undeterministically produces cyclic constraint or index rename statements for redundant constraints or indexes --- .../main/java/org/jooq/impl/Comparators.java | 22 ++++---- jOOQ/src/main/java/org/jooq/impl/Diff.java | 50 +++++++++++-------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/Comparators.java b/jOOQ/src/main/java/org/jooq/impl/Comparators.java index b3677c511d..cd95721857 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Comparators.java +++ b/jOOQ/src/main/java/org/jooq/impl/Comparators.java @@ -63,13 +63,17 @@ import org.jooq.Table; */ final class Comparators { - static final Comparator NAMED_COMP = comparing(Named::getQualifiedName); - static final Comparator UNQUALIFIED_COMP = comparing(Named::getUnqualifiedName); - static final Comparator> TABLE_VIEW_COMP = comparing(t -> t.getTableType() == TABLE ? 0 : 1); - static final Comparator> KEY_COMP = new KeyComparator(); - static final Comparator> FOREIGN_KEY_COMP = new ForeignKeyComparator(); - static final Comparator> CHECK_COMP = comparing(c -> c.condition().toString()); - static final Comparator INDEX_COMP = new IndexComparator(); + static final Comparator NAMED_COMP = comparing(Named::getQualifiedName); + static final Comparator UNQUALIFIED_COMP = comparing(Named::getUnqualifiedName); + static final Comparator> TABLE_VIEW_COMP = comparing(t -> t.getTableType() == TABLE ? 0 : 1); + static final Comparator> UNNAMED_KEY_COMP = new KeyComparator(); + static final Comparator> KEY_COMP = UNNAMED_KEY_COMP.thenComparing(NAMED_COMP); + static final Comparator> UNNAMED_FOREIGN_KEY_COMP = new ForeignKeyComparator(); + static final Comparator> FOREIGN_KEY_COMP = UNNAMED_FOREIGN_KEY_COMP.thenComparing(NAMED_COMP); + static final Comparator> UNNAMED_CHECK_COMP = comparing(c -> c.condition().toString()); + static final Comparator> CHECK_COMP = UNNAMED_CHECK_COMP.thenComparing(NAMED_COMP); + static final Comparator UNNAMED_INDEX_COMP = new IndexComparator(); + static final Comparator INDEX_COMP = UNNAMED_INDEX_COMP.thenComparing(NAMED_COMP); private static final class KeyComparator implements Comparator> { @Override @@ -95,12 +99,12 @@ final class Comparators { private static final class ForeignKeyComparator implements Comparator> { @Override public int compare(ForeignKey o1, ForeignKey o2) { - int c = KEY_COMP.compare(o1, o2); + int c = UNNAMED_KEY_COMP.compare(o1, o2); if (c != 0) return c; else - return KEY_COMP.compare(o1.getKey(), o2.getKey()); + return UNNAMED_KEY_COMP.compare(o1.getKey(), o2.getKey()); } } diff --git a/jOOQ/src/main/java/org/jooq/impl/Diff.java b/jOOQ/src/main/java/org/jooq/impl/Diff.java index ec653a758c..7ca11108ae 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Diff.java +++ b/jOOQ/src/main/java/org/jooq/impl/Diff.java @@ -52,6 +52,10 @@ import static org.jooq.impl.Comparators.FOREIGN_KEY_COMP; import static org.jooq.impl.Comparators.INDEX_COMP; import static org.jooq.impl.Comparators.KEY_COMP; import static org.jooq.impl.Comparators.NAMED_COMP; +import static org.jooq.impl.Comparators.UNNAMED_CHECK_COMP; +import static org.jooq.impl.Comparators.UNNAMED_FOREIGN_KEY_COMP; +import static org.jooq.impl.Comparators.UNNAMED_INDEX_COMP; +import static org.jooq.impl.Comparators.UNNAMED_KEY_COMP; import static org.jooq.impl.Comparators.UNQUALIFIED_COMP; import static org.jooq.impl.ConstraintType.CHECK; import static org.jooq.impl.ConstraintType.FOREIGN_KEY; @@ -216,7 +220,7 @@ final class Diff extends AbstractScope { } private final DiffResult appendCatalogs(DiffResult result, List l1, List l2) { - return append(result, l1, l2, null, + return append(result, l1, l2, null, null, // TODO Implement this for SQL Server support. null, @@ -229,7 +233,7 @@ final class Diff extends AbstractScope { } private final DiffResult appendSchemas(DiffResult result, List l1, List l2) { - return append(result, l1, l2, null, + return append(result, l1, l2, null, null, (r, s) -> r.queries.addAll(Arrays.asList(ctx.ddl(s).queries())), (r, s) -> { if (s.getTables().isEmpty() && s.getSequences().isEmpty()) { @@ -295,7 +299,7 @@ final class Diff extends AbstractScope { } private final DiffResult appendSequences(DiffResult result, List> l1, List> l2) { - return append(result, l1, l2, null, + return append(result, l1, l2, null, null, (r, s) -> r.queries.add(ddl.createSequence(s)), dropSequence(), (r, s1, s2) -> { @@ -403,7 +407,7 @@ final class Diff extends AbstractScope { @SuppressWarnings({ "unchecked", "rawtypes" }) private final DiffResult appendDomains(DiffResult result, List> l1, List> l2) { - return append(result, l1, l2, null, + return append(result, l1, l2, null, null, (r, d) -> r.queries.add(ddl.createDomain(d)), (r, d) -> r.queries.add(ctx.dropDomain(d)), (r, d1, d2) -> { @@ -505,7 +509,7 @@ final class Diff extends AbstractScope { }; private final DiffResult appendTables(DiffResult result, List> l1, List> l2) { - return append(result, l1, l2, null, createTable(), dropTable(), MERGE_TABLE); + return append(result, l1, l2, null, null, createTable(), dropTable(), MERGE_TABLE); } private final List> removePrimary(List> list) { @@ -558,7 +562,7 @@ final class Diff extends AbstractScope { final List> add = new ArrayList<>(); final List> drop = new ArrayList<>(); - result = append(result, l1, l2, UNQUALIFIED_COMP, + result = append(result, l1, l2, UNQUALIFIED_COMP, UNQUALIFIED_COMP, (r, f) -> { // Ignore synthetic columns @@ -730,7 +734,7 @@ final class Diff extends AbstractScope { r.queries.add(ctx.alterTable(t1).dropPrimaryKey(pk.constraint())); }; - return append(result, pk1, pk2, KEY_COMP, + return append(result, pk1, pk2, KEY_COMP, UNNAMED_KEY_COMP, create, drop, keyMerge(t1, create, drop, PRIMARY_KEY), @@ -742,7 +746,7 @@ final class Diff extends AbstractScope { final Create> create = (r, u) -> r.queries.add(ctx.alterTable(t1).add(u.constraint())); final Drop> drop = (r, u) -> r.queries.add(ctx.alterTable(t1).dropUnique(u.constraint())); - return append(result, uk1, uk2, KEY_COMP, + return append(result, uk1, uk2, KEY_COMP, UNNAMED_KEY_COMP, create, drop, keyMerge(t1, create, drop, UNIQUE), @@ -839,7 +843,7 @@ final class Diff extends AbstractScope { r.queries.add(ctx.alterTable(t1).dropForeignKey(fk.constraint())); }; - return append(result, fk1, fk2, FOREIGN_KEY_COMP, + return append(result, fk1, fk2, FOREIGN_KEY_COMP, UNNAMED_FOREIGN_KEY_COMP, create, drop, keyMerge(t1, create, drop, FOREIGN_KEY), @@ -851,7 +855,7 @@ final class Diff extends AbstractScope { final Create> create = (r, c) -> r.queries.add(ctx.alterTable(t1).add(c.constraint())); final Drop> drop = (r, c) -> r.queries.add(ctx.alterTable(t1).drop(c.constraint())); - return append(result, c1, c2, CHECK_COMP, + return append(result, c1, c2, CHECK_COMP, UNNAMED_CHECK_COMP, create, drop, keyMerge(t1, create, drop, CHECK), @@ -863,7 +867,7 @@ final class Diff extends AbstractScope { final Create> create = (r, c) -> r.queries.add(ctx.alterDomain(d1).add(c.constraint())); final Drop> drop = (r, c) -> r.queries.add(ctx.alterDomain(d1).dropConstraint(c.constraint())); - return append(result, c1, c2, CHECK_COMP, + return append(result, c1, c2, CHECK_COMP, UNNAMED_CHECK_COMP, create, drop, keyMerge(d1, create, drop), @@ -875,11 +879,11 @@ final class Diff extends AbstractScope { final Create create = (r, i) -> r.queries.add((i.getUnique() ? ctx.createUniqueIndex(i) : ctx.createIndex(i)).on(t1, i.getFields())); final Drop drop = (r, i) -> r.queries.add(ctx.dropIndex(i).on(t1)); - return append(result, l1, l2, INDEX_COMP, + return append(result, l1, l2, INDEX_COMP, UNNAMED_INDEX_COMP, create, drop, (r, ix1, ix2) -> { - if (INDEX_COMP.compare(ix1, ix2) != 0) { + if (UNNAMED_INDEX_COMP.compare(ix1, ix2) != 0) { drop.drop(r, ix1); create.create(r, ix2); } @@ -897,32 +901,36 @@ final class Diff extends AbstractScope { DiffResult result, List l1, List l2, - Comparator comp, + Comparator compForSort, + Comparator compForCompare, Create create, Drop drop, Merge merge ) { - return append(result, l1, l2, comp, create, drop, merge, false); + return append(result, l1, l2, compForSort, compForCompare, create, drop, merge, false); } private final DiffResult append( DiffResult result, List l1, List l2, - Comparator comp, + Comparator compForSort, + Comparator compForCompare, Create create, Drop drop, Merge merge, boolean dropMergeCreate ) { - if (comp == null) - comp = NAMED_COMP; + if (compForSort == null) + compForSort = NAMED_COMP; + if (compForCompare == null) + compForCompare = compForSort; N s1 = null; N s2 = null; - Iterator i1 = sorted(l1, comp); - Iterator i2 = sorted(l2, comp); + Iterator i1 = sorted(l1, compForSort); + Iterator i2 = sorted(l2, compForSort); DiffResult dropped = dropMergeCreate ? new DiffResult(new ArrayList<>(), new ArrayList<>(), result.addedFks, result.droppedFks) : result; DiffResult merged = dropMergeCreate ? new DiffResult(new ArrayList<>(), new ArrayList<>(), result.addedFks, result.droppedFks) : result; @@ -942,7 +950,7 @@ final class Diff extends AbstractScope { ? 1 : s2 == null ? -1 - : comp.compare(s1, s2); + : compForCompare.compare(s1, s2); if (c < 0) { if (drop != null)