Re: MERGE ... RETURNING

On Sat, Nov 18, 2023 at 8:55 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> The v13 patch still applies on top of this, so I won't re-post it.
>

Hi.
minor issues based on v13.

+<synopsis>
+<function id="function-merging">MERGING</function> (
<replaceable>property</replaceable> )
+</synopsis>
+ The following are valid property values specifying what to return:
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>ACTION</literal></term>
+ <listitem>
+ <para>
+ The merge action command executed for the current row
+ (<literal>'INSERT'</literal>, <literal>'UPDATE'</literal>, or
+ <literal>'DELETE'</literal>).
+ </para>
+ </listitem>
+ </varlistentry>
do we change to <literal>property</literal>?
Maybe the main para should be two sentences like:
The merge action command executed for the current row. Possible values
are: <literal>'INSERT'</literal>, <literal>'UPDATE'</literal>,
<literal>'DELETE'</literal>.

static Node *
+transformMergingFunc(ParseState *pstate, MergingFunc *f)
+{
+ /*
+ * Check that we're in the RETURNING list of a MERGE command.
+ */
+ if (pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
+ {
+ ParseState *parent_pstate = pstate->parentParseState;
+
+ while (parent_pstate &&
+ parent_pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
+ parent_pstate = parent_pstate->parentParseState;
+
+ if (!parent_pstate ||
+ parent_pstate->p_expr_kind != EXPR_KIND_MERGE_RETURNING)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("MERGING() can only be used in the RETURNING list of a MERGE command"),
+ parser_errposition(pstate, f->location));
+ }
+
+ return (Node *) f;
+}
+
the object is correct, but not in the right place.
maybe we should change errcode(ERRCODE_WRONG_OBJECT_TYPE) to
errcode(ERRCODE_INVALID_OBJECT_DEFINITION)
also do we need to add some comments explain that why we return it as
is when it's EXPR_KIND_MERGE_RETURNING.
(my understanding is that, if key words not match, then it will fail
at gram.y, like syntax error, else MERGING will based on keywords make
a MergingFunc node and assign mfop, mftype, location to it)

in src/backend/executor/functions.c
/*
* Break from loop if we didn't shut down (implying we got a
* lazily-evaluated row). Otherwise we'll press on till the whole
* function is done, relying on the tuplestore to keep hold of the
* data to eventually be returned. This is necessary since an
* INSERT/UPDATE/DELETE RETURNING that sets the result might be
* followed by additional rule-inserted commands, and we want to
* finish doing all those commands before we return anything.
*/
Does the above comments need to change to INSERT/UPDATE/DELETE/MERGE?

in src/backend/nodes/nodeFuncs.c
case T_UpdateStmt:
{
UpdateStmt *stmt = (UpdateStmt *) node;
if (WALK(stmt->relation))
return true;
if (WALK(stmt->targetList))
return true;
if (WALK(stmt->whereClause))
return true;
if (WALK(stmt->fromClause))
return true;
if (WALK(stmt->returningList))
return true;
if (WALK(stmt->withClause))
return true;
}
break;
case T_MergeStmt:
{
MergeStmt *stmt = (MergeStmt *) node;
if (WALK(stmt->relation))
return true;
if (WALK(stmt->sourceRelation))
return true;
if (WALK(stmt->joinCondition))
return true;
if (WALK(stmt->mergeWhenClauses))
return true;
if (WALK(stmt->withClause))
return true;
}
break;

you add "returningList" to MergeStmt.
do you need to do the following similar to UpdateStmt, even though
it's so abstract, i have no idea what's going on.
`
if (WALK(stmt->returningList))
return true;
`