Re: Allow foreign keys to reference a superset of unique columns

From: Kaiting Chen <ktchen14(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, hellopfm(at)gmail(dot)com
Subject: Re: Allow foreign keys to reference a superset of unique columns
Date: 2022-09-28 15:14:00
Message-ID: CA+CLzG-mkhPh70BM5FiyH=VFA=4tLN4CAgbgC+_0Pu6L9PSU4Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> For one example of where the semantics get fuzzy, it's not
> very clear how the extra-baggage columns ought to participate in
> CASCADE updates. Currently, if we have
> CREATE TABLE foo (a integer PRIMARY KEY, b integer);
> then an update that changes only foo.b doesn't need to update
> referencing tables, and I think we even have optimizations that
> assume that if no unique-key columns are touched then RI checks
> need not be made.

Regarding optimizations that skip RI checks on the PK side of the
relationship,
I believe the relevant code is here (in trigger.c):

if (TRIGGER_FIRED_BY_UPDATE(event) || TRIGGER_FIRED_BY_DELETE(event)) {
...

switch (RI_FKey_trigger_type(trigger->tgfoid)) {
...

case RI_TRIGGER_PK:
...

/* Update or delete on trigger's PK table */
if (!RI_FKey_pk_upd_check_required(trigger, rel, oldslot,
newslot))
{
/* skip queuing this event */
continue;
}

...

And the checks done in RI_FKey_pk_upd_check_required() are:

const RI_ConstraintInfo *riinfo;

riinfo = ri_FetchConstraintInfo(trigger, pk_rel, true);

/*
* If any old key value is NULL, the row could not have been referenced
by
* an FK row, so no check is needed.
*/
if (ri_NullCheck(RelationGetDescr(pk_rel), oldslot, riinfo, true) !=
RI_KEYS_NONE_NULL)
return false;

/* If all old and new key values are equal, no check is needed */
if (newslot && ri_KeysEqual(pk_rel, oldslot, newslot, riinfo, true))
return false;

/* Else we need to fire the trigger. */
return true;

The columns inspected by both ri_NullCheck() and ri_KeysEqual() are based
on the
riinfo->pk_attnums:

if (rel_is_pk)
attnums = riinfo->pk_attnums;
else
attnums = riinfo->fk_attnums;

The check in ri_NullCheck() is, expectedly, a straightforward NULL check:

for (int i = 0; i < riinfo->nkeys; i++)
{
if (slot_attisnull(slot, attnums[i]))
nonenull = false;
else
allnull = false;
}

The check in ri_KeysEqual() is a bytewise comparison:

/* XXX: could be worthwhile to fetch all necessary attrs at once */
for (int i = 0; i < riinfo->nkeys; i++)
{
Datum oldvalue;
Datum newvalue;
bool isnull;

/*
* Get one attribute's oldvalue. If it is NULL - they're not
equal.
*/
oldvalue = slot_getattr(oldslot, attnums[i], &isnull);
if (isnull)
return false;

/*
* Get one attribute's newvalue. If it is NULL - they're not
equal.
*/
newvalue = slot_getattr(newslot, attnums[i], &isnull);
if (isnull)
return false;

if (rel_is_pk)
{
/*
* If we are looking at the PK table, then do a bytewise
* comparison. We must propagate PK changes if the
value is
* changed to one that "looks" different but would
compare as
* equal using the equality operator. This only makes a
* difference for ON UPDATE CASCADE, but for consistency
we treat
* all changes to the PK the same.
*/
Form_pg_attribute att =
TupleDescAttr(oldslot->tts_tupleDescriptor, attnums[i] - 1);

if (!datum_image_eq(oldvalue, newvalue, att->attbyval,
att->attlen))
return false;
}
else
{
/*
* For the FK table, compare with the appropriate
equality
* operator. Changes that compare equal will still
satisfy the
* constraint after the update.
*/
if (!ri_AttributesEqual(riinfo->ff_eq_oprs[i],
RIAttType(rel, attnums[i]),
oldvalue,
newvalue))
return false;
}
}

It seems like neither optimization actually requires the presence of the
unique
index. And, as my proposed patch would leave both riinfo->nkeys and
riinfo->pk_attnums exactly the same as before, I don't believe that it
should
have any impact on these optimizations.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-09-28 15:16:09 Re: Avoid memory leaks during base backups
Previous Message Zhang Mingli 2022-09-28 15:07:49 Re: Summary function for pg_buffercache