Re: [HACKERS] MERGE SQL Statement for PG11

On Tue, Mar 27, 2018 at 1:54 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:

> On 26 March 2018 at 17:06, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> > On 26 March 2018 at 15:39, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
> wrote:
> >
>
> > That's all I can see so far.
>
> * change comment “once to” to “once” in src/include/nodes/execnodes.h
> * change comment “and to run” to “and once to run”
> * change “result relation” to “target relation”
>

Fixed all of that in the patch v26 set I just sent.

>
> * XXX we probably need to check plan output for CMD_MERGE also
>

Yeah. Added those checks for MERGE action's target lists in v26.

>
> * Spurious line feed in src/backend/optimizer/prep/preptlist.c
>

Couldn't spot it. Will look closer, but any hint will be appreciated.

>
> * No need to remove whitespace in src/backend/optimizer/util/relnode.c
>

Fixed in v26.

>
> * README should note that the TABLEOID junk column is not strictly
> needed when joining to a non-partitioned table but we don't try to
> optimize that away. Is that an XXX to fix in future or do we just
> think the extra 4 bytes won't make much difference so we leave it?
>

I actually took the opportunity to conditionally fetch tableoid only if we
are dealing with partitioned table.

>
> * Comment in rewriteTargetListMerge() should mention TABLEOID exists
> to allow us to find the correct relation, not the correct row, comment
> just copied from CTID above it.
>
>
Fixed in v26.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services