On Wed, Apr 2, 2025 at 12:41 PM Amit Kapila wrote:
>
> On Tue, Apr 1, 2025 at 4:28 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com>
> wrote:
> >
> > Here is the V3 patch set which addressed all the comments.
> >
>
> Comment 0n 0001
> <literal>NULL</literal> for logical slots where
> + <structfield>two_phase</structfield> is false and physical slots.
> + </para></entry>
>
> change above to:
> <literal>NULL</literal> for logical slots where
> <structfield>two_phase</structfield> is false and for physical slots.
Changed.
>
> Comment on 0002
> +# Create a subscription with two_phase enabled
> +$subscriber1->safe_psql('postgres',
> + "CREATE SUBSCRIPTION regress_mysub2 CONNECTION
> '$publisher_connstr'
> PUBLICATION regress_mypub WITH (slot_name = lsub1_slot, create_slot =
> false, enabled = false, two_phase = true);"
> +);
> +
> +# Enable failover for the subscription ($result, $stdout, $stderr) =
> +$subscriber1->psql('postgres', "ALTER SUBSCRIPTION regress_mysub2
> +SET (failover = true)"); ok( $stderr =~
> +/ERROR: cannot enable failover for a two_phase
> enabled subscription/,
> + "Enabling failover is not allowed for a two_phase enabled
> + subscription");
>
> Is there a need for this test to be in .pl file? Can't we add it in .sql file?
Right, I moved the test into subscription.sql
> Apart from the above, I have made minor modifications to the PG17
> patch in the attached.
Here is V4 patch set which addressed above comments and passed
pgindent test.
Best Regards,
Hou zj
