Hi Rahila, Thanks for the review.
On Thu, Feb 18, 2021 at 7:08 PM Rahila Syed <rahilasye...@gmail.com> wrote: >> Here is an updated version of the patch with some cosmetic changes >> from the previous version. I moved the code being added to >> AfterTriggerSaveEvent() and ExecUpdate() into separate subroutines to >> improve readability, hopefully. >> > I tested these patches. > > It works as expected in case of cross partition updates, by correctly > updating the > referencing table. It works fine for ON UPDATE SET NULL and SET DEFAULT > options as well. > Also, tested by having a table reference only a partition and not the parent. > In this case, the delete > trigger is correctly called when the row is moved out of referenced partition. I assume these are comments for the v3-0001 & v3-0002 patches... > The partition-key-update-1.spec test fails with the following error message > appearing in the diffs. > > step s1u3pc: UPDATE foo_range_parted SET a=11 WHERE a=7; > +ERROR: cannot move row being updated to another partition ...whereas, this error happens with the patch I posted in my last email (prevent-row-movement-on-pk-table.patch) that is not meant to be considered for HEAD, but for back-branches (if at all). I also see that cfbot got triggered by it and shows the same failure. I am not going to try to take care of these failures unless we want to do something in the back-branches. To be clear, patches for HEAD do pass make check-world. > I think the documentation update is missing from the patches. Hmm, I don't think we document the behavior that is improved by the v3 patches as a limitation of any existing feature, neither of foreign keys referencing partitioned tables nor of the update row movement feature. So maybe there's nothing in the existing documentation that is to be updated. However, the patch does add a new error message for a case that the patch doesn't handle, so maybe we could document that as a limitation. Not sure if in the Notes section of the UPDATE reference page which has some notes on row movement or somewhere else. Do you have suggestions? Attaching rebased version of the patches for HEAD to appease the cfbot. -- Amit Langote EDB: http://www.enterprisedb.com
v4-0001-Create-foreign-key-triggers-in-partitioned-tables.patch
Description: Binary data
v4-0002-Enforce-foreign-key-correctly-during-cross-partit.patch
Description: Binary data