On 2022-Mar-18, Zhihong Yu wrote:
> +#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) ?
They *are* sequential -- See
https://www.postgresql.org/message-id/202201172215.2tse3vjjgi2b%40alvherre.pgsql
> +#define AFTER_TRIGGER_CP_UPDATE 0x08000000
>
> It would be better to add a comment for this constant, explaining what CP
> means (cross partition).
Sure.
> + 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.
I don't think it matters. We don't really expect to hit this.
> + /* Ignore the root ancestor, because ...?? */
>
> Please fill out the remainder of the comment.
I actually would like to know what's the rationale for this myself.
Amit?
> + 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.
Well, the constant is about the trigger *function*, not about any
constraint. This code is testing "is this a noncloned trigger, and does
that trigger use an FK-related function?" If you have a favorite
comment to include, I'm all ears.
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"It takes less than 2 seconds to get to 78% complete; that's a good sign.
A few seconds later it's at 90%, but it seems to have stuck there. Did
somebody make percentages logarithmic while I wasn't looking?"
http://smylers.hates-software.com/2005/09/08/1995c749.html
