Dagfinn Ilmari Mannsåker <ilm...@ilmari.org> writes: > Peter Smith <smithpb2...@gmail.com> writes: > >> On Thu, Dec 12, 2024 at 2:53 PM Michael Paquier <mich...@paquier.xyz> wrote: >> ... >> >>> > So, AFAICT I can workaround the perltidy wrapping just by putting all >>> > the noarg options at the bottom of the command, then all the >>> > option/optarg pairs (ie 2s) will stay together. I can post another >>> > patch to do it this way unless you think it is too hacky. >>> >>> This trick works for me if that makes the long list of option easier >>> to read. With two elements of the array perl line, I would just put >>> some --dry-run or --verbose at the end of their respective arrays. >>> -- >>> Michael >> >> Hi Michael. >> >> Yes, that is the workaround that I was proposing. > > A better option, IMO, is to use the fat comma (=>) between options and > their values. This makes it clear both to humans and perltidy that they > belong together, and we can put all the valueless options first without > things being rewrapped.
Here's a more thorough patch, that also applies the fat comma treatment to other pg_createsubscriber invocations in the same file that don't currently happen to be mangled by perltidy. It also adds trailing commas to the last item in multi-line command arrays, which is common perl style. - ilmari
>From 953d0c8ca8202d6f53af833fd53e3f6b1929fa77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org> Date: Thu, 12 Dec 2024 11:56:15 +0000 Subject: [PATCH v4] Modify wrapping to make command options easier to read Use fat comma after options that take values, both to make it clearer to humans, and to stop perltidy from re-wrapping them. Also remove pointless quoting of $PostgreSQL::Test::Utils::timeout_default. --- .../t/040_pg_createsubscriber.pl | 255 +++++++++--------- 1 file changed, 135 insertions(+), 120 deletions(-) diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 0a900edb656..369846db0d0 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -46,69 +46,75 @@ sub generate_db command_fails(['pg_createsubscriber'], 'no subscriber data directory specified'); command_fails( - [ 'pg_createsubscriber', '--pgdata', $datadir ], + [ 'pg_createsubscriber', '--pgdata' => $datadir ], 'no publisher connection string specified'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', ], 'no database name specified'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432', - '--database', 'pg1', - '--database', 'pg1' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', + '--database' => 'pg1', + '--database' => 'pg1', ], 'duplicate database name'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432', - '--publication', 'foo1', - '--publication', 'foo1', - '--database', 'pg1', - '--database', 'pg2' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', + '--publication' => 'foo1', + '--publication' => 'foo1', + '--database' => 'pg1', + '--database' => 'pg2', ], 'duplicate publication name'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432', - '--publication', 'foo1', - '--database', 'pg1', - '--database', 'pg2' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', + '--publication' => 'foo1', + '--database' => 'pg1', + '--database' => 'pg2', ], 'wrong number of publication names'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432', - '--publication', 'foo1', - '--publication', 'foo2', - '--subscription', 'bar1', - '--database', 'pg1', - '--database', 'pg2' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', + '--publication' => 'foo1', + '--publication' => 'foo2', + '--subscription' => 'bar1', + '--database' => 'pg1', + '--database' => 'pg2', ], 'wrong number of subscription names'); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $datadir, - '--publisher-server', 'port=5432', - '--publication', 'foo1', - '--publication', 'foo2', - '--subscription', 'bar1', - '--subscription', 'bar2', - '--replication-slot', 'baz1', - '--database', 'pg1', - '--database', 'pg2' + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $datadir, + '--publisher-server' => 'port=5432', + '--publication' => 'foo1', + '--publication' => 'foo2', + '--subscription' => 'bar1', + '--subscription' => 'bar2', + '--replication-slot' => 'baz1', + '--database' => 'pg1', + '--database' => 'pg2', ], 'wrong number of replication slot names'); @@ -168,41 +174,44 @@ sub generate_db # Run pg_createsubscriber on a promoted server command_fails( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_t->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_t->host, '--subscriber-port', - $node_t->port, '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_t->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_t->host, + '--subscriber-port' => $node_t->port, + '--database' => $db1, + '--database' => $db2, ], 'target server is not in recovery'); # Run pg_createsubscriber when standby is running command_fails( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--database' => $db1, + '--database' => $db2, ], 'standby is up and running'); # Run pg_createsubscriber on about-to-fail node F command_fails( [ - 'pg_createsubscriber', '--verbose', - '--pgdata', $node_f->data_dir, - '--publisher-server', $node_p->connstr($db1), - '--socketdir', $node_f->host, - '--subscriber-port', $node_f->port, - '--database', $db1, - '--database', $db2 + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $node_f->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_f->host, + '--subscriber-port' => $node_f->port, + '--database' => $db1, + '--database' => $db2 ], 'subscriber data directory is not a copy of the source database cluster'); @@ -216,14 +225,15 @@ sub generate_db # Run pg_createsubscriber on node C (P -> S -> C) command_fails( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_c->data_dir, '--publisher-server', - $node_s->connstr($db1), '--socketdir', - $node_c->host, '--subscriber-port', - $node_c->port, '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_c->data_dir, + '--publisher-server' => $node_s->connstr($db1), + '--socketdir' => $node_c->host, + '--subscriber-port' => $node_c->port, + '--database' => $db1, + '--database' => $db2, ], 'primary server is in recovery'); @@ -239,14 +249,16 @@ sub generate_db $node_s->stop; command_fails( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--database' => $db1, + '--database' => $db2, + ], 'primary contains unmet conditions on node P'); # Restore default settings here but only apply it after testing standby. Some @@ -268,14 +280,15 @@ sub generate_db }); command_fails( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--database' => $db1, + '--database' => $db2, ], 'standby contains unmet conditions on node S'); $node_s->append_conf( @@ -321,19 +334,20 @@ sub generate_db # dry run mode on node S command_ok( [ - 'pg_createsubscriber', '--verbose', - '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default", - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--publication', - 'pub1', '--publication', - 'pub2', '--subscription', - 'sub1', '--subscription', - 'sub2', '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default, + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--publication' => 'pub1', + '--publication' => 'pub2', + '--subscription' => 'sub1', + '--subscription' => 'sub2', + '--database' => $db1, + '--database' => $db2, ], 'run pg_createsubscriber --dry-run on node S'); @@ -346,32 +360,33 @@ sub generate_db # pg_createsubscriber can run without --databases option command_ok( [ - 'pg_createsubscriber', '--verbose', - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--replication-slot', - 'replslot1' + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--replication-slot' => 'replslot1', ], 'run pg_createsubscriber without --databases'); # Run pg_createsubscriber on node S command_ok( [ - 'pg_createsubscriber', '--verbose', - '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default", - '--verbose', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr($db1), '--socketdir', - $node_s->host, '--subscriber-port', - $node_s->port, '--publication', - 'pub1', '--publication', - 'Pub2', '--replication-slot', - 'replslot1', '--replication-slot', - 'replslot2', '--database', - $db1, '--database', - $db2 + 'pg_createsubscriber', + '--verbose', + '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default, + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--publication' => 'pub1', + '--publication' => 'pub2', + '--replication-slot' => 'replslot1', + '--replication-slot' => 'replslot2', + '--database' => $db1, + '--database' => $db2, ], 'run pg_createsubscriber on node S'); -- 2.47.1
