Re: adding partitioned tables to publications

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/