Re: create subscription with (origin = none, copy_data = on)

On Wed, 29 Jan 2025 at 15:58, vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 24 Jan 2025 at 09:52, Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> > I have added the test in the latest patch.
>
> Few comments:
> 1) Let's rearrange this query slightly so that the "PT.pubname IN
> (&lt;pub-names&gt;)" appears at the end, the reason being that it will
> be easy to copy/paste and edit it to include the publications names if
> it is at the end:
> +++ b/doc/src/sgml/ref/create_subscription.sgml
> @@ -534,13 +534,15 @@ CREATE SUBSCRIPTION <replaceable
> class="parameter">subscription_name</replaceabl
>  <programlisting>
>  # substitute &lt;pub-names&gt; below with your publication name(s) to
> be queried
>  SELECT DISTINCT PT.schemaname, PT.tablename
> -FROM pg_publication_tables PT,
> +FROM pg_publication_tables PT
> +     JOIN pg_class C ON (C.relname = PT.tablename)
> +     JOIN pg_namespace N ON (N.nspname = PT.schemaname),
>       pg_subscription_rel PS
> -     JOIN pg_class C ON (C.oid = PS.srrelid)
> -     JOIN pg_namespace N ON (N.oid = C.relnamespace)
> -WHERE N.nspname = PT.schemaname AND
> -      C.relname = PT.tablename AND
> -      PT.pubname IN (&lt;pub-names&gt;);
> +WHERE C.relnamespace = N.oid AND
> +      PT.pubname IN (&lt;pub-names&gt;) AND
> +      (PS.srrelid = C.oid OR
> +      C.oid IN (SELECT relid FROM pg_partition_ancestors(PS.srrelid) UNION
> +                SELECT relid FROM pg_partition_tree(PS.srrelid)));
>
> 2) The same should be handled in the PG17 version patch too.
>
> 3) Currently the setup is done like:
> node_B(table tab_part2 - publication pub_b_a) replicating to
> node_A(sub_a_b subscription)
> node_A(table tab_main - publication pub_a_c) replicating to node_C(sub_a_c)
>
> +###############################################################################
> +# Specifying origin = NONE and copy_data = on must raise WARNING if
> we subscribe
> +# to a partitioned table and this table contains any remotely originated data.
> +###############################################################################
> +
> +# create a partition table on node A
> +$node_A->safe_psql(
> +       'postgres', qq(
> +CREATE TABLE tab_main(a int) PARTITION BY RANGE(a);
> +CREATE TABLE tab_part1 PARTITION OF tab_main FOR VALUES FROM (0) TO (5);
> +CREATE TABLE tab_part2(a int) PARTITION BY RANGE(a);
> +CREATE TABLE tab_part2_1 PARTITION OF tab_part2 FOR VALUES FROM (5) TO (10);
> +ALTER TABLE tab_main ATTACH PARTITION tab_part2 FOR VALUES FROM (5) to (10);
> +));
> +
> +# create a table on node B which will act as a source for a partition on node A
> +$node_B->safe_psql(
> +       'postgres', qq(
>
> Can we change this like below to make review easier:
> node_A(table tab_part2 - publication pub_b_a) replicating to
> node_B(sub_a_b subscription)
> node_B(table tab_main - publication pub_a_c) replicating to node_C(sub_a_c)
>
> Also add something similar like above to the comment.
>

I have addressed the comments. Here is an updated patch.


Thanks and Regards,
Shlok Kyal