[adding Peter Geoghegan to the email addresses]

On Wed, Oct 12, 2016 at 7:11 AM, Vitaly Burovoy <vitaly.buro...@gmail.com>
wrote:
> On 10/12/16, Thomas Munro <thomas.mu...@enterprisedb.com> wrote:

>> This happens in both SERIALIZABLE and REPEATABLE READ when a single
>> command inserts conflicting rows with an ON CONFLICT cause, and it
>> comes from the check in ExecCheckHeapTupleVisible whose comment says:
>>
>> /*
>>  * ExecCheckHeapTupleVisible -- verify heap tuple is visible
>>  *
>>  * It would not be consistent with guarantees of the higher isolation
levels to
>>  * proceed with avoiding insertion (taking speculative insertion's
alternative
>>  * path) on the basis of another tuple that is not visible to MVCC
snapshot.
>>  * Check for the need to raise a serialization failure, and do so as
necessary.
>>  */
>>
>> So it seems to be working as designed.  Perhaps someone could argue
>> that you should make an exception for tuples inserted by the current
>> command.
>
> I disagree. It is designed to prevent updating a tuple which was
> updated in a parallel transaction which has been just committed.
> This case is a little bit different: this tuple has been inserted in
> the same transaction in the same command.
> I think it is an obvious bug because there is no "concurrent update".
> There is no update at all.

Yeah, the concept of a serialization failure is that the
transaction would have succeeded except for the actions of a
concurrent transaction.  It is supposed to indicate that a retry
has a chance to succeed, because the conflicting transaction is
likely to have completed.  To generate a "serialization failure"
(or, IMO, any error with a SQLSTATE in class "40") from the actions
of a single transaction is completely wrong, and should be
considered a bug.

> ON CONFLICT handling just does not cover all possible ways which can
happen.
> Normally (without "ON CONFLICT" clause) INSERT raises "duplicate key
> value violates unique constraint" and doesn't run to
> "ExecCheckHeapTupleVisible" check.
> The "ExecInsert" handles constraint checks but not later checks like
> ExecCheckHeapTupleVisible.

The test in ExecCheckHeapTupleVisible() seems wrong to me.  It's
not immediately obvious what the proper fix is.  Peter, do you have
any ideas on this?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Reply via email to