On Wed, Oct 12, 2016 at 3:02 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> I agree that the multi-value case is a bug.
> I think that it should be pretty obvious to you why the check exists
> at all, Kevin. It exists because it would be improper to decide to
> take the DO NOTHING path on the basis of some other committed tuple
> existing that is not visible to the original MVCC snapshot, at higher
> isolation levels.
That's only true if it causes a cycle in the apparent order of
execution. If we rip out the check what we have is behavior
completely consistent with the other transaction executing first;
in other words, it creates a read-write dependency (a/k/a
rw-conflict) from the current transaction to the concurrent
transaction which succeeds in its insert. That may or may not
cause a cycle, depending on what else is happening.
Now, generating a write conflict serialization failure will prevent
serialization anomalies, but unless I'm missing something it is a
cruder test than is required, and will generate false positives in
simple cases like Thomas created. What I think would be more
appropriate is to record the dependency and test for a
serialization failure.
> There is a similar consideration for DO UPDATE.
But in DO UPDATE the current transaction is writing something that
the other transaction attempted to read, so that *does* create
write skew and its attendant anomalies. There I think we *do* need
to throw a serialization failure error.
> I'm slightly surprised that you're contemplating just ripping the check out.
The lack of any regression tests to demonstrate the failure the
code is preventing puts the burden on others to figure it out fresh
each time, which had me a little miffed. Note that you are
mis-quoting me a bit -- I said "if I can't find some justification
for this function I will rip it (and the calls to it) out of the
code." I was still looking. I don't think the write conflict
justifies it, but the rw-conflict and attendant risk of a cycle in
apparent order of execution does.
If the "proper" fix is impossible (or just too freaking ugly) we
might fall back on the fix Thomas suggested, but I would like to
take advantage of the "special properties" of the INSERT/ON
CONFLICT DO NOTHING code to avoid false positives where we can.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
