Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-26 Thread Kevin Grittner
On Wed, Oct 26, 2016 at 3:20 PM, Peter Geoghegan wrote: > On Mon, Oct 24, 2016 at 8:07 AM, Kevin Grittner wrote: >> My initial thought is that since reducing the false positive rate >> would only help when there was a high rate of conflicts under the >> existing

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-26 Thread Peter Geoghegan
On Mon, Oct 24, 2016 at 8:07 AM, Kevin Grittner wrote: > My initial thought is that since reducing the false positive rate > would only help when there was a high rate of conflicts under the > existing patch, and it would add code complexity and cost for the > case where

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-24 Thread Kevin Grittner
On Thu, Oct 13, 2016 at 5:26 PM, Thomas Munro wrote: > (1) postgres=# create table bank_account (id int primary key, cash int); > (1) CREATE TABLE > (1) postgres=# begin transaction isolation level serializable ; > (1) BEGIN > > (2) postgres=# begin

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-23 Thread Tom Lane
I've pushed a simplified (no refactoring) version of the fix proposed by Thomas and Peter, so that we have some kind of fix in place for tomorrow's releases. I think further improvement along the lines suggested by Kevin can wait till later. I noticed that the ON CONFLICT DO NOTHING and ON

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-14 Thread Kevin Grittner
On Thu, Oct 13, 2016 at 5:26 PM, Thomas Munro wrote: > On Fri, Oct 14, 2016 at 2:04 AM, Kevin Grittner wrote: >> Where do you see a problem if REPEATABLE READ handles INSERT/ON >> CONFLICT without error? > I think the ON CONFLICT > equivalent

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Thomas Munro
On Fri, Oct 14, 2016 at 2:04 AM, Kevin Grittner wrote: > On Wed, Oct 12, 2016 at 8:06 PM, Thomas Munro > wrote: >> The "higher isolation levels" probably shouldn't be treated the same way. >> >> I think Peter's right about REPEATABLE READ. We

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Kevin Grittner
On Thu, Oct 13, 2016 at 3:16 PM, Kevin Grittner wrote: > On Thu, Oct 13, 2016 at 2:16 PM, Peter Geoghegan wrote: >> We must still determine if a fix along the lines of the one proposed >> by Thomas is basically acceptable (that is, that it does not clearly >>

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Kevin Grittner
On Thu, Oct 13, 2016 at 2:16 PM, Peter Geoghegan wrote: > On Thu, Oct 13, 2016 at 6:19 AM, Kevin Grittner wrote: >> Every situation that generates a false positive hurts performance; >> we went to great lengths to minimize those cases. >> To generate a >>

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Peter Geoghegan
On Thu, Oct 13, 2016 at 6:19 AM, Kevin Grittner wrote: >> I was under the impression that false positives of this kind are >> allowed by SSI. Why focus on this false positive scenario in >> particular? > > Every situation that generates a false positive hurts performance; > we

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 8:32 PM, Peter Geoghegan wrote: > On Wed, Oct 12, 2016 at 6:06 PM, Thomas Munro > wrote: >> But yeah, the existing code raises false positive serialization >> failures under SERIALIZABLE, and that's visible in the isolation

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 8:06 PM, Thomas Munro wrote: > On Thu, Oct 13, 2016 at 10:06 AM, Kevin Grittner wrote: >> On Wed, Oct 12, 2016 at 3:02 PM, Peter Geoghegan wrote: >> >>> I agree that the multi-value case is a bug. >> >>> I

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-13 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 5:21 PM, Peter Geoghegan wrote: > On Wed, Oct 12, 2016 at 2:06 PM, Kevin Grittner wrote: >> 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

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Thomas Munro
On Thu, Oct 13, 2016 at 2:32 PM, Peter Geoghegan wrote: > On Wed, Oct 12, 2016 at 6:06 PM, Thomas Munro > wrote: >> But yeah, the existing code raises false positive serialization >> failures under SERIALIZABLE, and that's visible in the isolation

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Peter Geoghegan
On Wed, Oct 12, 2016 at 6:06 PM, Thomas Munro wrote: > But yeah, the existing code raises false positive serialization > failures under SERIALIZABLE, and that's visible in the isolation test > I posted: there is actually a serial order of those transactions with >

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Thomas Munro
On Thu, Oct 13, 2016 at 10:06 AM, Kevin Grittner wrote: > On Wed, Oct 12, 2016 at 3:02 PM, Peter Geoghegan 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

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Peter Geoghegan
On Wed, Oct 12, 2016 at 2:06 PM, Kevin Grittner wrote: > 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

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 3:55 PM, Peter Geoghegan wrote: > On Wed, Oct 12, 2016 at 1:41 PM, Kevin Grittner wrote: >> Aren't these two completely separate and independent bugs? > > Technically they are, but they are both isolated to the same small > function.

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 3:02 PM, Peter Geoghegan 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

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Peter Geoghegan
On Wed, Oct 12, 2016 at 1:41 PM, Kevin Grittner wrote: > Aren't these two completely separate and independent bugs? Technically they are, but they are both isolated to the same small function. Surely it's better to fix them both at once? -- Peter Geoghegan -- Sent via

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 3:07 PM, Peter Geoghegan wrote: > On Wed, Oct 12, 2016 at 1:05 PM, Thomas Munro > wrote: >> Here's a patch that shows one way to fix it. I think it does make >> sense to change this, because otherwise automatic >>

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Peter Geoghegan
On Wed, Oct 12, 2016 at 1:05 PM, Thomas Munro wrote: > Here's a patch that shows one way to fix it. I think it does make > sense to change this, because otherwise automatic > retry-on-serialization-failure strategies will be befuddle by this > doomed transaction.

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Thomas Munro
On Thu, Oct 13, 2016 at 8:45 AM, Kevin Grittner wrote: > On Wed, Oct 12, 2016 at 10:06 AM, Kevin Grittner wrote: > >> The test in ExecCheckHeapTupleVisible() seems wrong to me. It's >> not immediately obvious what the proper fix is. > > To identify what

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Peter Geoghegan
On Wed, Oct 12, 2016 at 12:45 PM, Kevin Grittner wrote: >> The test in ExecCheckHeapTupleVisible() seems wrong to me. It's >> not immediately obvious what the proper fix is. (prune old e-mail address from cc list) I agree that the multi-value case is a bug. > To identify

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 10:06 AM, Kevin Grittner wrote: > The test in ExecCheckHeapTupleVisible() seems wrong to me. It's > not immediately obvious what the proper fix is. To identify what cases ExecCheckHeapTupleVisible() was meant to cover I commented out the body of the

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
[adding Peter Geoghegan to the email addresses] On Wed, Oct 12, 2016 at 7:11 AM, Vitaly Burovoy wrote: > On 10/12/16, Thomas Munro wrote: >> This happens in both SERIALIZABLE and REPEATABLE READ when a single >> command inserts

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Kevin Grittner
On Wed, Oct 12, 2016 at 2:50 AM, Albe Laurenz wrote: > Kevin Grittner wrote: >> I don't see that on development HEAD. What version are you >> running? What is your setting for default_transaction_isolation? > > The subject says SERIALIZABLE, and I can see it on my

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Vitaly Burovoy
On 10/12/16, Thomas Munro wrote: > On Wed, Oct 12, 2016 at 8:50 PM, Albe Laurenz > wrote: >> Kevin Grittner wrote: >>> On Tue, Oct 11, 2016 at 2:29 PM, Jason Dusek >>> wrote: I notice the following oddity: >>>

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Thomas Munro
On Wed, Oct 12, 2016 at 8:50 PM, Albe Laurenz wrote: > Kevin Grittner wrote: >> On Tue, Oct 11, 2016 at 2:29 PM, Jason Dusek wrote: >>> I notice the following oddity: >> >>> =# CREATE TABLE with_pk (i integer PRIMARY KEY); >>> CREATE TABLE >> >>>

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-12 Thread Albe Laurenz
Kevin Grittner wrote: > Sent: Tuesday, October 11, 2016 10:00 PM > To: Jason Dusek > Cc: pgsql-general@postgresql.org > Subject: Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES > > On Tue, Oct 11, 2016 at 2:29 PM, Jason Dusek <jason.du...@gmail.com> wrote: >

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-11 Thread Jason Dusek
SELECT version(), (SELECT setting FROM pg_settings WHERE name = 'default_transaction_deferrable') AS default_transaction_deferrable, (SELECT setting FROM pg_settings WHERE name = 'default_transaction_isolation') AS default_transaction_isolation; ─[ RECORD 1

Re: [GENERAL] SERIALIZABLE and INSERTs with multiple VALUES

2016-10-11 Thread Kevin Grittner
On Tue, Oct 11, 2016 at 2:29 PM, Jason Dusek wrote: > I notice the following oddity: > =# CREATE TABLE with_pk (i integer PRIMARY KEY); > CREATE TABLE > =# BEGIN; > BEGIN > =# INSERT INTO with_pk VALUES (2), (2) ON CONFLICT DO NOTHING; > ERROR: could not serialize