Hi,
On 02/04/2020 14:23, Peter Eisentraut wrote:
> 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.
> > 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?)
>
It works because it's just a MemoryContext and it's long lived. I wonder
if the fix here is to simply remove the LogicalRepPartMapContext...
> 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.
>
FWIW it looks okay to me as well from perspective of somebody who
implemented something similar outside of core.
> 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.
>
+1 (disclaimer: I was one of the people who discussed this offline)
--
Petr Jelinek
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/