Fix handling of savepoint commands within multi-statement Query strings.

Issuing a savepoint-related command in a Query message that contains
multiple SQL statements led to a FATAL exit with a complaint about
"unexpected state STARTED".  This is a shortcoming of commit 4f896dac1,
which attempted to prevent such misbehaviors in multi-statement strings;
its quick hack of marking the individual statements as "not top-level"
does the wrong thing in this case, and isn't a very accurate description
of the situation anyway.

To fix, let's introduce into xact.c an explicit model of what happens for
multi-statement Query strings.  This is an "implicit transaction block
in progress" state, which for many purposes works like the normal
TBLOCK_INPROGRESS state --- in particular, IsTransactionBlock returns true,
causing the desired result that PreventTransactionChain will throw error.
But in case of error abort it works like TBLOCK_STARTED, allowing the
transaction to be cancelled without need for an explicit ROLLBACK command.

Commit 4f896dac1 is reverted in toto, so that we go back to treating the
individual statements as "top level".  We could have left it as-is, but
this allows sharpening the error message for PreventTransactionChain
calls inside functions.

Except for getting a normal error instead of a FATAL exit for savepoint
commands, this patch should result in no user-visible behavioral change
(other than that one error message rewording).  There are some things
we might want to do in the line of changing the appearance or wording of
error and warning messages around this behavior, which would be much
simpler to do now that it's an explicitly modeled state.  But I haven't
done them here.

Although this fixes a long-standing bug, no backpatch.  The consequences
of the bug don't seem severe enough to justify the risk that this commit
itself creates some new issue.

Patch by me, but it owes something to previous investigation by
Takayuki Tsunakawa, who also reported the bug in the first place.
Also thanks to Michael Paquier for reviewing.

Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F6BE40D@G01JPEXMBYT05

2 files changed, 138 insertions, 0 deletions

diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index d9b702d016c..a7fdcf45fda 100644
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -659,6 +659,90 @@ ERROR:  portal "ctt" cannot be run
 COMMIT;
 DROP FUNCTION create_temp_tab();
 DROP FUNCTION invert(x float8);
+-- Test assorted behaviors around the implicit transaction block created
+-- when multiple SQL commands are sent in a single Query message.  These
+-- tests rely on the fact that psql will not break SQL commands apart at a
+-- backslash-quoted semicolon, but will send them as one Query.
+create temp table i_table (f1 int);
+-- psql will show only the last result in a multi-statement Query
+SELECT 1\; SELECT 2\; SELECT 3;
+ ?column? 
+----------
+        3
+(1 row)
+
+-- this implicitly commits:
+insert into i_table values(1)\; select * from i_table;
+ f1 
+----
+  1
+(1 row)
+
+-- 1/0 error will cause rolling back the whole implicit transaction
+insert into i_table values(2)\; select * from i_table\; select 1/0;
+ERROR:  division by zero
+select * from i_table;
+ f1 
+----
+  1
+(1 row)
+
+rollback;  -- we are not in a transaction at this point
+WARNING:  there is no transaction in progress
+-- can use regular begin/commit/rollback within a single Query
+begin\; insert into i_table values(3)\; commit;
+rollback;  -- we are not in a transaction at this point
+WARNING:  there is no transaction in progress
+begin\; insert into i_table values(4)\; rollback;
+rollback;  -- we are not in a transaction at this point
+WARNING:  there is no transaction in progress
+-- begin converts implicit transaction into a regular one that
+-- can extend past the end of the Query
+select 1\; begin\; insert into i_table values(5);
+commit;
+select 1\; begin\; insert into i_table values(6);
+rollback;
+-- commit in implicit-transaction state commits but issues a warning.
+insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0;
+WARNING:  there is no transaction in progress
+ERROR:  division by zero
+-- similarly, rollback aborts but issues a warning.
+insert into i_table values(9)\; rollback\; select 2;
+WARNING:  there is no transaction in progress
+ ?column? 
+----------
+        2
+(1 row)
+
+select * from i_table;
+ f1 
+----
+  1
+  3
+  5
+  7
+(4 rows)
+
+rollback;  -- we are not in a transaction at this point
+WARNING:  there is no transaction in progress
+-- implicit transaction block is still a transaction block, for e.g. VACUUM
+SELECT 1\; VACUUM;
+ERROR:  VACUUM cannot run inside a transaction block
+SELECT 1\; COMMIT\; VACUUM;
+WARNING:  there is no transaction in progress
+ERROR:  VACUUM cannot run inside a transaction block
+-- we disallow savepoint-related commands in implicit-transaction state
+SELECT 1\; SAVEPOINT sp;
+ERROR:  SAVEPOINT can only be used in transaction blocks
+SELECT 1\; COMMIT\; SAVEPOINT sp;
+WARNING:  there is no transaction in progress
+ERROR:  SAVEPOINT can only be used in transaction blocks
+ROLLBACK TO SAVEPOINT sp\; SELECT 2;
+ERROR:  ROLLBACK TO SAVEPOINT can only be used in transaction blocks
+SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+ERROR:  RELEASE SAVEPOINT can only be used in transaction blocks
+-- but this is OK, because the BEGIN converts it to a regular xact
+SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
 -- Test for successful cleanup of an aborted transaction at session exit.
 -- THIS MUST BE THE LAST TEST IN THIS FILE.
 begin;
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index bf9cb059713..82661ab610c 100644
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -419,6 +419,60 @@ DROP FUNCTION create_temp_tab();
 DROP FUNCTION invert(x float8);
 
 
+-- Test assorted behaviors around the implicit transaction block created
+-- when multiple SQL commands are sent in a single Query message.  These
+-- tests rely on the fact that psql will not break SQL commands apart at a
+-- backslash-quoted semicolon, but will send them as one Query.
+
+create temp table i_table (f1 int);
+
+-- psql will show only the last result in a multi-statement Query
+SELECT 1\; SELECT 2\; SELECT 3;
+
+-- this implicitly commits:
+insert into i_table values(1)\; select * from i_table;
+-- 1/0 error will cause rolling back the whole implicit transaction
+insert into i_table values(2)\; select * from i_table\; select 1/0;
+select * from i_table;
+
+rollback;  -- we are not in a transaction at this point
+
+-- can use regular begin/commit/rollback within a single Query
+begin\; insert into i_table values(3)\; commit;
+rollback;  -- we are not in a transaction at this point
+begin\; insert into i_table values(4)\; rollback;
+rollback;  -- we are not in a transaction at this point
+
+-- begin converts implicit transaction into a regular one that
+-- can extend past the end of the Query
+select 1\; begin\; insert into i_table values(5);
+commit;
+select 1\; begin\; insert into i_table values(6);
+rollback;
+
+-- commit in implicit-transaction state commits but issues a warning.
+insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0;
+-- similarly, rollback aborts but issues a warning.
+insert into i_table values(9)\; rollback\; select 2;
+
+select * from i_table;
+
+rollback;  -- we are not in a transaction at this point
+
+-- implicit transaction block is still a transaction block, for e.g. VACUUM
+SELECT 1\; VACUUM;
+SELECT 1\; COMMIT\; VACUUM;
+
+-- we disallow savepoint-related commands in implicit-transaction state
+SELECT 1\; SAVEPOINT sp;
+SELECT 1\; COMMIT\; SAVEPOINT sp;
+ROLLBACK TO SAVEPOINT sp\; SELECT 2;
+SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+
+-- but this is OK, because the BEGIN converts it to a regular xact
+SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+
+
 -- Test for successful cleanup of an aborted transaction at session exit.
 -- THIS MUST BE THE LAST TEST IN THIS FILE.