Hi all, This is a follow-up of the discussion which happened here after Tom has committed fb57f40: https://www.postgresql.org/message-id/20190828012439.ga1...@paquier.xyz
I have reviewed the TAP tests, and we have much more spots where it is better to use PostgresNode::safe_psql instead PostgresNode::psql so as the test dies immediately if there is a failure on a query which should never fail. Attached are the spots I have found. Any thoughts or opinions? Thanks, -- Michael
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl index 6e2f051187..5f40fcfba6 100644 --- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl +++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl @@ -47,12 +47,12 @@ is($primary->slot($slot_name)->{'slot_type'}, # Generate some WAL. Use --synchronous at the same time to add more # code coverage. Switch to the next segment first so that subsequent # restarts of pg_receivewal will see this segment as full.. -$primary->psql('postgres', 'CREATE TABLE test_table(x integer);'); -$primary->psql('postgres', 'SELECT pg_switch_wal();'); +$primary->safe_psql('postgres', 'CREATE TABLE test_table(x integer);'); +$primary->safe_psql('postgres', 'SELECT pg_switch_wal();'); my $nextlsn = $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();'); chomp($nextlsn); -$primary->psql('postgres', +$primary->safe_psql('postgres', 'INSERT INTO test_table VALUES (generate_series(1,100));'); # Stream up to the given position. diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl index 99154bcf39..b225b3b4ee 100644 --- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl +++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl @@ -47,8 +47,8 @@ $node->command_ok( my $slot = $node->slot('test'); isnt($slot->{'restart_lsn'}, '', 'restart lsn is defined for new slot'); -$node->psql('postgres', 'CREATE TABLE test_table(x integer)'); -$node->psql('postgres', +$node->safe_psql('postgres', 'CREATE TABLE test_table(x integer)'); +$node->safe_psql('postgres', 'INSERT INTO test_table(x) SELECT y FROM generate_series(1, 10) a(y);'); my $nextlsn = $node->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index f064ea1063..3e7412dbbf 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -3282,7 +3282,7 @@ if ($collation_check_stderr !~ /ERROR: /) } # Create a second database for certain tests to work against -$node->psql('postgres', 'create database regress_pg_dump_test;'); +$node->safe_psql('postgres', 'create database regress_pg_dump_test;'); # Start with number of command_fails_like()*2 tests below (each # command_fails_like is actually 2 tests) diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index 3c743d7d7c..42af151ac9 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -125,7 +125,7 @@ test_target_session_attrs($node_standby_1, $node_master, $node_standby_1, # role. note "testing SHOW commands for replication connection"; -$node_master->psql( +$node_master->safe_psql( 'postgres', " CREATE ROLE repl_role REPLICATION LOGIN; GRANT pg_read_all_settings TO repl_role;"); diff --git a/src/test/recovery/t/008_fsm_truncation.pl b/src/test/recovery/t/008_fsm_truncation.pl index ddab464a97..a164b81777 100644 --- a/src/test/recovery/t/008_fsm_truncation.pl +++ b/src/test/recovery/t/008_fsm_truncation.pl @@ -30,7 +30,7 @@ $node_standby->init_from_backup($node_master, 'master_backup', has_streaming => 1); $node_standby->start; -$node_master->psql( +$node_master->safe_psql( 'postgres', qq{ create table testtab (a int, b char(100)); insert into testtab select generate_series(1,1000), 'foo'; @@ -39,7 +39,7 @@ delete from testtab where ctid > '(8,0)'; }); # Take a lock on the table to prevent following vacuum from truncating it -$node_master->psql( +$node_master->safe_psql( 'postgres', qq{ begin; lock table testtab in row share mode; @@ -47,14 +47,14 @@ prepare transaction 'p1'; }); # Vacuum, update FSM without truncation -$node_master->psql('postgres', 'vacuum verbose testtab'); +$node_master->safe_psql('postgres', 'vacuum verbose testtab'); # Force a checkpoint -$node_master->psql('postgres', 'checkpoint'); +$node_master->safe_psql('postgres', 'checkpoint'); # Now do some more insert/deletes, another vacuum to ensure full-page writes # are done -$node_master->psql( +$node_master->safe_psql( 'postgres', qq{ insert into testtab select generate_series(1,1000), 'foo'; delete from testtab where ctid > '(8,0)'; @@ -62,16 +62,16 @@ vacuum verbose testtab; }); # Ensure all buffers are now clean on the standby -$node_standby->psql('postgres', 'checkpoint'); +$node_standby->safe_psql('postgres', 'checkpoint'); # Release the lock, vacuum again which should lead to truncation -$node_master->psql( +$node_master->safe_psql( 'postgres', qq{ rollback prepared 'p1'; vacuum verbose testtab; }); -$node_master->psql('postgres', 'checkpoint'); +$node_master->safe_psql('postgres', 'checkpoint'); my $until_lsn = $node_master->safe_psql('postgres', "SELECT pg_current_wal_lsn();"); @@ -83,7 +83,7 @@ $node_standby->poll_query_until('postgres', $caughtup_query) # Promote the standby $node_standby->promote; -$node_standby->psql('postgres', 'checkpoint'); +$node_standby->safe_psql('postgres', 'checkpoint'); # Restart to discard in-memory copy of FSM $node_standby->restart; diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl index 1b748ad857..88d2eee3f9 100644 --- a/src/test/recovery/t/009_twophase.pl +++ b/src/test/recovery/t/009_twophase.pl @@ -52,7 +52,8 @@ my ($cur_master, $cur_standby) = ($node_london, $node_paris); my $cur_master_name = $cur_master->name; # Create table we'll use in the test transactions -$cur_master->psql('postgres', "CREATE TABLE t_009_tbl (id int, msg text)"); +$cur_master->safe_psql('postgres', + "CREATE TABLE t_009_tbl (id int, msg text)"); ############################################################################### # Check that we can commit and abort transaction after soft restart. @@ -61,7 +62,7 @@ $cur_master->psql('postgres', "CREATE TABLE t_009_tbl (id int, msg text)"); # files. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (1, 'issued to ${cur_master_name}'); @@ -88,7 +89,7 @@ is($psql_rc, '0', 'Rollback prepared transaction after restart'); # transaction using dedicated WAL records. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " CHECKPOINT; BEGIN; @@ -114,7 +115,7 @@ is($psql_rc, '0', 'Rollback prepared transaction after teardown'); # Check that WAL replay can handle several transactions with same GID name. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " CHECKPOINT; BEGIN; @@ -139,7 +140,7 @@ is($psql_rc, '0', 'Replay several transactions with same GID'); # while replaying transaction commits. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (13, 'issued to ${cur_master_name}'); @@ -160,13 +161,13 @@ $psql_rc = $cur_master->psql( PREPARE TRANSACTION 'xact_009_7';"); is($psql_rc, '0', "Cleanup of shared memory state for 2PC commit"); -$cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_7'"); +$cur_master->safe_psql('postgres', "COMMIT PREPARED 'xact_009_7'"); ############################################################################### # Check that WAL replay will cleanup its shared memory state on running standby. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (17, 'issued to ${cur_master_name}'); @@ -186,15 +187,15 @@ is($psql_out, '0', # prepare and commit to use on-disk twophase files. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (19, 'issued to ${cur_master_name}'); SAVEPOINT s1; INSERT INTO t_009_tbl VALUES (20, 'issued to ${cur_master_name}'); PREPARE TRANSACTION 'xact_009_9';"); -$cur_standby->psql('postgres', "CHECKPOINT"); -$cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_9'"); +$cur_standby->safe_psql('postgres', "CHECKPOINT"); +$cur_master->safe_psql('postgres', "COMMIT PREPARED 'xact_009_9'"); $cur_standby->psql( 'postgres', "SELECT count(*) FROM pg_prepared_xacts", @@ -206,7 +207,7 @@ is($psql_out, '0', # Check that prepared transactions can be committed on promoted standby. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (21, 'issued to ${cur_master_name}'); @@ -238,7 +239,7 @@ $cur_standby->start; # consistent. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (23, 'issued to ${cur_master_name}'); @@ -265,14 +266,14 @@ is($psql_out, '1', $cur_standby->enable_streaming($cur_master); $cur_standby->start; -$cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_11'"); +$cur_master->safe_psql('postgres', "COMMIT PREPARED 'xact_009_11'"); ############################################################################### # Check that prepared transactions are correctly replayed after standby hard # restart while master is down. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; INSERT INTO t_009_tbl VALUES (25, 'issued to ${cur_master_name}'); @@ -308,7 +309,7 @@ $cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_12'"); # replay of XLOG_STANDBY_LOCK wal record. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; CREATE TABLE t_009_tbl2 (id int, msg text); @@ -339,7 +340,7 @@ is($psql_out, '1', "Replay prepared transaction with DDL"); # of the master. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; CREATE TABLE t_009_tbl3 (id int, msg text); @@ -366,7 +367,7 @@ is($psql_rc, '0', 'Rollback prepared transaction after teardown'); # of the master. ############################################################################### -$cur_master->psql( +$cur_master->safe_psql( 'postgres', " BEGIN; CREATE TABLE t_009_tbl5 (id int, msg text); diff --git a/src/test/recovery/t/012_subtransactions.pl b/src/test/recovery/t/012_subtransactions.pl index 292cd40fe2..ffc1e91e50 100644 --- a/src/test/recovery/t/012_subtransactions.pl +++ b/src/test/recovery/t/012_subtransactions.pl @@ -16,7 +16,7 @@ $node_master->append_conf( )); $node_master->start; $node_master->backup('master_backup'); -$node_master->psql('postgres', "CREATE TABLE t_012_tbl (id int)"); +$node_master->safe_psql('postgres', "CREATE TABLE t_012_tbl (id int)"); # Setup standby node my $node_standby = get_new_node('standby'); @@ -29,7 +29,7 @@ $node_master->append_conf( 'postgresql.conf', qq( synchronous_standby_names = '*' )); -$node_master->psql('postgres', "SELECT pg_reload_conf()"); +$node_master->safe_psql('postgres', "SELECT pg_reload_conf()"); my $psql_out = ''; my $psql_rc = ''; @@ -39,7 +39,7 @@ my $psql_rc = ''; # so that it won't conflict with savepoint xids. ############################################################################### -$node_master->psql( +$node_master->safe_psql( 'postgres', " BEGIN; DELETE FROM t_012_tbl; @@ -59,7 +59,7 @@ $node_master->psql( $node_master->stop; $node_master->start; -$node_master->psql( +$node_master->safe_psql( 'postgres', " -- here we can get xid of previous savepoint if nextXid -- wasn't properly advanced @@ -79,10 +79,10 @@ is($psql_out, '6', "Check nextXid handling for prepared subtransactions"); # PGPROC_MAX_CACHED_SUBXIDS subtransactions and also show data properly # on promotion ############################################################################### -$node_master->psql('postgres', "DELETE FROM t_012_tbl"); +$node_master->safe_psql('postgres', "DELETE FROM t_012_tbl"); # Function borrowed from src/test/regress/sql/hs_primary_extremes.sql -$node_master->psql( +$node_master->safe_psql( 'postgres', " CREATE OR REPLACE FUNCTION hs_subxids (n integer) RETURNS void @@ -95,7 +95,7 @@ $node_master->psql( RETURN; EXCEPTION WHEN raise_exception THEN NULL; END; \$\$;"); -$node_master->psql( +$node_master->safe_psql( 'postgres', " BEGIN; SELECT hs_subxids(127); @@ -126,10 +126,10 @@ $node_standby->psql( stdout => \$psql_out); is($psql_out, '8128', "Visible"); -$node_master->psql('postgres', "DELETE FROM t_012_tbl"); +$node_master->safe_psql('postgres', "DELETE FROM t_012_tbl"); # Function borrowed from src/test/regress/sql/hs_primary_extremes.sql -$node_master->psql( +$node_master->safe_psql( 'postgres', " CREATE OR REPLACE FUNCTION hs_subxids (n integer) RETURNS void @@ -142,7 +142,7 @@ $node_master->psql( RETURN; EXCEPTION WHEN raise_exception THEN NULL; END; \$\$;"); -$node_master->psql( +$node_master->safe_psql( 'postgres', " BEGIN; SELECT hs_subxids(127); @@ -178,7 +178,7 @@ $node_master->psql( stdout => \$psql_out); is($psql_out, '8128', "Visible"); -$node_master->psql('postgres', "DELETE FROM t_012_tbl"); +$node_master->safe_psql('postgres', "DELETE FROM t_012_tbl"); $node_master->psql( 'postgres', " BEGIN;
signature.asc
Description: PGP signature