Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
Subject: Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0
Date: 2015-03-17 23:25:16
Message-ID: CAM3SWZRBNvByx=1yyGd3KYfcWtUvv38hZSGTBF8Z56HrTmcePQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 16, 2015 at 8:10 AM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> (Note there is some bitrot in gram.y that prevents the first patch
> from applying cleanly to HEAD)

That's trivially fixable. I'll have those fixes in the next revision,
once I firm some things up with Heikki.

> I tested using the attached script, and one test didn't behave as I
> expected. I believe the following should have been a valid upsert
> (following the update path) but actually it failed:
>
> INSERT INTO t1 VALUES (4, 0) ON CONFLICT (a) UPDATE SET b = 1;
>
> AFAICT, it is applying a WITH CHECK OPTION with qual "b > 0 AND a % 2
> = 0" to the about-to-be-updated tuple (a=4, b=0), which is wrong
> because the "b > 0" check (policy p3) should only be applied to the
> post-update tuple.
>
> Possibly I'm missing something though.

I think that you may have. Did you read the commit message/docs of the
RLS commit 0004-*? You must consider the second point here, I believe:

""""
The 3 places that RLS policies are enforced are:

* Against row actually inserted, after insertion proceeds successfully
(INSERT-applicable policies only).

* Against row in target table that caused conflict. The implementation
is careful not to leak the contents of that row in diagnostic
messages (INSERT-applicable *and* UPDATE-applicable policies).

* Against the version of the row added by to the relation after
ExecUpdate() is called (INSERT-applicable *and* UPDATE-applicable
policies).

""""

You're seeing a failure that applies to the target tuple of the UPDATE
(the tuple that we can't leak the contents of). I felt it was best to
check all policies against the target/existing tuple, including both
WITH CHECK OPTIONS and USING quals (which are both enforced).

I can see why you might not like that behavior, but it is the intended
behavior. I thought that this whole intersection of RLS + UPSERT is
complex enough that it would be best to be almost as conservative as
possible in what fails and what succeeds. The one exception is when
the insert path is actually taken, since the statement is an INSERT
statement.
--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-03-17 23:34:04 Re: Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)
Previous Message Peter Geoghegan 2015-03-17 23:21:16 Re: Rethinking the parameter access hooks for plpgsql's benefit