RE: Data is copied twice when specifying both child and parent table in publication

On Fri, Mar 24, 2023 at 14:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > ======
> > > > src/test/subscription/t/028_row_filter.pl
> > > >
> > > > 7.
> > > > +# insert data into partitioned table.
> > > > +$node_publisher->safe_psql('postgres',
> > > > + "INSERT INTO tab_rowfilter_viaroot_part(a) VALUES(13), (17)");
> > > > +
> > > >  $node_subscriber->safe_psql('postgres',
> > > >   "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr
> > > > application_name=$appname' PUBLICATION tap_pub_1, tap_pub_2,
> > > > tap_pub_3, tap_pub_4a, tap_pub_4b, tap_pub_5a, tap_pub_5b,
> > > > tap_pub_toast, tap_pub_inherits, tap_pub_viaroot_2, tap_pub_viaroot_1"
> > > >  );
> > > > @@ -707,13 +711,17 @@ is($result, qq(t|1), 'check replicated rows to
> > > > tab_rowfilter_toast');
> > > >  # the row filter for the top-level ancestor:
> > > >  #
> > > >  # tab_rowfilter_viaroot_part filter is: (a > 15)
> > > > +# - INSERT (13)        NO, 13 < 15
> > > >  # - INSERT (14)        NO, 14 < 15
> > > >  # - INSERT (15)        NO, 15 = 15
> > > >  # - INSERT (16)        YES, 16 > 15
> > > > +# - INSERT (17)        YES, 17 > 15
> > > >  $result =
> > > >    $node_subscriber->safe_psql('postgres',
> > > > - "SELECT a FROM tab_rowfilter_viaroot_part");
> > > > -is($result, qq(16), 'check replicated rows to tab_rowfilter_viaroot_part');
> > > > + "SELECT a FROM tab_rowfilter_viaroot_part ORDER BY 1");
> > > > +is( $result, qq(16
> > > > +17),
> > > > + 'check replicated rows to tab_rowfilter_viaroot_part');
> > > >
> > > > ~
> > > >
> > > > I'm not 100% sure this is testing quite what you want to test. AFAICT
> > > > the subscription is created like:
> > > >
> > > > "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr
> > > > application_name=$appname' PUBLICATION tap_pub_1, tap_pub_2,
> > > > tap_pub_3, tap_pub_4a, tap_pub_4b, tap_pub_5a, tap_pub_5b,
> > > > tap_pub_toast, tap_pub_inherits, tap_pub_viaroot_2, tap_pub_viaroot_1"
> > >
> > > I think this is the scenario we fixed : Simultaneously subscribing to two
> > > publications that publish the parent and child respectively, then want to use
> > > the parent's identity and schema).
> > >
> >
> > Yeah, but currently BOTH the tap_pub_viaroot_2, tap_pub_viaroot_1 are
> > using "WITH (publish_via_partition_root)", so IMO the user would
> > surely expect that only the root table would be published even when a
> > subscription combines those publications. OTOH, I thought a subtle
> > point of this patch is that now the same result will happen even if
> > only ONE of the publications was using "WITH
> > (publish_via_partition_root)". So it’s that scenario of “only ONE
> > publication is using the option” that I thought ought to be explicitly
> > tested.
> >
> 
> The current change to existing tests is difficult to understand. I
> suggest writing a separate test for row filter and then cover the
> scenario you have suggested.

Changed as suggested.

And I found there is a problem in the three back-branch patches (HEAD_v21_0002*,
REL15_* and REL14_*):
In the function fetch_table_list, we use pg_partition_ancestors to get the list
of tables from the publisher. But the pg_partition_ancestors was introduced in
v12, which means that if the publisher is v11 and the subscriber is v14+, this
will cause an error.
Since we are going to first submit the fix for the publisher >= 16 case on HEAD,
I think we could discuss this issue later if needed. Also, I will update these
three patches later if needed.

Attach the new patch.

Regards,
Wang Wei