From: | Paul Martinez <hellopfm(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Cc: | Zhihong Yu <zyu(at)yugabyte(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Partial foreign key updates in referential integrity triggers |
Date: | 2021-11-23 04:03:48 |
Message-ID: | CAF+2_SGo161i9UonU5D=ppjWMmBQf24S0esaXZXd=K-rMBKCiA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 11, 2021 at 4:34 AM Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> 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.
Thank you! I really, really appreciate the thorough review and the
comments and corrections.
> 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.
Done.
> - 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.
Done.
> - 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.
Got it. I've reverted the changes in that file.
> - 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.
I've reversed the order, so it's now RI_PLAN_<action>_<trigger>, and
renamed the RESTRICT one to just RI_PLAN_RESTRICT.
I've attached an updated patch, including your changes and the additional
changes mentioned above.
- Paul
Attachment | Content-Type | Size |
---|---|---|
referential-actions-on-delete-only-set-cols-v4.patch | application/octet-stream | 45.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2021-11-23 04:22:25 | Re: Sequence's value can be rollback after a crashed recovery. |
Previous Message | Tomas Vondra | 2021-11-23 04:00:34 | Re: Sequence's value can be rollback after a crashed recovery. |