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

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Paul Martinez <hellopfm(at)gmail(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Partial foreign key updates in referential integrity triggers
Date: 2021-11-11 10:34:10
Message-ID: 99ed102c-727b-73fd-6a40-7432953615b7@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I have reviewed your patch
referential-actions-on-delete-only-set-cols-v3.patch. Attached are two
patches that go on top of yours that contain my recommended changes.

Basically, this all looks pretty good to me. My changes are mostly
stylistic.

Some notes of substance:

- The omission of the referential actions details from the CREATE TABLE
reference page surprised me. I have committed that separately (without
the column support, of course). So you should rebase your patch on top
of that. Note that ALTER TABLE would now also need to be updated by
your patch.

- I recommend setting pg_constraint.confdelsetcols to null for the
default behavior of setting all columns, instead of an empty array the
way you have done it. I have noted the places in the code that need to
be changed for that.

- The outfuncs.c support shouldn't be included in the final patch.
There is nothing wrong it, but I don't think it should be part of this
patch to add piecemeal support like that. I have included a few changes
there anyway for completeness.

- In gram.y, I moved the error check around to avoid duplication.

- In ri_triggers.c, I follow your renaming of the constants, but
RI_PLAN_ONTRIGGER_RESTRICT seems a little weird. Maybe do _ONBOTH_, or
else reverse the order, like RI_PLAN_SETNULL_ONDELETE, which would then
allow RI_PLAN_RESTRICT.

Please look through this and provide an updated patch, and then it
should be good to go.

Attachment Content-Type Size
0001-Fix-whitespace.patch text/plain 5.1 KB
0002-Review-changes.patch text/plain 18.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sergei Kornilov 2021-11-11 11:00:56 unexpected plan with id = any('{}') condition
Previous Message Daniel Gustafsson 2021-11-11 09:56:55 Re: On login trigger: take three