From 157f8739ad18278ca946758b2f274a8109a0aa08 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 6 Jun 2022 11:05:59 +0900 Subject: [PATCH] Fix psql's single transaction mode on client-side errors with -c/-f switches psql --single-transaction is able to handle multiple -c and -f switches in a single transaction since d5563d7d, but this had the surprising behavior of forcing a transaction COMMIT even if psql failed with an error in the client (for example incorrect path given to \copy), which would generate an error, but still commit any changes that were already applied in the backend. This commit makes the behavior more consistent, by enforcing a transaction ROLLBACK if any commands fail, both client-side and backend-side, so as no changes are applied if one error happens in any of them. Some tests are added on HEAD to provide some coverage about all that. Backend-side errors are unreliable as IPC::Run can complain on SIGPIPE if psql quits before reading a query result, but that should work properly in the case where any errors come from psql itself, which is what the original report is about. Reported-by: Christoph Berg Author: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/17504-76b68018e130415e@postgresql.org Backpatch-through: 10 --- doc/src/sgml/ref/psql-ref.sgml | 6 ++-- src/bin/psql/startup.c | 4 ++- src/bin/psql/t/001_basic.pl | 64 ++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 47bf3342a5..ababd371df 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -584,8 +584,10 @@ EOF psql to issue a BEGIN command before the first such option and a COMMIT command after the last one, thereby wrapping all the commands into a single - transaction. This ensures that either all the commands complete - successfully, or no changes are applied. + transaction. If any of the commands fails, a + ROLLBACK command is sent instead. This ensures that + either all the commands complete successfully, or no changes are + applied. diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index ddff903915..574f49d4c9 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -426,7 +426,9 @@ main(int argc, char *argv[]) if (options.single_txn) { - if ((res = PSQLexec("COMMIT")) == NULL) + res = PSQLexec((successResult == EXIT_SUCCESS) ? + "COMMIT" : "ROLLBACK"); + if (res == NULL) { if (pset.on_error_stop) { diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index c3ed18e84d..d7e20f0ac6 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -198,4 +198,68 @@ like( ^LOCATION: .*$/m, '\errverbose after \gdesc with error'); +# Check behavior when using multiple -c and -f switches. +# Note that we cannot test backend-side errors as tests are unstable in this +# case: IPC::Run can complain about a SIGPIPE if psql quits before reading a +# query result. +my $tempdir = PostgreSQL::Test::Utils::tempdir; +$node->safe_psql('postgres', "CREATE TABLE tab_psql_single (a int);"); +$node->command_ok( + [ + 'psql', '-X', + '--single-transaction', '-v', + 'ON_ERROR_STOP=1', '-c', + 'INSERT INTO tab_psql_single VALUES (1)', '-c', + 'INSERT INTO tab_psql_single VALUES (2)' + ], + '--single-transaction and multiple -c switches'); +my $row_count = + $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); +is($row_count, '2', + '--single-transaction commits transaction, multiple -c switches'); + +$node->command_fails( + [ + 'psql', '-X', + '--single-transaction', '-v', + 'ON_ERROR_STOP=1', '-c', + 'INSERT INTO tab_psql_single VALUES (3)', '-c', + "\\copy tab_psql_single FROM '$tempdir/nonexistent'" + ], + '--single-transaction and multiple -c switches, error'); +$row_count = + $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); +is($row_count, '2', + 'client-side error rolls back transaction, multiple -c switches'); + +# Tests mixing files and commands. +my $copy_sql_file = "$tempdir/tab_copy.sql"; +my $insert_sql_file = "$tempdir/tab_insert.sql"; +append_to_file($copy_sql_file, + "\\copy tab_psql_single FROM '$tempdir/nonexistent';"); +append_to_file($insert_sql_file, 'INSERT INTO tab_psql_single VALUES (4);'); +$node->command_ok( + [ + 'psql', '-X', '--single-transaction', '-v', + 'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f', + $insert_sql_file + ], + '--single-transaction and multiple -f switches'); +$row_count = + $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); +is($row_count, '4', + '--single-transaction commits transaction, multiple -f switches'); + +$node->command_fails( + [ + 'psql', '-X', '--single-transaction', '-v', + 'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f', + $copy_sql_file + ], + '--single-transaction and multiple -f switches, error'); +$row_count = + $node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single'); +is($row_count, '4', + 'client-side error rolls back transaction, multiple -f switches'); + done_testing(); -- 2.39.5