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/reasonablerestriction.
* 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
meansthat using upsert will potentially cause deadlocks in other sessions :(. I think you'll have to determine what
lockto 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
differentfrom the final insertion of the actual tuple. For one it seems cleaner to me, for another it will avoid the
uglynessaround logical decoding. I think also that the separation will make it more realistic to use something like
thisfor 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
