On 2020-03-30 17:42, Amit Langote wrote:
I have updated the comments in apply_handle_tuple_routing() (see 0002)
to better explain what's going on with UPDATE handling. I also
rearranged the tests a bit for clarity.
Attached updated patches.
Test coverage for 0002 is still a bit lacking. Please do a coverage
build yourself and get at least one test case to exercise every branch
in apply_handle_tuple_routing(). Right now, I don't see any coverage
for updates without attribute remapping and updates that don't move to a
new partition.
Also, the coverage report reveals that in logicalrep_partmap_init(), the
patch is mistakenly initializing LogicalRepRelMapContext instead of
LogicalRepPartMapContext. (Hmm, how does it even work like that?)
I think apart from some of these details, this patch is okay, but I
don't have deep experience in the partitioning code, I can just see that
it looks like other code elsewhere. Perhaps someone with more knowledge
can give this a look as well.
About patch 0003, I was talking to some people offline about the name of
the option. There was some confusion about using the term "schema".
How about naming it "publish_via_partition_root", which also matches the
name of the analogous option in pg_dump.
Code coverage here could also be improved. A lot of the new code in
pgoutput.c is not tested.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
