Re: MERGE SQL statement for PG12

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: MERGE SQL statement for PG12
Date: 2019-01-14 17:14:03
Message-ID: CANP8+j+qCE8=jnNduEaeasVS3X5s7OEVTuxNi1LW=3DWbHpg0w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 14 Jan 2019 at 15:45, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:

>
>
> On 1/11/19 12:26 AM, Peter Geoghegan wrote:
> > On Thu, Jan 10, 2019 at 1:15 PM Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> >> I feel like there has been some other thread where this was discussed,
> >> but I can't find it right now. I think that the "query construction"
> >> logic in transformMergeStmt is fundamentally the wrong way to do this.
> >> I think several others have said the same. And I don't think this
> >> should be considered for commit until that is rewritten.
> >
> > I agree with that.
> >
> > I think that it's worth acknowledging that Pavan is in a difficult
> > position with this patch. As things stand, Pavan really needs input
> > from a couple of people to put the query construction stuff on the
> > right path, and that input has not been forthcoming. I'm not
> > suggesting that anybody has failed to meet an obligation to Pavan to
> > put time in here, or that anybody has suggested that this is down to a
> > failure on Pavan's part. I'm merely pointing out that Pavan is in an
> > unenviable position with this patch, through no fault of his own, and
> > despite a worthy effort.
> >
>
> I 100% agree with this. I guess this is the phase where you have a patch
> that generally does the thing you want it to do, but others are saying
> the approach is not the right one. But you're under the spell of your
> approach and can't really see why would it be wrong ...
>

There has been *no* discussion on this point, just assertions that it is
wrong. I have no clue how 3 people can all agree something is wrong without
any discussion and nobody even questions it. So I think there is a spell
here, either Darkness or Cone of Silence.

Where is the further detail? Why is it wrong? What bad effects happen if
you do it this way? What is a better way? Is there even a different way to
do it? Nothing has been said, at all, ever.

The mechanism is exactly the one that Heikki recommended for the MERGE
patch some years ago, so he obviously thought it would work. That was not
my invention, nor Pavan's. We already use SQL assembly in multiple other
places without problem, namely Mat Views and RI Triggers; the approach used
here is better than that and doesn't rely on string handling at all.

Most importantly, it works.

BUT if there really is something wrong, Pavan would love to know and is
willing to make any changes suggested. Review requested.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-01-14 18:23:31 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message Tom Lane 2019-01-14 17:04:31 Re: Proving IS NOT NULL inference for ScalarArrayOpExpr's