Hi Hackers!

When using psql interactively one might be tempted to guard potentially
destructive commands such as "UPDATE / DELETE / DROP <object>" by starting
the input line with an explicit "BEGIN; ...".  This has the added benefit
that then you invoke the command by reverse-searching the command history,
you get it together with the guarding transaction open statement.

This, however, is not 100% safe as I've found out a few days ago.  Should
the connection to the server get lost, the first of semicolon-separated
statements, "BEGIN;" will only trigger connection reset, and if that is
successful the following command(s) are going to be executed on the newly
opened connection, but without the transaction guard.

I'm not the first one to discover that, a search in archives gives at least
3 results:


The second one even resulted in a TODO item:

  Prevent psql from sending remaining single-line multi-statement queries
  after reconnection

I was thinking that simply adding a bool flag in the pset struct, to
indicate that connection was reset during attempt to execute the last query
would do the trick, but it only helps in exactly the case described above.

Since this is already an improvement, I'm attaching a patch.

If on the other hand, someone is pasting into psql's terminal a block of
commands enclosed in BEGIN/COMMIT, the same bug is triggered: BEGIN doesn't
have effect and the rest of commands run outside of transaction.

Is it possible at all to protect against the latter case?  How?

From 3eae5e5d91084e0882a286bac464782701e17d21 Mon Sep 17 00:00:00 2001
From: Oleksandr Shulgin <oleksandr.shul...@zalando.de>
Date: Thu, 20 Oct 2016 12:24:48 +0200
Subject: [PATCH] psql: stop sending commands after connection reset

Previsouly an input line such as "BEGIN; UPDATE something..." could
result in UPDATE running outside of transaction if the first statement
happen to trigger connection reset.

NB: this does *not* protect from blocks of commands pasted into the
 src/bin/psql/common.c   | 2 ++
 src/bin/psql/mainloop.c | 5 ++++-
 src/bin/psql/settings.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..34a4507 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -386,6 +386,8 @@ CheckConnection(void)
+		pset.conn_was_reset = true;
 	return OK;
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..6d39ce8 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -391,11 +391,14 @@ MainLoop(FILE *source)
 			/* fall out of loop if lexer reached EOL */
-			if (scan_result == PSCAN_INCOMPLETE ||
+			if (pset.conn_was_reset ||
+				scan_result == PSCAN_INCOMPLETE ||
 				scan_result == PSCAN_EOL)
+		pset.conn_was_reset = false;
 		/* Add line to pending history if we didn't execute anything yet */
 		if (pset.cur_cmd_interactive && !line_saved_in_history)
 			pg_append_history(line, history_buf);
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..39a4be0 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -102,6 +102,9 @@ typedef struct _psqlSettings
 	FILE	   *cur_cmd_source; /* describe the status of the current main
 								 * loop */
 	bool		cur_cmd_interactive;
+	bool		conn_was_reset;	/* indicates that the connection was reset
+								 * during the last attempt to execute an
+								 * interactive command */
 	int			sversion;		/* backend server version */
 	const char *progname;		/* in case you renamed psql */
 	char	   *inputfile;		/* file being currently processed, if any */

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to