Thread: minor comments issue in ResultRelInfo src/include/nodes/execnodes.h

hi.
{
    /*
     * For UPDATE/DELETE result relations, the attribute number of the row
     * identity junk attribute in the source plan's output tuples
     */
    AttrNumber    ri_RowIdAttNo;

    /* Projection to generate new tuple in an INSERT/UPDATE */
    ProjectionInfo *ri_projectNew;

    /* arrays of stored generated columns expr states, for INSERT and UPDATE */
    ExprState **ri_GeneratedExprsI;
    ExprState **ri_GeneratedExprsU;
}
for the struct ResultRelInfo, i've checked the above fields.

I think first ri_RowIdAttNo applies to MERGE also. so the comments may
not be correct?
Other files comments are fine.


see:
ExecInitModifyTable
        /*
         * For UPDATE/DELETE/MERGE, find the appropriate junk attr now, either
         * a 'ctid' or 'wholerow' attribute depending on relkind.  For foreign
         * tables, the FDW might have created additional junk attr(s), but
         * those are no concern of ours.
         */
        if (operation == CMD_UPDATE || operation == CMD_DELETE ||
            operation == CMD_MERGE)

On Mon, 12 Aug 2024 at 22:03, jian he <jian.universality@gmail.com> wrote:
>     AttrNumber    ri_RowIdAttNo;
>
>     /* arrays of stored generated columns expr states, for INSERT and UPDATE */
>     ExprState **ri_GeneratedExprsI;
>     ExprState **ri_GeneratedExprsU;
> }
> for the struct ResultRelInfo, i've checked the above fields.
>
> I think first ri_RowIdAttNo applies to MERGE also. so the comments may
> not be correct?

Yeah, ri_RowIdAttNo is used for MERGE. We should fix that comment.

> Other files comments are fine.

I'd say ri_GeneratedExprsI and ri_GeneratedExprsU are also used for
MERGE and the comment for those is also outdated. See:

ExecMergeMatched -> ExecUpdateAct -> ExecUpdatePrepareSlot ->
ExecComputeStoredGenerated(..., CMD_UPDATE)
ExecMergeNotMatched -> ExecInsert -> ExecComputeStoredGenerated(..., CMD_INSERT)

David

On Mon, 12 Aug 2024 at 22:32, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Mon, 12 Aug 2024 at 22:03, jian he <jian.universality@gmail.com> wrote:
> > I think first ri_RowIdAttNo applies to MERGE also. so the comments may
> > not be correct?
>
> Yeah, ri_RowIdAttNo is used for MERGE. We should fix that comment.
>
> > Other files comments are fine.
>
> I'd say ri_GeneratedExprsI and ri_GeneratedExprsU are also used for
> MERGE and the comment for those is also outdated. See:

I've pushed a patch to fix these.  Thanks for the report.

David