RE: pg_upgrade and logical replication

Dear Vignesh,

Thanks for reviewing! PSA new version.

>
> Thanks for the updated patch, few suggestions:
> 1) Can we use a new publication for this subscription too so that the
> publication and subscription naming will become consistent throughout
> the test case:
> +# Table will be in 'd' (data is being copied) state as table sync will fail
> +# because of primary key constraint error.
> +my $started_query =
> + "SELECT count(1) = 1 FROM pg_subscription_rel WHERE srsubstate = 'd'";
> +$old_sub->poll_query_until('postgres', $started_query)
> + or die
> + "Timed out while waiting for the table state to become 'd' (datasync)";
> +
> +# Create another subscription and drop the subscription's replication origin
> +$old_sub->safe_psql('postgres',
> + "CREATE SUBSCRIPTION regress_sub3 CONNECTION '$connstr'
> PUBLICATION regress_pub2 WITH (enabled = false)"
> +);
>
> So after the change it will become like subscription regress_sub3 for
> publication regress_pub3, subscription regress_sub4 for publication
> regress_pub4 and subscription regress_sub5 for publication
> regress_pub5.

A new publication was defined.

> 2) The tab_upgraded1 table can be created along with create
> publication and create subscription itself:
> $publisher->safe_psql('postgres',
> "CREATE PUBLICATION regress_pub3 FOR TABLE tab_upgraded1");
> $old_sub->safe_psql('postgres',
> "CREATE SUBSCRIPTION regress_sub4 CONNECTION '$connstr' PUBLICATION
> regress_pub3 WITH (failover = true)"
> );

The definition of tab_upgraded1 was moved to the place you pointed.

> 3) The tab_upgraded2 table can be created along with create
> publication and create subscription itself to keep it consistent:
> $publisher->safe_psql('postgres',
> - "ALTER PUBLICATION regress_pub2 ADD TABLE tab_upgraded2");
> + "CREATE PUBLICATION regress_pub4 FOR TABLE tab_upgraded2");
> $old_sub->safe_psql('postgres',
> - "ALTER SUBSCRIPTION regress_sub2 REFRESH PUBLICATION");
> + "CREATE SUBSCRIPTION regress_sub5 CONNECTION '$connstr'
> PUBLICATION regress_pub4"
> +);

Ditto.

> With above fixes, the following can be removed:
> # Initial setup
> $publisher->safe_psql(
> 'postgres', qq[
> CREATE TABLE tab_upgraded1(id int);
> CREATE TABLE tab_upgraded2(id int);
> ]);
> $old_sub->safe_psql(
> 'postgres', qq[
> CREATE TABLE tab_upgraded1(id int);
> CREATE TABLE tab_upgraded2(id int);
> ]);

Yes, earlier definitions were removed instead.
Also, some comments were adjusted based on these fixes.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/