On Thu, Feb 27, 2020 at 10:22 PM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > > * The "root" is not necessarily the root partitioned table, but instead > it's the table that was named in the command. Because of this, we don't > need to acquire locks on the tables, since the executor already has the > tables open and locked (thus they cannot be modified by concurrent > commands).
I believe this is because of the partition level constraints on the table that was named in the command would catch any violation in the partition key change in the levels above that table. Will it be easier to subject the new tuple to the partition level constraints themselves and report if those are violated. See RelationGetPartitionQual() for getting partition constraints. This function includes partition constraints from all the levels so in your function you don't have to walk up the partition tree. It includes constraints from the level above the table that was named in the command, but that might be fine. We will catch the error earlier and may be provide a better error message. > > * The new function I added, ReportTriggerPartkeyChange(), contains one > serious bug (namely: it doesn't map attribute numbers properly if > partitions are differently defined). IIUC the code in your patch, it seems you are just looking at partnatts. But partition key can contain expressions also which are stored in partexprs. So, I think the code won't catch change in the partition key values when it contains expression. Using RelationGetPartitionQual() will fix this problem and also problem of attribute remapping across the partition hierarchy. -- Best Wishes, Ashutosh Bapat