[jOOQ/jOOQ#19320] Diff undeterministically produces cyclic constraint or

index rename statements for redundant constraints or indexes
This commit is contained in:
Lukas Eder 2025-11-03 11:09:55 +01:00
parent 22316d1fd9
commit d0835cd665
2 changed files with 42 additions and 30 deletions

View File

@ -63,13 +63,17 @@ import org.jooq.Table;
*/
final class Comparators {
static final Comparator<Named> NAMED_COMP = comparing(Named::getQualifiedName);
static final Comparator<Named> UNQUALIFIED_COMP = comparing(Named::getUnqualifiedName);
static final Comparator<Table<?>> TABLE_VIEW_COMP = comparing(t -> t.getTableType() == TABLE ? 0 : 1);
static final Comparator<Key<?>> KEY_COMP = new KeyComparator();
static final Comparator<ForeignKey<?, ?>> FOREIGN_KEY_COMP = new ForeignKeyComparator();
static final Comparator<Check<?>> CHECK_COMP = comparing(c -> c.condition().toString());
static final Comparator<Index> INDEX_COMP = new IndexComparator();
static final Comparator<Named> NAMED_COMP = comparing(Named::getQualifiedName);
static final Comparator<Named> UNQUALIFIED_COMP = comparing(Named::getUnqualifiedName);
static final Comparator<Table<?>> TABLE_VIEW_COMP = comparing(t -> t.getTableType() == TABLE ? 0 : 1);
static final Comparator<Key<?>> UNNAMED_KEY_COMP = new KeyComparator();
static final Comparator<Key<?>> KEY_COMP = UNNAMED_KEY_COMP.thenComparing(NAMED_COMP);
static final Comparator<ForeignKey<?, ?>> UNNAMED_FOREIGN_KEY_COMP = new ForeignKeyComparator();
static final Comparator<ForeignKey<?, ?>> FOREIGN_KEY_COMP = UNNAMED_FOREIGN_KEY_COMP.thenComparing(NAMED_COMP);
static final Comparator<Check<?>> UNNAMED_CHECK_COMP = comparing(c -> c.condition().toString());
static final Comparator<Check<?>> CHECK_COMP = UNNAMED_CHECK_COMP.thenComparing(NAMED_COMP);
static final Comparator<Index> UNNAMED_INDEX_COMP = new IndexComparator();
static final Comparator<Index> INDEX_COMP = UNNAMED_INDEX_COMP.thenComparing(NAMED_COMP);
private static final class KeyComparator implements Comparator<Key<?>> {
@Override
@ -95,12 +99,12 @@ final class Comparators {
private static final class ForeignKeyComparator implements Comparator<ForeignKey<?, ?>> {
@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());
}
}

View File

@ -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<Catalog> l1, List<Catalog> 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<Schema> l1, List<Schema> 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<? extends Sequence<?>> l1, List<? extends Sequence<?>> 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<? extends Domain<?>> l1, List<? extends Domain<?>> 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<? extends Table<?>> l1, List<? extends Table<?>> 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<UniqueKey<?>> removePrimary(List<? extends UniqueKey<?>> list) {
@ -558,7 +562,7 @@ final class Diff extends AbstractScope {
final List<Field<?>> add = new ArrayList<>();
final List<Field<?>> 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<UniqueKey<?>> create = (r, u) -> r.queries.add(ctx.alterTable(t1).add(u.constraint()));
final Drop<UniqueKey<?>> 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<Check<?>> create = (r, c) -> r.queries.add(ctx.alterTable(t1).add(c.constraint()));
final Drop<Check<?>> 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<Check<?>> create = (r, c) -> r.queries.add(ctx.alterDomain(d1).add(c.constraint()));
final Drop<Check<?>> 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<Index> create = (r, i) -> r.queries.add((i.getUnique() ? ctx.createUniqueIndex(i) : ctx.createIndex(i)).on(t1, i.getFields()));
final Drop<Index> 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<? extends N> l1,
List<? extends N> l2,
Comparator<? super N> comp,
Comparator<? super N> compForSort,
Comparator<? super N> compForCompare,
Create<N> create,
Drop<N> drop,
Merge<N> merge
) {
return append(result, l1, l2, comp, create, drop, merge, false);
return append(result, l1, l2, compForSort, compForCompare, create, drop, merge, false);
}
private final <N extends Named> DiffResult append(
DiffResult result,
List<? extends N> l1,
List<? extends N> l2,
Comparator<? super N> comp,
Comparator<? super N> compForSort,
Comparator<? super N> compForCompare,
Create<N> create,
Drop<N> drop,
Merge<N> 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<? extends N> i1 = sorted(l1, comp);
Iterator<? extends N> i2 = sorted(l2, comp);
Iterator<? extends N> i1 = sorted(l1, compForSort);
Iterator<? extends N> 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)