On Sat, Mar 15, 2025 at 03:26:10PM +0900, Michael Paquier wrote:
> Thanks for digging into all that. Agreed that it would be nice to
> apply a more consistent style for this release. I'll look more into
> what you have here.
The changes in pg_dump's 010_dump_connstr.pl read the same.
extension_schema in test_pg_dump adds silently a --no-sync.
> Hmm. I can partially get behind this one, seeing things like that in
> what you have sent:
>
> @@ -24,8 +24,8 @@ my $node = PostgreSQL::Test::Cluster->new('primary');
> # Make sure pg_hba.conf is set up to allow connections from backupuser.
> # This is only needed on Windows machines that don't use UNIX sockets.
> $node->init(
> - 'allows_streaming' => 1,
> - 'auth_extra' => [ '--create-role' => 'backupuser' ]);
> + allows_streaming => 1,
> + auth_extra => [ '--create-role' => 'backupuser' ]);
>
> This option handling style is inconsistent in the tree. Most of the
> time these keywords are not quoted when it comes to our internal test
> modules..
So even for the option handling, what we have here is a mixed bad of
inconsistencies and rather consistent-ish behaviors.
Some notes:
src/bin/pg_basebackup/t/010_pg_basebackup.pl quotes tablespace_map.
src/bin/pg_basebackup/t/011_in_place_tablespace.pl quotes allows_streaming.
slot_type and restart_lsn are quoted everywhere now.
src/bin/pg_combinebackup/t/008_promote.pl quotes has_streaming.
ENV is a mixed bag of various things, single, double or no quotes.
pg_verifybackup/t/003_corruption.pl, 008, 009, 010 are a set of local
changes.
001_uri.pl was inconsistent, still local.
The parts about auth_extra are inconsistent (like in
002_connection_limits.pl).
Inconsistency in BackgroundPsql.pm, but it's always quoted now.
021_row_visibility.pl and 032_relfilenode_reuse.pl are local. Hm, not
planning to bother here.
Okay for 003_sslinfo.pl and 002_scram.pl, that mix both styles,
unquoting looks fine.
Not sure about the build scripts as a whole.
One point that applies for sure is that some existing options use an
inconsistent style across the tree in the TAP tests, mainly for init()
and init_from_backup(). I've extracted these from 0002, and applied
the subset. For the rest, if there are more opinions, feel free..
--
Michael
