Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0 - Mailing list pgsql-hackers
From | Peter Geoghegan |
---|---|
Subject | Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0 |
Date | |
Msg-id | CAM3SWZQN=bSgmiPkp=97=ajigVotTzxWXCucjhcZt+M5rpR5Mw@mail.gmail.com Whole thread Raw |
In response to | Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0 (Peter Geoghegan <pg@heroku.com>) |
Responses |
Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0
Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0 Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0 |
List | pgsql-hackers |
On Fri, Apr 17, 2015 at 1:04 PM, Peter Geoghegan <pg@heroku.com> wrote: > What you meant was that you'd decided that this pattern is not broken, > *provided* that the implementation simply account for the fact that a > REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT change could come before some > other (non-REORDER_BUFFER_CHANGE_INTERNAL_DELETE, A.K.A. > non-superdelete) change came? And that we might need to be more > "patient" about deciding that a speculative insertion succeeded (more > "patient" than considering only one single non-superdelete change, > that can be anything else)? Attached patch, V3.4, implements what I believe you and Heikki have in mind here. There is a minimal, dedicated "affirmation" WAL record now. Importantly, it is not possible for committed tuples to be speculative (i.e. to have a magic value in their t_ctid field that indicates being a speculative tuple - Andres felt very strongly that we ought to be able to assume that committed tuples are not speculative). There are a bunch of new assertions to make sure that this actually holds now, all within tqual.c. I found Dean's recent argument [1] for mostly following the existing RLS behaviors convincing. I'm now I'm tracking what Dean called insertCheckQuals, updateUsingQuals and updateCheckQuals separately within the executor, while enforcing each (including updateUsingQuals) in the manner of WCOs (i.e. there are no silent failures when updateUsingQuals does not pass on a TARGET.* tuple, just as before - there is a WCO-style error thrown instead). And, as Dean and Stephen recently suggested, there is one and only one tuple (per ExecInsert() call) involved in enforcement for each of these three quals (or these three OR'd list of quals). These three tuples are the post-insert, the pre-update, and the post-update tuples, for the insertCheckQuals, updateUsingQuals and updateCheckQuals respectively. The new implementation has extensive revised tests - the only sane way to write something like ON CONFLICT UPDATE's RLS support is using test-driven development, IMV. There is one intentional difference between what I've done here, and what I believe Dean wants: I don't bother checking the user-supplied quals under any circumstances (so I don't "Test user-supplied auxiliary WHERE clause (skip if not true)", as described in [1]). I think we should *always* throw an error, and not silently skip the UPDATE just because the user supplied quals also limits the UPDATE. I don't want to introduce a complicated further distinction like that, because it seems like a distinction without a difference. Adding this behavior will not make something work that would not otherwise work - it will make a failure to UPDATE silent, and nothing more, AFAICT. That's much more subtle and much less necessary in the context of an auxiliary UPDATE (compared to a regular UPDATE). Other changes: * Changed magic offset, per Heikki's request. * RLS documentation updated, in line with new ON CONFLICT UPDATE behavior (this made it simpler). * Moved a bit of code from the IGNORE commit to the ON CONFLICT UPDATE commit (this concerns new possibility of heap_lock_tuple() HeapTupleInvisible return code introduced by ON CONFLICT UPDATE). It clearly belongs there. * RLS error messages advertise what type of command the violation originated from, and if the WCO originated as a USING security barrier qual (i.e. whether or not the target tuple that necessitates taking the UPDATE path was where the violation occurred, or whether it occurred on the post-update tuple intended to be appended to the relation). ON CONFLICT UPDATE makes this distinction important, since it may not be obvious which policy was violated (maybe this should go as far as naming the policy directly - I'm waiting for Stephen to push Dean's work, actually, because it will probably bitrot what I have here). These additional diagnostics were very helpful when writing those new RLS ON CONFLICT UPDATE tests, and will probably be helpful generally. Thoughts? [1] http://www.postgresql.org/message-id/CAEZATCVDdYRFbF4NMXTF-NKYibbR2VSfNXRWPyyByaCpV1jwEw@mail.gmail.com -- Peter Geoghegan
Attachment
pgsql-hackers by date: