pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Petr Jelinek <petr@2ndquadrant.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: adding partitioned tables to publications
Date: Fri, 3 Apr 2020 09:52:07 +0200
Message-ID: <ff6007b3-02c2-8296-2896-db99a51f57e2@2ndquadrant.com> (raw)
In-Reply-To: <4f999141-97c8-c968-a726-0a1ec3e36e47@2ndquadrant.com>
References: <CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com>
	<CA+FpmFfve1TtLvj5CwpgEZrjNiFG-EWh4ywpbtrB7-UTkZo2gg@mail.gmail.com>
	<CA+HiwqHfV73E4aJxX6d79xSEZjp+stFKnD0+7OktPw2_tD=pYg@mail.gmail.com>
	<73a1877a-6f1d-9f2e-da51-bc2eb57e07d5@2ndquadrant.com>
	<CA+HiwqFjo=vCXDdkwprf6ipaVeegCpW79n-Ds8BK5cNQsSey2A@mail.gmail.com>
	<d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com>
	<CA+HiwqFBni-cXy5NBN0PdwxidXgcCEio0N_AOnOM0pR2dMWUyA@mail.gmail.com>
	<CA+HiwqGVbp0yh-7-FscZiYu1B_CJjaDZRTYkPTjnjpUgSzE6RA@mail.gmail.com>
	<b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com>
	<CA+HiwqFZbbJzAZgTjqbRarp_xeqyymuNmGg8Q5tDKTZbk3=c2A@mail.gmail.com>
	<c0d69728-c3d8-ede8-ad31-941b2bbaff66@2ndquadrant.com>
	<CA+HiwqE+6gNgr-BVYeCMT38naRi9cJaanJb7+FCNsX9XgMwHsQ@mail.gmail.com>
	<CA+HiwqFjYE5anArxvkjr37AQMd52L-LZtz9Ld2QrLQ3YfcYhTw@mail.gmail.com>
	<CA+HiwqE+z2=wLv3a41h7JopcnUZAiFPeaiQyxZeGQ62QV3vsPQ@mail.gmail.com>
	<4f999141-97c8-c968-a726-0a1ec3e36e47@2ndquadrant.com>

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/





view thread (86+ messages)  latest in thread

Message-ID: <ff6007b3-02c2-8296-2896-db99a51f57e2@2ndquadrant.com>
Permalink:  ../ff6007b3-02c2-8296-2896-db99a51f57e2@2ndquadrant.com/
Also on:    postgresql.org/message-id/ff6007b3-02c2-8296-2896-db99a51f57e2@2ndquadrant.com

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: petr@2ndquadrant.com, peter.eisentraut@2ndquadrant.com, amitlangote09@gmail.com, rafia.pghackers@gmail.com
  Subject: Re: adding partitioned tables to publications
  In-Reply-To: <ff6007b3-02c2-8296-2896-db99a51f57e2@2ndquadrant.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox