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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
