Hi, On Thu, Jan 25, 2024 at 02:57:30AM +0000, Zhijie Hou (Fujitsu) wrote: > On Wednesday, January 24, 2024 6:31 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > > > On Tue, Jan 23, 2024 at 5:13 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > > > Thanks Ajin for testing the patch. PFA v66 which fixes this issue. > > > > > > > I think we should try to commit the patch as all of the design concerns are > > resolved now. To achieve that, can we split the failover setting patch into > > the > > following: (a) setting failover property via SQL commands and display it in > > pg_replication_slots (b) replication protocol command (c) failover property > > via > > subscription commands? > > > > It will make each patch smaller and it would be easier to detect any > > problem in > > the same after commit. > > Agreed. I split the original 0001 patch into 3 patches as suggested. > Here is the V68 patch set.
Thanks! Some comments. Looking at 0002: 1 === + <para>The following options are supported:</para> What about "The following option is supported"? (as currently only the "FAILOVER" is) 2 === What about adding some TAP tests too? (I can see that ALTER_REPLICATION_SLOT test is added in v68-0004 but I think having some in 0002 would make sense too). Looking at 0003: 1 === + parameter specified in the subscription. When creating the slot, + ensure the slot <literal>failover</literal> property matches the + <link linkend="sql-createsubscription-params-with-failover"><literal>failover</literal></link> + parameter value of the subscription. What about explaining what would be the consequence of not doing so? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
