Re: [PATCH] Partial foreign key updates in referential integrity triggers

On Wed, Jul 14, 2021 at 6:51 AM Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
> I think this is an interesting feature with a legitimate use case.

Great! I'm glad to hear that.

> Consider what should happen when you update users.id. Per SQL standard,
> for MATCH SIMPLE an ON UPDATE SET NULL should only set to null the
> referencing column that corresponds to the referenced column actually
> updated, not all of them. PostgreSQL doesn't do this, but if it did,
> then this would work just fine.
>
> ...
>
> So, unless I'm missing an angle here, I would suggest leaving out the ON
> UPDATE variant of this feature.

I understand what you're saying here, but are you sure that is the
behavior specified in the SQL standard?

I can't find a copy of a more recent standard, but at least in SQL 1999 [1],
it has this to say (page 459 of the linked PDF, page 433 of the standard):

> 8) If a non-null value of a referenced column in the referenced table is
> updated to a value that is distinct from the current value of that
> column, then for every member F of the subtable family of the
> referencing table:
>
> Case:
> a) If SIMPLE is specified or implicit, or if FULL is specified, then
>
> Case:
> ii) If the <update rule> specifies SET NULL, then
>
> Case:
> 1) If SIMPLE is specified or implicit, then:
>
> A) <snipped stuff about triggers>
>
> B) For every F, in every matching row in F, each referencing
> column in F that corresponds with a referenced column is set to
> the null value.

This phrasing doesn't seem to make any reference to which columns were
actually updated.

The definitions a few pages earlier do explicitly define the set of
updated columns ("Let UMC be the set of referencing columns that
correspond with updated referenced columns"), but that defined set is
only referenced in the behavior when PARTIAL is specified, implying that
the set of updated columns is _not_ relevant in the SIMPLE case.

If my understanding of this is correct, then Postgres _isn't_ out of spec,
and this extension still works fine for the ON UPDATE triggers. (But, wow,
this spec is not easy to read, so I could definitely be wrong.)

I'm not sure how MATCH PARTIAL fits into this; I know it's
unimplemented, but I don't know what its use cases are, and I don't
think I understand how ON DELETE / ON UPDATE should work with MATCH
PARTIAL, let alone how they would work combined with this extension.

This lack of clarity may be a good-enough reason to leave out the ON
UPDATE case.

On Wed, Jul 14, 2021 at 12:36 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:
> Patch does not apply on head, I am marking the status "Waiting on author"
> http://cfbot.cputube.org/patch_33_2932.log

I've rebased my original patch and it should work now. This is
referential-actions-set-cols-v2.patch.

I've also created a second patch that leaves out the ON UPDATE behavior, if
we think that's the safer route. This is
referential-actions-on-delete-only-set-cols-v1.patch.

[1]: http://web.cecs.pdx.edu/~len/sql1999.pdf

On Wed, Jul 14, 2021 at 12:36 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:
>
>
>
> On Wed, Jul 14, 2021 at 6:51 PM Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>>
>>
>> On 05.01.21 22:40, Paul Martinez wrote:
>> > I've created a patch to better support referential integrity constraints when
>> > using composite primary and foreign keys. This patch allows creating a foreign
>> > key using the syntax:
>> >
>> > FOREIGN KEY (tenant_id, fk_id) REFERENCES fktable ON DELETE SET NULL (fk_id)
>> >
>> > which means that only the fk_id column will be set to NULL when the referenced
>> > row is deleted, rather than both the tenant_id and fk_id columns.
>>
>> I think this is an interesting feature with a legitimate use case.
>>
>> I'm wondering a bit about what the ON UPDATE side of this is supposed to
>> mean. Consider your example:
>>
>> > CREATE TABLE tenants (id serial PRIMARY KEY);
>> > CREATE TABLE users (
>> > tenant_id int REFERENCES tenants ON DELETE CASCADE,
>> > id serial,
>> > PRIMARY KEY (tenant_id, id),
>> > );
>> > CREATE TABLE posts (
>> > tenant_id int REFERENCES tenants ON DELETE CASCADE,
>> > id serial,
>> > author_id int,
>> > PRIMARY KEY (tenant_id, id),
>> > FOREIGN KEY (tenant_id, author_id) REFERENCES users ON DELETE SET NULL
>> > );
>> >
>> > INSERT INTO tenants VALUES (1);
>> > INSERT INTO users VALUES (1, 101);
>> > INSERT INTO posts VALUES (1, 201, 101);
>> > DELETE FROM users WHERE id = 101;
>> > ERROR: null value in column "tenant_id" violates not-null constraint
>> > DETAIL: Failing row contains (null, 201, null).
>>
>> Consider what should happen when you update users.id. Per SQL standard,
>> for MATCH SIMPLE an ON UPDATE SET NULL should only set to null the
>> referencing column that corresponds to the referenced column actually
>> updated, not all of them. PostgreSQL doesn't do this, but if it did,
>> then this would work just fine.
>>
>> Your feature requires specifying a fixed column or columns to update, so
>> it cannot react differently to what column actually updated. In fact,
>> you might want different referential actions depending on what columns
>> are updated, like what you can do with general triggers.
>>
>> So, unless I'm missing an angle here, I would suggest leaving out the ON
>> UPDATE variant of this feature.
>>
>>
>
> Patch does not apply on head, I am marking the status "Waiting on author"
> http://cfbot.cputube.org/patch_33_2932.log
>
> --
> Ibrar Ahmed