Subject: Re: adding partitioned tables to publications
To: Amit Langote <amitlangote09@gmail.com>
References: 
 <CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com>
 <CA+HiwqEF4K+SxBZkU-=q6krGD74ysZZZv5z10_2C=+UcE2vTYw@mail.gmail.com>
 <CA+FpmFf2HVQYRQNh1ZvWPwatyr8_5n4pHRzNtmD5zVqs1zoLbQ@mail.gmail.com>
 <CA+HiwqEgn5da9KvWP4y0rBjcMtHdRao01Lr6TgTw78+UGceJwQ@mail.gmail.com>
 <037d3133-1689-108c-360d-b782e3209cfb@2ndquadrant.com>
 <CA+HiwqHf8_bcyHt=6R4O_Ju4P_Wz6wu1MkOAe14URdpdafS_KQ@mail.gmail.com>
 <CA+HiwqEQb5-26-=-K=vrWKQhvn7kqOfS-Vgj+H2YMw5XtdUUQw@mail.gmail.com>
 <eed6fb07-94d6-a2a5-33e1-84679dca9363@2ndquadrant.com>
 <CA+HiwqF7JD0CLkN12KNuGsf5GfaLuVb-Nc6+nTyL_iOhrSuA0g@mail.gmail.com>
 <CA+HiwqFWK3raKUU5Fp789jBsju-pYVpAiw0OsuO5sMX1CkYwFQ@mail.gmail.com>
 <b1e7d3c5-d4f8-2bc6-80a1-18b18c5db347@2ndquadrant.com>
 <CA+HiwqFwZV4F5sznWvgqf4eR6SebWLTq--gLVytNdtiCDMn=vg@mail.gmail.com>
 <7a23e60b-ffe8-bce1-5c44-db48d3bc163e@2ndquadrant.com>
 <CA+HiwqGZuouqjvWGv2MZ3EOyVej2_6Z=Yv2GHia4Mfk+YY_fHA@mail.gmail.com>
 <CA+HiwqGUZJYaJSM5cPKKFrFSJbTsDyJ9o4Vd=M2d7i5brKvGbA@mail.gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Organization: 2ndQuadrant
Message-ID: <b6d8519b-353f-1949-79b5-460a6712e20f@2ndquadrant.com>
Date: Wed, 11 Dec 2019 16:48:25 +0100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:52.0)
 Gecko/20100101 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: 
 <CA+HiwqGUZJYaJSM5cPKKFrFSJbTsDyJ9o4Vd=M2d7i5brKvGbA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2019-12-06 08:48, Amit Langote wrote:
> 0001:  Adding a partitioned table to a publication implicitly adds all
> its partitions.  The receiving side must have tables matching the
> published partitions, which is typically the case, because the same
> partition tree is defined on both nodes.

This looks pretty good to me now.  But you need to make all the changed 
queries version-aware so that you can still replicate from and to older 
versions.  (For example, pg_partition_tree is not very old.)

This part looks a bit fishy:

+       /*
+        * If either table is partitioned, skip copying.  Individual 
partitions
+        * will be copied instead.
+        */
+       if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
+               remote_relkind == RELKIND_PARTITIONED_TABLE)
+       {
+               logicalrep_rel_close(relmapentry, NoLock);
+               return;
+       }

I don't think you want to filter out a partitioned table on the local 
side, since (a) COPY can handle that, and (b) it's (as of this patch) an 
error to have a partitioned table in the subscription table set.

I'm not a fan of the new ValidateSubscriptionRel() function.  It's too 
obscure, especially the return value.  Doesn't seem worth it.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services