Re: altering a column's collation leaves an invalid foreign key

On Thu, Oct 17, 2024 at 7:14 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
>
> So I took the v5 patch you had posted and started working from there.
> The rule that you had picked isn't quite what we want, I think.  It's
> okay to have nondeterministic collations on foreign keys, as long as the
> collation is the same on both sides.  That's what I have implemented.
> See attached.
>
> This approach also allows cleaning up a bunch of hackiness in
> ri_triggers.c, which feels satisfying.
>
>
yech, i missed FK, PK both are nondeterministic but with same collation OID.
your work is more neat.
However FK, PK same nondeterministic collation OID have implications
for ri_KeysEqual.

ri_KeysEqual definitely deserves some comments.
for rel_is_pk, the equality is collation agnostic;
for rel_is_pk is false, the equality is collation aware.

for example:
DROP TABLE IF EXISTS fktable, pktable;
CREATE TABLE pktable (x text COLLATE case_insensitive PRIMARY KEY);
CREATE TABLE fktable (x text collate case_insensitive REFERENCES
pktable on update restrict on delete restrict);
INSERT INTO pktable VALUES ('A'), ('Å');
INSERT INTO fktable VALUES ('a');
update pktable set x = 'a' where x = 'A';
ERROR:  update or delete on table "pktable" violates foreign key
constraint "fktable_x_fkey" on table "fktable"
DETAIL:  Key (x)=(A) is still referenced from table "fktable".
this should not happen?
If so, the below change can solve the problem.

@@ -2930,6 +2915,16 @@ ri_KeysEqual(Relation rel, TupleTableSlot
*oldslot, TupleTableSlot *newslot,
              */
             Form_pg_attribute att =
TupleDescAttr(oldslot->tts_tupleDescriptor, attnums[i] - 1);
+            Oid        collation =  RIAttCollation(rel, attnums[i]);
+            if (OidIsValid(collation) &&
!get_collation_isdeterministic(collation))
+            {
+                Oid            eq_opr;
+                bool        result;
+                eq_opr = riinfo->pp_eq_oprs[i];
+                result = ri_CompareWithCast(eq_opr, RIAttType(rel, attnums[i]),
+                                        collation, newvalue, oldvalue);
+                return result;
+            }
             if (!datum_image_eq(oldvalue, newvalue, att->attbyval,
att->attlen))
                 return false;

The above change will make the ri_KeysEqual equality coalition aware
regardless rel_is_pk's value.
to see the effect, we can test it BEFORE and AFTER applying the above
ri_KeysEqual changes
with the attached sql script.