On Mon, Feb 22, 2021 at 3:04 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Fri, Feb 19, 2021 at 5:04 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Mon, Feb 15, 2021 at 10:37 PM Amit Langote <amitlangot...@gmail.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/
