Re: [HACKERS] MERGE SQL Statement for PG11

From: Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, 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-03-26 14:39:38
Message-ID: CABOikdM9hvMCt_nsunLpk1KhQQVwW+-cGH7BfMoh4HqMdbbn-g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 23, 2018 at 4:37 PM, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
wrote:

>
>
> On Fri, Mar 23, 2018 at 12:57 PM, Amit Langote <
> Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
>>
>> Also, it seems that the delta patch I sent in the last email didn't
>> contain all the changes I had to make. It didn't contain, for example,
>> replacing adjust_and_expand_inherited_tlist() with
>> adjust_partition_tlist(). I guess you'll know when you rebase anyway.
>>
>
> Yes, I am planning to fix that once the ON CONFLICT patch is
> ready/committed.
>
>
Now that ON CONFLICT patch is in, here are rebased patches. The second
patch is to add support for CTE (thanks Peter).

Apart from rebase, the following things are fixed/improved:

- Added test cases for column level privileges as suggested by Peter. One
problem got discovered during the process. Since we expand and track source
relation targetlist, the exiting code was demanding SELECT privileges on
all attributes, even though MERGE is only referencing a few attributes on
which the user has privilege. Fixed that by disassociating expansion from
the actual referencing.

- Added a test case for RLS where SELECT policy actually hides some rows,
as suggested by Stephen in the past

- Added check to compare result relation's and merge target relation's
OIDs, as suggested by Robert. Simon thinks it's not necessary given that we
now scan catalog using MVCC snapshot. So will leave it to his discretion
when he takes it up for commit

- Improved explanation regarding why we need a second RTE for merge target
relation and general cleanup/improvements in that area

I think it will be a good idea to send any further patches as add-on
patches for reviewer/committer's sake. I will do that unless someone
disagrees.

Thanks,
Pavan

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

Attachment Content-Type Size
v25-0002-Add-support-for-CTE.patch application/octet-stream 13.5 KB
v25-0001-Version-25-of-MERGE-patch-based-on-ON-CONFLICT-D.patch application/octet-stream 322.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-03-26 14:56:59 Re: Proposal: http2 wire format
Previous Message Claudio Freire 2018-03-26 14:26:51 Re: [HACKERS] [PATCH] Vacuum: Update FSM more frequently