Re: INSERT...ON DUPLICATE KEY IGNORE

On Fri, Aug 30, 2013 at 5:47 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> This is awesome.

Thanks.

> All that seems sane to me. I very, very much do not want it to deal with
> NOT NULL violations.

Sure. But there's nothing stopping us from doing that as a totally
orthogonal thing. Not that I personally find it to be terribly
compelling or anything. It's an easy addition, but I'm certainly not
going to try and mandate it as integral to what I've done here if that
doesn't suit you.

> Ok, so what we would like to do is basically to follow a protocol
> (simplified) like:
>
> 1) replay INSERT, using ON DUPLICATE IGNORE
> 2) if INSERT succeeded, be happy, no conflict (another INSERT with the
> same key might conflict though)
> 3) if the INSERT failed, lock tuple for UPDATE
> 4) if the tuple got deleted by another session, goto 1
> 5) check whether local tuple or the already inserted tuple wins conflict resolution
> 6) UPDATE or skip accordingly

Right, I thought that's what you meant. I'll have to ponder it
further. However, I don't think locking the old tuple(s) is mandatory
to make this a useful feature - as I've noted, MySQL doesn't do that.
That said, I really want to support that, perhaps as another DUPLICATE
KEY variant. As I've noted, if we had that, we almost wouldn't need
upsert - loosely speaking it would be mere syntactic sugar on top of
what you require.

> If there's a variant of INSERT ... ON DUPLICATE that gets a FOR UPDATE
> lock on the existing row this gets simpler because there's no chance
> anymore we need to loop or look for other versions of the row.
>
> Makes sense?

Perfect sense.

> Puh. It took me some time to even start to understand what you're doing
> here...
>
> While I ponder on it some more, could you explain whether I understood
> something correctly? Consider the following scenario:
>
> CREATE TABLE foo(a int UNIQUE, b int UNIQUE);
>
> S1: INSERT INTO foo(0, 0);
> S1: BEGIN;
> S1: INSERT INTO foo(1, 1);
> S1: SELECT pg_sleep(3600);
> S2: BEGIN;
> S2: INSERT INTO foo(2, 1);
> S3: SELECT * FROM foo WHERE a = 0;
>
> Won't S2 in this case exclusively lock a page in foo_a_key (the one
> where 2 will reside on) for 3600s, while it's waiting for S1 to finish
> while getting the speculative insertion into foo_b_key?

Yes. The way the patch currently handles that case is unacceptable. It
needs to be more concerned about holding an exclusive lock on
earlier-locked indexes when locking the second and subsequent indexes
iff it blocks on another transaction in the course of locking the
second and subsequent indexes.

> ISTM we could use something like a new lock level to make that work more
> sensibly, basically something like LW_FOR_UPDATE which does not conflict
> with _SHARED but conflicts with _EXCLUSIVE.

I'll ponder it further. There are probably a number of options.

--
Peter Geoghegan