On Thu, Nov 11, 2021 at 4:34 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.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