Re: a misbehavior of partition row movement (?) - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: a misbehavior of partition row movement (?)
Date
Msg-id CALNJ-vTN0P1eb6XLYj3gno1kokL_R+TkFK-GgxPmzJKN_SWDiQ@mail.gmail.com
Whole thread Raw
In response to Re: a misbehavior of partition row movement (?)  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: a misbehavior of partition row movement (?)
List pgsql-hackers


On Fri, Mar 18, 2022 at 9:38 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
I rebased this patch; v15 attached.  Other than fixing the (very large)
conflicts due to nodeModifyTable.c rework, the most important change is
moving GetAncestorResultRels into execMain.c and renaming it to have the
"Exec-" prefix.  The reason is that what this code is doing is affect
struct ResultRelInfo, which is owned by execMain.c, so it seemed bogus
to do that in nodeModifyTable.c and then let execMain.c's
ExecCloseResultRelations() do the cleanup.  I added a little commentary
in the latter routine too.

--
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/
"World domination is proceeding according to plan"        (Andrew Morton)

Hi,

+#define AFTER_TRIGGER_OFFSET           0x07FFFFFF  /* must be low-order bits */
+#define AFTER_TRIGGER_DONE             0x80000000
+#define AFTER_TRIGGER_IN_PROGRESS      0x40000000

Is it better if the order of AFTER_TRIGGER_DONE and AFTER_TRIGGER_IN_PROGRESS is swapped (for the ordinal values to be sequential) ? 

+#define AFTER_TRIGGER_CP_UPDATE            0x08000000

It would be better to add a comment for this constant, explaining what CP means (cross partition).

+   if (!partRel->rd_rel->relispartition)
+       elog(ERROR, "cannot find ancestors of a non-partition result relation");

It would be better to include the relation name in the error message.

+       /* Ignore the root ancestor, because ...?? */

Please fill out the remainder of the comment.

+               if (!trig->tgisclone &&
+                   RI_FKey_trigger_type(trig->tgfoid) == RI_TRIGGER_PK)
+               {
+                   has_noncloned_fkey = true;

The variable says fkey, but the constant is not RI_TRIGGER_FK. Maybe add a comment explaining why.

Cheers

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Remove INT64_FORMAT in translatable strings
Next
From: Zheng Li
Date:
Subject: Re: Support logical replication of DDLs