RE: Replica Identity check of partition table on subscriber

On Tue, Jun 14, 2022 2:18 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Mon, Jun 13, 2022 at 9:26 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > On Mon, Jun 13, 2022 at 1:03 PM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > On Monday, June 13, 2022 1:53 PM Amit Kapila
> <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > I have separated out the bug-fix for the subscriber-side.
> > > And fix the typo and function name.
> > > Attach the new version patch set.
> > >
> >
> > The first patch looks good to me. I have slightly modified one of the
> > comments and the commit message. I think we need to backpatch this
> > through 13 where we introduced support to replicate into partitioned
> > tables (commit f1ac27bf). If you guys are fine, I'll push this once
> > the work for PG14.4 is done.
>
> Both the code changes and test cases look good to me. Just a couple
> of minor nitpicks with test changes:

Thanks for your comments.

>
> + CREATE UNIQUE INDEX tab5_a_idx ON tab5 (a);
> + ALTER TABLE tab5 REPLICA IDENTITY USING INDEX tab5_a_idx;
> + ALTER TABLE tab5_1 REPLICA IDENTITY USING INDEX tab5_1_a_idx;});
>
> Not sure if we follow it elsewhere, but should we maybe avoid using
> the internally generated index name as in the partition's case above?
>

I saw some existing tests also use internally generated index name (e.g.
replica_identity.sql, ddl.sql and 031_column_list.pl), so maybe it's better to
fix them all in a separate patch. I didn't change this.

> +# Test the case that target table on subscriber is a partitioned table and
> +# check that the changes are replicated correctly after changing the schema
> of
> +# table on subcriber.
>
> The first sentence makes it sound like the tests that follow are the
> first ones in the file where the target table is partitioned, which is
> not true, so I think we should drop that part. Also how about being
> more specific about the test intent, say:
>
> Test that replication continues to work correctly after altering the
> partition of a partitioned target table.
>

OK, modified.

Attached the new version of patch set, and the patches for pg14 and pg13.

Regards,
Shi yu