Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e0d13dc

Browse files
committed
Fix thinko in construction of old_conpfeqop list.
This should lappend the OIDs, not lcons them; the existing code produced a list in reversed order. This is harmless for single-key FKs or FKs where all the key columns are of the same type, which probably explains how it went unnoticed. But if those conditions are not met, ATAddForeignKeyConstraint would make the wrong decision about whether an existing FK needs to be revalidated. I think it would almost always err in the safe direction by revalidating a constraint that didn't need it. You could imagine scenarios where the pfeqop check was fooled by swapping the types of two FK columns in one ALTER TABLE, but that case would probably be rejected by other tests, so it might be impossible to get to the worst-case scenario where an FK should be revalidated and isn't. (And even then, it's likely to be fine, unless there are weird inconsistencies in the equality behavior of the replacement types.) However, this is a performance bug at least. Noted while poking around to see whether lcons calls could be converted to lappend. This bug is old, dating to commit cb3a7c2, so back-patch to all supported branches.
1 parent 178ca6b commit e0d13dc

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

src/backend/commands/tablecmds.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7927,7 +7927,6 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
79277927
new_castfunc == old_castfunc &&
79287928
(!IsPolymorphicType(pfeqop_right) ||
79297929
new_fktype == old_fktype));
7930-
79317930
}
79327931

79337932
pfeqoperators[i] = pfeqop;
@@ -11479,7 +11478,7 @@ TryReuseForeignKey(Oid oldId, Constraint *con)
1147911478

1148011479
/* stash a List of the operator Oids in our Constraint node */
1148111480
for (i = 0; i < numkeys; i++)
11482-
con->old_conpfeqop = lcons_oid(rawarr[i], con->old_conpfeqop);
11481+
con->old_conpfeqop = lappend_oid(con->old_conpfeqop, rawarr[i]);
1148311482

1148411483
ReleaseSysCache(tup);
1148511484
}

0 commit comments

Comments
 (0)