On Thu, Feb 27, 2020 at 10:22 PM Alvaro Herrera
<alvherre@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