[adding Peter Geoghegan to the email addresses]
On Wed, Oct 12, 2016 at 7:11 AM, Vitaly Burovoy <vitaly.burovoy@xxxxxxxxx> wrote:
> On 10/12/16, Thomas Munro <thomas.munro@xxxxxxxxxxxxxxxx> 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
On Wed, Oct 12, 2016 at 7:11 AM, Vitaly Burovoy <vitaly.burovoy@xxxxxxxxx> wrote:
> On 10/12/16, Thomas Munro <thomas.munro@xxxxxxxxxxxxxxxx> 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
