Re: [HACKERS] MERGE SQL Statement for PG11

Hi Stephen,

On Wed, Feb 14, 2018 at 9:59 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

>
>
> The reasoning behind the INSERT ON CONFLICT DO UPDATE approach when it
> comes to RLS is that we specifically didn't want to end up "losing"
> data- an INSERT which doesn't actually INSERT a row (or take the UPDATE
> action if the row already exists) ends up throwing that data away even
> though clearly the user expected us to do something with it, which is
> why we throw an error in that case instead.
>
> For my part, at least, it seems like MERGE would likewise possibly be
> throwing away data that the user is expecting to be incorporated if no
> action is taken due to RLS and that then argues that we should be
> throwing an error in such a case, similar to the INSERT ON CONFLICT DO
> UPDATE case.
>

Thanks for confirming the approach. That matches my own thinking too.

Here is an updated v16a patch. This now supports RLS based on what we
agreed above. I've added a few test cases to confirm that RLS works
correctly with MERGE. We can more later if needed.

This version now also adds support for OVERRIDING clause in the INSERT
statement. The whole var referencing issue pointed out by Peter is also
fixed by this version. So to summarise the following things are now working:

- Partitioning
- Subqueries in WHEN AND quals and UPDATE targetlists
- Row level security (documentation changes are pending)
- OVERRIDING clause
- Various bugs reported by Peter are fixed (I haven't double checked if all
issues are addressed or not, but we should be fairly close)
- Issues so far reported by Andreas Seltenreich as part of sqlsmith testing
are fixed

I haven't yet addressed Tomas's review comment on using WAL position for
write detection. I'm waiting for Simon to come back from holidays before
doing anything there. One idea is to use some variant
of contain_mutable_functions() and throw error during query planning, but
not do anything during execution.

Other than that, I think we are getting to a point where the patch is
mostly feature complete. We still need to decide the concurrent execution
behaviour, but hopefully code changes there will be limited (unless we try
to do something very invasive, which I hope we don't).

I'm not entirely happy with the way we're going about resolving column
references in INSERT/UPDATE targetlist and subsequently what we are doing
in setrefs.c. I'll continue to look for improvements there and also hoping
to get suggestions from this list.

Thanks,
Pavan

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