Tweak behavior of psql --single-transaction depending on ON_ERROR_STOP

This commit, in completion of 157f873, forces a ROLLBACK for
--single-transaction only when ON_ERROR_STOP is used when one of the
steps defined by -f/-c fails.  Hence, COMMIT is always used when
ON_ERROR_STOP is not set, ignoring the status code of the last action
taken in the set of switches specified by -c/-f (previously ROLLBACK
would have been issued even without ON_ERROR_STOP if the last step
failed, while COMMIT was issued if a step in-between failed as long as
the last step succeeded, leading to more inconsistency).

While on it, this adds much more test coverage in this area when not
using ON_ERROR_STOP with multiple switch patterns involving -c and -f
for query files, single queries and slash commands.

The behavior of ON_ERROR_STOP is arguably a bug, but there was no much
support for a backpatch to force a ROLLBACK on a step failure, so this
change is done only on HEAD for now.

Per discussion with Tom Lane and Kyotaro Horiguchi.

Discussion: https://postgr.es/m/Yqbc8bAdwnP02na4@paquier.xyz
This commit is contained in:
Michael Paquier 2022-06-15 11:24:52 +09:00
parent ba412c905a
commit a3ff08e0b0
3 changed files with 73 additions and 11 deletions

View File

@ -584,7 +584,8 @@ EOF
<application>psql</application> to issue a <command>BEGIN</command> command <application>psql</application> to issue a <command>BEGIN</command> command
before the first such option and a <command>COMMIT</command> command after before the first such option and a <command>COMMIT</command> command after
the last one, thereby wrapping all the commands into a single the last one, thereby wrapping all the commands into a single
transaction. If any of the commands fails, a transaction. If any of the commands fails and the variable
<varname>ON_ERROR_STOP</varname> was set, a
<command>ROLLBACK</command> command is sent instead. This ensures that <command>ROLLBACK</command> command is sent instead. This ensures that
either all the commands complete successfully, or no changes are either all the commands complete successfully, or no changes are
applied. applied.

View File

@ -426,8 +426,13 @@ main(int argc, char *argv[])
if (options.single_txn) if (options.single_txn)
{ {
res = PSQLexec((successResult == EXIT_SUCCESS) ? /*
"COMMIT" : "ROLLBACK"); * Rollback the contents of the single transaction if the caller
* has set ON_ERROR_STOP and one of the steps has failed. This
* check needs to match the one done a couple of lines above.
*/
res = PSQLexec((successResult != EXIT_SUCCESS && pset.on_error_stop) ?
"ROLLBACK" : "COMMIT");
if (res == NULL) if (res == NULL)
{ {
if (pset.on_error_stop) if (pset.on_error_stop)

View File

@ -204,6 +204,8 @@ like(
# query result. # query result.
my $tempdir = PostgreSQL::Test::Utils::tempdir; my $tempdir = PostgreSQL::Test::Utils::tempdir;
$node->safe_psql('postgres', "CREATE TABLE tab_psql_single (a int);"); $node->safe_psql('postgres', "CREATE TABLE tab_psql_single (a int);");
# Tests with ON_ERROR_STOP.
$node->command_ok( $node->command_ok(
[ [
'psql', '-X', 'psql', '-X',
@ -212,11 +214,12 @@ $node->command_ok(
'INSERT INTO tab_psql_single VALUES (1)', '-c', 'INSERT INTO tab_psql_single VALUES (1)', '-c',
'INSERT INTO tab_psql_single VALUES (2)' 'INSERT INTO tab_psql_single VALUES (2)'
], ],
'--single-transaction and multiple -c switches'); 'ON_ERROR_STOP, --single-transaction and multiple -c switches');
my $row_count = my $row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '2', is($row_count, '2',
'--single-transaction commits transaction, multiple -c switches'); '--single-transaction commits transaction, ON_ERROR_STOP and multiple -c switches'
);
$node->command_fails( $node->command_fails(
[ [
@ -226,11 +229,12 @@ $node->command_fails(
'INSERT INTO tab_psql_single VALUES (3)', '-c', 'INSERT INTO tab_psql_single VALUES (3)', '-c',
"\\copy tab_psql_single FROM '$tempdir/nonexistent'" "\\copy tab_psql_single FROM '$tempdir/nonexistent'"
], ],
'--single-transaction and multiple -c switches, error'); 'ON_ERROR_STOP, --single-transaction and multiple -c switches, error');
$row_count = $row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '2', is($row_count, '2',
'client-side error rolls back transaction, multiple -c switches'); 'client-side error rolls back transaction, ON_ERROR_STOP and multiple -c switches'
);
# Tests mixing files and commands. # Tests mixing files and commands.
my $copy_sql_file = "$tempdir/tab_copy.sql"; my $copy_sql_file = "$tempdir/tab_copy.sql";
@ -244,11 +248,12 @@ $node->command_ok(
'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f', 'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f',
$insert_sql_file $insert_sql_file
], ],
'--single-transaction and multiple -f switches'); 'ON_ERROR_STOP, --single-transaction and multiple -f switches');
$row_count = $row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '4', is($row_count, '4',
'--single-transaction commits transaction, multiple -f switches'); '--single-transaction commits transaction, ON_ERROR_STOP and multiple -f switches'
);
$node->command_fails( $node->command_fails(
[ [
@ -256,10 +261,61 @@ $node->command_fails(
'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f', 'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f',
$copy_sql_file $copy_sql_file
], ],
'--single-transaction and multiple -f switches, error'); 'ON_ERROR_STOP, --single-transaction and multiple -f switches, error');
$row_count = $row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '4', is($row_count, '4',
'client-side error rolls back transaction, multiple -f switches'); 'client-side error rolls back transaction, ON_ERROR_STOP and multiple -f switches'
);
# Tests without ON_ERROR_STOP.
# The last switch fails on \copy. The command returns a failure and the
# transaction commits.
$node->command_fails(
[
'psql', '-X',
'--single-transaction', '-f',
$insert_sql_file, '-f',
$insert_sql_file, '-c',
"\\copy tab_psql_single FROM '$tempdir/nonexistent'"
],
'no ON_ERROR_STOP, --single-transaction and multiple -f/-c switches');
$row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '6',
'client-side error commits transaction, no ON_ERROR_STOP and multiple -f/-c switches'
);
# The last switch fails on \copy coming from an input file. The command
# returns a success and the transaction commits.
$node->command_ok(
[
'psql', '-X', '--single-transaction', '-f',
$insert_sql_file, '-f', $insert_sql_file, '-f',
$copy_sql_file
],
'no ON_ERROR_STOP, --single-transaction and multiple -f switches');
$row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '8',
'client-side error commits transaction, no ON_ERROR_STOP and multiple -f switches'
);
# The last switch makes the command return a success, and the contents of
# the transaction commit even if there is a failure in-between.
$node->command_ok(
[
'psql', '-X',
'--single-transaction', '-c',
'INSERT INTO tab_psql_single VALUES (5)', '-f',
$copy_sql_file, '-c',
'INSERT INTO tab_psql_single VALUES (6)'
],
'no ON_ERROR_STOP, --single-transaction and multiple -c switches');
$row_count =
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
is($row_count, '10',
'client-side error commits transaction, no ON_ERROR_STOP and multiple -c switches'
);
done_testing(); done_testing();