Re: INSERT ... ON CONFLICT IGNORE (and UPDATE) 3.0

Hi,

Just had a longer chat with Peter about this patch.

* Not a fan of the heap flags usage, the reusage seems sketch to me
* Explain should show the arbiter index in text as well
* AddUniqueSpeculative is a bad name, it should refer IndexInfo
* Work on the ExecInsert() comments
* Let's remove the planner choosing the "cheapest" arbiter index; it
should do all.
* s/infer_unique_index/infer_arbiter_index/
* Not supporting inheritance properly makes me uncomfortable. I don't
think users will think that's a acceptable/reasonable restriction.
* Let's not use t_ctid directly, but add a wrapper
* The code uses LockTupleExclusive to lock rows. That means the fkey key
locking doesn't work; That's annoying. This means that using upsert
will potentially cause deadlocks in other sessions :(. I think you'll
have to determine what lock to acquire by fetching the tuple, doing
the key comparison, and acquire the appropriate lock. That should be
fine.
* I think we should decouple the insertion and wal logging more. I think
the promise tuple insertion should be different from the final
insertion of the actual tuple. For one it seems cleaner to me, for
another it will avoid the uglyness around logical decoding. I think
also that the separation will make it more realistic to use something
like this for a COPY variant that doesn't raise unique violations and
such.
* We discussed the infererence and that it should just reuse (code,
grammar, docs) the column specification from create index.
* Some more stuff I don't recall.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services