Re: a misbehavior of partition row movement (?)

On Fri, Mar 18, 2022 at 9:38 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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