Re: adding partitioned tables to publications

Petr Jelinek <petr@2ndquadrant.com>

From: Petr Jelinek <petr@2ndquadrant.com>

To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Amit Langote <amitlangote09@gmail.com>

Cc: Rafia Sabih <rafia.pghackers@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>

Date: 2020-04-03T07:52:07Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Fix RELCACHE_FORCE_RELEASE issue
- 12fb189bfeaf 13.0 landed
Allow publishing partition changes via ancestors
- 83fd4532a721 13.0 landed
Add logical replication support to replicate into partitioned tables
- f1ac27bfda6c 13.0 landed
Refactor code to look up local replication tuple
- d8653f468789 13.0 landed
Some refactoring of logical/worker.c
- 97ee604d9bce 13.0 landed
Prepare to support non-tables in publications
- c314c147c056 13.0 landed
Support adding partitioned tables to publication
- 17b9e7f9fe23 13.0 landed

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/