I made a bunch of further edits and I think this v10 is ready to push.
Before doing so I'll give it a final look, particularly because of the
new elog(ERROR) I added. Post-commit review is of course always
appreciated.
Most notable change is because I noticed that if you mention an
intermediate partition level in the INSERT command, and the index is on
the top level, arbiter index selection fails to find the correct index
because it walks all the way to the top instead of stopping in the
middle, as it should (the command was still working because it ended up
with an empty arbiter index list).
To fix this, I had to completely rework the "get partition parent root"
stuff into "get list of ancestors of this partition".
Because of this, I added a new check that the partition's arbiter index
list is same length as parent's; if not, throw an error. I couldn't get
it to fire (so it's just an elog not ereport), but maybe I just didn't
try any weird enough scenarios.
Other changes:
* I added a copyObject() call for nodes we're operating upon. Maybe
this is unnecessary but the comments claimed "we're working on a copy"
and I couldn't find any place where we were actually making one.
Anyway it seems sane to make a copy, because we're scribbling on those
nodes ... I hope I didn't introduce any serious memory leaks.
* I made the new OnConflictSetState thing into a proper node. Like
ResultRelInfo, it doesn't have any niceties like nodeToString support,
but it seems saner this way (palloc -> makeNode). I reworked the
formatting of that struct definition too, and renamed members.
* I removed an assertion block at the bottom of adjust_partition_tlist.
It seemed quite pointless, since it was just about checking that the
resno values were sorted, but by construction we already know that
they are indeed sorted ...
* General code style improvements, comment rewording, etc.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
