Skip to content

Commit 157f873

Browse files
michaelpqmichaelpq
committed
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 d5563d7, 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
1 parent f00a4f0 commit 157f873

File tree

3 files changed

+71
-3
lines changed

3 files changed

+71
-3
lines changed

‎doc/src/sgml/ref/psql-ref.sgml

+4-2
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,10 @@ EOF
584584
<application>psql</application> to issue a <command>BEGIN</command> command
585585
before the first such option and a <command>COMMIT</command> command after
586586
the last one, thereby wrapping all the commands into a single
587-
transaction. This ensures that either all the commands complete
588-
successfully, or no changes are applied.
587+
transaction. If any of the commands fails, a
588+
<command>ROLLBACK</command> command is sent instead. This ensures that
589+
either all the commands complete successfully, or no changes are
590+
applied.
589591
</para>
590592

591593
<para>

‎src/bin/psql/startup.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ main(int argc, char *argv[])
426426

427427
if (options.single_txn)
428428
{
429-
if ((res = PSQLexec("COMMIT")) == NULL)
429+
res = PSQLexec((successResult == EXIT_SUCCESS) ?
430+
"COMMIT" : "ROLLBACK");
431+
if (res == NULL)
430432
{
431433
if (pset.on_error_stop)
432434
{

‎src/bin/psql/t/001_basic.pl

+64
Original file line numberDiff line numberDiff line change
@@ -198,4 +198,68 @@ sub psql_fails_like
198198
^LOCATION: .*$/m,
199199
'\errverbose after \gdesc with error');
200200

201+
# Check behavior when using multiple -c and -f switches.
202+
# Note that we cannot test backend-side errors as tests are unstable in this
203+
# case: IPC::Run can complain about a SIGPIPE if psql quits before reading a
204+
# query result.
205+
my $tempdir = PostgreSQL::Test::Utils::tempdir;
206+
$node->safe_psql('postgres', "CREATE TABLE tab_psql_single (a int);");
207+
$node->command_ok(
208+
[
209+
'psql', '-X',
210+
'--single-transaction', '-v',
211+
'ON_ERROR_STOP=1', '-c',
212+
'INSERT INTO tab_psql_single VALUES (1)', '-c',
213+
'INSERT INTO tab_psql_single VALUES (2)'
214+
],
215+
'--single-transaction and multiple -c switches');
216+
my $row_count =
217+
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
218+
is($row_count, '2',
219+
'--single-transaction commits transaction, multiple -c switches');
220+
221+
$node->command_fails(
222+
[
223+
'psql', '-X',
224+
'--single-transaction', '-v',
225+
'ON_ERROR_STOP=1', '-c',
226+
'INSERT INTO tab_psql_single VALUES (3)', '-c',
227+
"\\copy tab_psql_single FROM '$tempdir/nonexistent'"
228+
],
229+
'--single-transaction and multiple -c switches, error');
230+
$row_count =
231+
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
232+
is($row_count, '2',
233+
'client-side error rolls back transaction, multiple -c switches');
234+
235+
# Tests mixing files and commands.
236+
my $copy_sql_file = "$tempdir/tab_copy.sql";
237+
my $insert_sql_file = "$tempdir/tab_insert.sql";
238+
append_to_file($copy_sql_file,
239+
"\\copy tab_psql_single FROM '$tempdir/nonexistent';");
240+
append_to_file($insert_sql_file, 'INSERT INTO tab_psql_single VALUES (4);');
241+
$node->command_ok(
242+
[
243+
'psql', '-X', '--single-transaction', '-v',
244+
'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f',
245+
$insert_sql_file
246+
],
247+
'--single-transaction and multiple -f switches');
248+
$row_count =
249+
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
250+
is($row_count, '4',
251+
'--single-transaction commits transaction, multiple -f switches');
252+
253+
$node->command_fails(
254+
[
255+
'psql', '-X', '--single-transaction', '-v',
256+
'ON_ERROR_STOP=1', '-f', $insert_sql_file, '-f',
257+
$copy_sql_file
258+
],
259+
'--single-transaction and multiple -f switches, error');
260+
$row_count =
261+
$node->safe_psql('postgres', 'SELECT count(*) FROM tab_psql_single');
262+
is($row_count, '4',
263+
'client-side error rolls back transaction, multiple -f switches');
264+
201265
done_testing();

0 commit comments

Comments
 (0)