On Fri, Mar 18, 2022 at 9:38 AM Alvaro Herrera <alvhe...@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
