Re: src/bin/pg_upgrade/t/004_subscription.pl test comment fix - Mailing list pgsql-hackers

On Wed, Jan 31, 2024 at 7:48 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> How about rewording it more extensively?  It doesn't read great IMO.
> I would use something like
>
> # In the upgraded instance, the running status and failover option of the
> # subscription with the failover option should have been preserved; the other
> # should not.
> # So regress_sub1 should still have subenabled,subfailover set to true,
> # while regress_sub2 should have both set to false.
>

IIUC this suggested comment is implying that the running status is
*only* preserved when the failover option is true. But AFAIK that is
not correct. e.g. I hacked the test to keep subscription regress_sub2
as ENABLED but the result after the upgrade was subenabled/subfailover
= t/f, not f/f.

> I think the symmetry between the two lines confuses more than helps.
> It's not a huge thing but since we're editing anyway, why not?
>

OK. Now using your suggested 2nd sentence:

+# The subscription's running status and failover option should be preserved
+# in the upgraded instance. So regress_sub1 should still have
subenabled,subfailover
+# set to true, while regress_sub2 should have both set to false.

~

I also tweaked some other nearby comments/messages which did not
mention the 'failover' preservation.

PSA v2.

======
Kind Regards,
Peter Smith.
Fujitsu Australia