On Thu, Jul 6, 2023 at 1:13 PM Gurjeet Singh <gurjeet@singh.im> wrote:
>
> On Sat, Jul 1, 2023 at 4:08 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> >
> > On Mon, 13 Mar 2023 at 13:36, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > >
> > > And another rebase.
> > >
> >
> > I ran out of cycles to pursue the MERGE patches in v16, but hopefully
> > I can make more progress in v17.
> >
> > Looking at this one with fresh eyes, it looks mostly in good shape.
>
> +1
>
> Most of the review was done with the v6 of the patch, minus the doc
> build step. The additional changes in v7 of the patch were eyeballed,
> and tested with `make check`.
>
> > To
> > recap, this adds support for the RETURNING clause in MERGE, together
> > with new support functions pg_merge_action() and
> > pg_merge_when_clause() that can be used in the RETURNING list of MERGE
>
> nit: s/can be used in/can be used only in/
>
> > to retrieve the kind of action (INSERT/UPDATE/DELETE), and the index
> > of the WHEN clause executed for each row merged. In addition,
> > RETURNING support allows MERGE to be used as the source query in COPY
> > TO and WITH queries.
> >
> > One minor annoyance is that, due to the way that pg_merge_action() and
> > pg_merge_when_clause() require access to the MergeActionState, they
> > only work if they appear directly in the RETURNING list. They can't,
> > for example, appear in a subquery in the RETURNING list, and I don't
> > see an easy way round that limitation.
>
> I believe that's a serious limitation, and would be a blocker for the feature.
>
> > Attached is an updated patch with some cosmetic updates, plus updated
> > ruleutils support.
>
> With each iteration of your patch, it is becoming increasingly clear
> that this will be a documentation-heavy patch :-)
>
> I think the name of function pg_merge_when_clause() can be improved.
> How about pg_merge_when_clause_ordinal().
>
> I think the name of function pg_merge_when_clause() can be improved.
> How about pg_merge_when_clause_ordinal().
another idea: pg_merge_action_ordinal()