Commit 3093eb2b authored by Tom Lane's avatar Tom Lane

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 cb3a7c2b, so back-patch to all
supported branches.
parent c2457769
...@@ -7926,7 +7926,6 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ...@@ -7926,7 +7926,6 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
new_castfunc == old_castfunc && new_castfunc == old_castfunc &&
(!IsPolymorphicType(pfeqop_right) || (!IsPolymorphicType(pfeqop_right) ||
new_fktype == old_fktype)); new_fktype == old_fktype));
} }
pfeqoperators[i] = pfeqop; pfeqoperators[i] = pfeqop;
...@@ -11478,7 +11477,7 @@ TryReuseForeignKey(Oid oldId, Constraint *con) ...@@ -11478,7 +11477,7 @@ TryReuseForeignKey(Oid oldId, Constraint *con)
/* stash a List of the operator Oids in our Constraint node */ /* stash a List of the operator Oids in our Constraint node */
for (i = 0; i < numkeys; i++) for (i = 0; i < numkeys; i++)
con->old_conpfeqop = lcons_oid(rawarr[i], con->old_conpfeqop); con->old_conpfeqop = lappend_oid(con->old_conpfeqop, rawarr[i]);
ReleaseSysCache(tup); ReleaseSysCache(tup);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment