Re: [HACKERS] MERGE SQL Statement for PG11

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Andres Freund <andres(at)anarazel(dot)de>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] MERGE SQL Statement for PG11
Date: 2018-04-06 07:38:43
Message-ID: 33e09a3f-753a-3377-8a6d-574375b0f4c6@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018/04/06 5:00, Andres Freund wrote:
> On 2018-04-05 11:31:48 +0530, Pavan Deolasee wrote:
>>> + /*
>>> + * If there are not WHEN MATCHED actions, we are done.
>>> + */
>>> + if (mergeMatchedActionStates == NIL)
>>> + return true;
>>>
>>> Maybe I'm confused, but why is mergeMatchedActionStates attached to
>>> per-partition info? How can this differ in number between partitions,
>>> requiring us to re-check it below fetching the partition info above?
>>>
>>>
>> Because each partition may have a columns in different order, dropped
>> attributes etc. So we need to give treatment to the quals/targetlists. See
>> ON CONFLICT DO UPDATE for similar code.
>
> But the count wouldn't change, no? So we return before building the
> partition info if there's no MATCHED action?

Yeah, I think we should return at the top if there are no matched actions.

With the current code, even if there aren't any matched actions we're
doing ExecFindPartitionByOid() and ExecInitPartitionInfo() to only then
see in the partition's resultRelInfo that the action states list is empty.

I think there should be an Assert where there currently is the check for
empty action states list and the check itself should be at the top of the
function, as done in the attached.

Thanks,
Amit

Attachment Content-Type Size
ExecMergeMatched-thinko.patch text/plain 963 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2018-04-06 07:49:20 Re: [HACKERS] Restrict concurrent update/delete with UPDATE of partition key
Previous Message amul sul 2018-04-06 07:37:25 Re: [HACKERS] Restrict concurrent update/delete with UPDATE of partition key