On Mon, Oct 28, 2024 at 5:45 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> On Mon, Oct 28, 2024 at 7:43 AM Peter Smith <smithpb2...@gmail.com> wrote:
> >
> > 7.
> > +$node_publisher->wait_for_catchup('sub_gen');
> > +
> > +is( $node_subscriber->safe_psql(
> > + 'postgres', "SELECT * FROM test_gen ORDER BY a"),
> > + qq(1|2),
> > + 'replication with generated columns in column list');
> > +
> >
> > But, this is only testing normal replication. You should also include
> > some initial table data so you can test that the initial table
> > synchronization works too. Otherwise, I think current this patch has
> > no proof that the initial 'copy_data' even works at all.
> >
>
> Per my tests, the initial copy doesn't work with 0001 alone. It needs
> changes in table sync.c from the 0002 patch. Now, we can commit 0001
> after fixing comments and mentioning in the commit message that this
> patch supports only the replication of generated columns when
> specified in the column list. The initial sync and replication of
> generated columns when not specified in the column list will be
> supported in future commits. OTOH, if the change to make table sync
> work is simple, we can even combine that change.
>If this comes to a vote, then my vote is to refactor the necessary tablesync COPY code back into patch 0001 so that patch 0001 can replicate initial data properly stand alone. Otherwise, (if we accept patch 0001 only partly works, like now) users would have to jump through hoops to get any benefit from this patch by itself. This is particularly true because the CREATE SUBSCRIPTION 'copy_data' parameter default is true, so patch 0001 is going to be broken by default if there is any pre-existing table data when publishing generated columns to default subscriptions. ====== Kind Regards, Peter Smith. Fujitsu Australia
