Re: a misbehavior of partition row movement (?) - Mailing list pgsql-hackers

Thank you for looking at this.

On Mon, Feb 15, 2021 at 4:12 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Wed, Jan 20, 2021 at 7:04 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > This shows that the way we've made these triggers behave in general
> > can cause some unintended behaviors for foreign keys during
> > cross-partition updates.  I started this thread to do something about
> > that and sent a patch to prevent cross-partition updates at all when
> > there are foreign keys pointing to it.  As others pointed out, that's
> > not a great long-term solution to the problem, but that's what we may
> > have to do in the back-branches if anything at all.
>
> I've started by reviewing the patch for back-patching that the first
> patch you posted[1].
>
> +       for (i = 0; i < trigdesc->numtriggers; i++)
> +       {
> +           Trigger *trigger = &trigdesc->triggers[i];
> +
> +           if (trigger->tgisinternal &&
> +               OidIsValid(trigger->tgconstrrelid) &&
> +               trigger->tgfoid == F_RI_FKEY_CASCADE_DEL)
> +           {
> +               found = true;
> +               break;
> +           }
> +       }
>
> IIUC the above checks if the partition table is referenced by a
> foreign key constraint on another table with ON DELETE CASCADE option.
> I think we should prevent cross-partition update also when ON DELETE
> SET NULL and ON DELETE SET DEFAULT. For example, with the patch, a
> tuple in a partition table is still updated to NULL when
> cross-partition update as follows:
>
> postgres=# create table p (a numeric primary key) partition by list (a);
> CREATE TABLE
> postgres=# create table p1 partition of p for values in (1);
> CREATE TABLE
> postgres=# create table p2 partition of p for values in (2);
> CREATE TABLE
> postgres=# insert into p values (1);
> INSERT 0 1
> postgres=# create table q (a int references p(a) on delete set null);
> CREATE TABLE
> postgres=# insert into q values (1);
> INSERT 0 1
> postgres=# update p set a = 2;
> UPDATE 1
> postgres=# table p;
>  a
> ---
>  2
> (1 row)
>
> postgres=# table q;
>  a
> ---
>
> (1 row)

Yeah, I agree that's not good.

Actually, I had meant to send an updated version of the patch to apply
in back-branches that would prevent such a cross-partition update, but
never did since starting to work on the patch for HEAD.  I have
attached it here.

Regarding the patch, I would have liked if it only prevented the
update when the foreign key that would be violated by the component
delete references the update query's main target relation.  If the
foreign key references the source partition directly, then the delete
would be harmless.  However there's not enough infrastructure in v12,
v13 branches to determine that, which makes back-patching this a bit
hard.  For example, there's no way to tell in the back-branches if the
foreign-key-enforcing triggers of a leaf partition have descended from
the parent table.  IOW, I am not so sure anymore if we should try to
do anything in the back-branches.

-- 
Amit Langote
EDB: http://www.enterprisedb.com