Re: a misbehavior of partition row movement (?)

On Mon, Feb 22, 2021 at 3:04 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Fri, Feb 19, 2021 at 5:04 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > On Mon, Feb 15, 2021 at 10:37 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > 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.
> >
> > Hmm I'm not sure the necessity of figuring out foreign-key-enforcing
> > triggers of a leaf partition have descended from the parent table. I
> > might be missing something but even if the foreign key references the
> > leaf partition directly, the same problem could happen if doing a
> > cross-partition update, is that right?
>
> Actually not, because in that case the referencing relations only care
> about the contents of that leaf partition, so it's okay that the ON
> DELETE actions are performed in that case, or IOW, no need to abort
> the update. Contrast that with when the foreign key references the
> parent table being updated, where both the old and the new row
> logically belong to the table being referenced, so performing ON
> DELETE actions using the source leaf partition's trigger is wrong.
>
> Does that make sense?

That makes sense. Thanks for your explanation.

One more thing, users are aware of a cross-partition update is
internally converted to DELETE + INSERT (i.g., documented somewhere)?
If not, users might get confused if a tuple referencing a partition
table through a foreign key constraint with ON DELETE CASCADE is
deleted when UPDATE on the parent table.

>
> > Also, the updated patch prevents a cross-partition update even when
> > the foreign key references another column of itself. This kind of
> > cross-update works as expected for now so it seems not to need to
> > disallow that. What do you think? The scenario is:
> >
> > create table p (a int primary key, b int references p(a) on delete
> > cascade) partition by list (a);
> > create table p1 partition of p for values in (1);
> > create table p2 partition of p for values in (2);
> > insert into p values (1, 1);
> > update p set a = 2, b = 2 where a = 1;
> > ERROR: cannot move row being updated to another partition
> > DETAIL: Moving the row may cause a foreign key involving the source
> > partition to be violated.
>
> Hmm yes, it would be nice to not cause an error in this case. But we
> don't have enough details about the actual foreign key in this part of
> the code (ExecUpdate). Given the current ResultRelInfo
> infrastructure, this code can only look at the trigger objects to get
> some details about the foreign key, such as whether the relation being
> operated on is the FK relation or the PK relation. More detailed
> properties of those foreign keys are only checked inside the trigger's
> functions (ri_trigger.c) and perhaps also in the dispatch code in
> trigger.c: AfterTriggerSaveEvent(). However, it would be hard to
> detect in those modules that a delete trigger event indeed comes from
> a delete performed as part of a cross-partition update, that is,
> without significantly changing their interfaces.

Agreed.

>
> Anyway, I am no longer sure if we should do anything in the back
> branches, which the patch you have been looking at is for. There have
> not been many field complaints about this other than the one that
> started this thread. Also, I suspect that aborting the
> cross-partition updates for any partitioned table that is referenced
> in a foreign key will annoy users, especially those who simply don't
> use ON DELETE/UPDATE clauses.

I thought to disallow creating foreign key constraint referencing a
partitioned table with ON DELETE/UPDATE actions other than NO ACTION
and RESTRICT but it also seems to annoy users.

>
> So, it may be better to focus on the patches for master that, instead
> of giving an error, make the cross-partition updates just work.

Okay, let's focus on the patches for master. I'll look at that patch next week.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/