Re: MERGE ... RETURNING - Mailing list pgsql-hackers

On Thu, 14 Mar 2024 at 05:30, jian he <jian.universality@gmail.com> wrote:
>
> Hi
> mainly document issues. Other than that, it looks good!

Thanks for the review.


> MERGE not supported in COPY
> MERGE not supported in WITH query
> These entries in src/backend/po.* need to be deleted if this patch is committed?

No, translation updates are handled separately.


>   <indexterm zone="dml-returning">
>    <primary>MERGE</primary>
>    <secondary>RETURNING</secondary>
>   </indexterm>
>
> in doc/src/sgml/dml.sgml, what is the point of these?
> It is not rendered in the html file, deleting it still generates all the html file.

These generate entries in the index -- see
https://www.postgresql.org/docs/current/bookindex.html


> The following part is about doc/src/sgml/plpgsql.sgml.
>
>     <para>
>      The <replaceable>query</replaceable> used in this type of <literal>FOR</literal>
>      statement can be any SQL command that returns rows to the caller:
>      <command>SELECT</command> is the most common case,
>      but you can also use <command>INSERT</command>, <command>UPDATE</command>, or
>      <command>DELETE</command> with a <literal>RETURNING</literal> clause.  Some utility
>      commands such as <command>EXPLAIN</command> will work too.
>     </para>
> here we need to add <command>MERGE</command>?

Ah yes. I'm not sure how I missed that one.


>    <para>
>     Row-level triggers fired <literal>BEFORE</literal> can return null to signal the
>     trigger manager to skip the rest of the operation for this row
>     (i.e., subsequent triggers are not fired, and the
>     <command>INSERT</command>/<command>UPDATE</command>/<command>DELETE</command> does not occur
>     for this row).  If a nonnull
> here we need to add <command>MERGE</command>?

No, because there are no MERGE triggers. I suppose it could be updated
to mention that this also applies to INSERT, UPDATE, and DELETE
actions in a MERGE, but I'm not sure it's really necessary. In any
case, that's not something changed in this patch, so if we want to do
this, it should be a separate doc patch.


>    <para>
>     Variable substitution currently works only in <command>SELECT</command>,
>     <command>INSERT</command>, <command>UPDATE</command>,
>     <command>DELETE</command>, and commands containing one of
>     these (such as <command>EXPLAIN</command> and <command>CREATE TABLE
>     ... AS SELECT</command>),
>     because the main SQL engine allows query parameters only in these
>     commands.  To use a non-constant name or value in other statement
>     types (generically called utility statements), you must construct
>     the utility statement as a string and <command>EXECUTE</command> it.
>    </para>
> here we need to add <command>MERGE</command>?

Yes, I suppose so (though arguably it falls into the category of
"commands containing" one of INSERT, UPDATE or DELETE). As above, this
isn't something changed by this patch, so it should be done
separately.


>     <literal>INSTEAD OF</literal> triggers (which are always row-level triggers,
>     and may only be used on views) can return null to signal that they did
>     not perform any updates, and that the rest of the operation for this
>     row should be skipped (i.e., subsequent triggers are not fired, and the
>     row is not counted in the rows-affected status for the surrounding
>     <command>INSERT</command>/<command>UPDATE</command>/<command>DELETE</command>).
> I am not sure we need to add <command>MERGE</command >. Maybe not.

Ditto.

Updated patch attached.

Regards,
Dean