On Mon, Jan 29, 2024 at 6:47 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Monday, January 29, 2024 7:30 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > > > > =================== > > 1. > > parse_subscription_options() > > { > > ... > > /* > > * We've been explicitly asked to not connect, that requires some > > * additional processing. > > */ > > if (!opts->connect && IsSet(supported_opts, SUBOPT_CONNECT)) { > > > > Here, along with other options, we need an explicit check for failover, so > > that if > > connect=false and failover=true, the statement should give error. I was > > expecting the below statement to fail but it passed with WARNING. > > postgres=# create subscription sub2 connection 'dbname=postgres' > > publication pub2 with(connect=false, failover=true); > > WARNING: subscription was created, but is not connected > > HINT: To initiate replication, you must manually create the replication > > slot, > > enable the subscription, and refresh the subscription. > > CREATE SUBSCRIPTION > > Added. >
In this regard, I feel we don't need to dump/restore the 'FAILOVER' option non-binary upgrade paths similar to the 'ENABLE' option. For binary upgrade, if the failover option is enabled, then we can enable it using Alter Subscription SET (failover=true). Let's add one test corresponding to this behavior in postgresql\src\bin\pg_upgrade\t\004_subscription. Additionally, we need to update the pg_dump docs for the 'failover' option. See "When dumping logical replication subscriptions, .." [1]. I think we also need to update the connect option docs in CREATE SUBSCRIPTION [2]. [1] - https://www.postgresql.org/docs/devel/app-pgdump.html [2] - https://www.postgresql.org/docs/devel/sql-createsubscription.html -- With Regards, Amit Kapila.