postgres.git / summary / log / commit / refs
commit 5c3067282ec12f1e89c5f2ae8a3a5968520716e8
Author: Michael Paquier <michael@paquier.xyz>
Date: Wed Nov 27 00:31:43 2024 +0000
First released in: 18.0, REL_13_STABLE (reverted before release), REL_14_STABLE (reverted before release), REL_15_STABLE (reverted before release), REL_16_STABLE (reverted before release), REL_17_STABLE (reverted before release)
Reverted by: 24da0e761837 (in 13.19), 3aae60c851ba (in 14.16), 6d1183515e4f (in 15.11), 76653134ae87 (in 16.7), 7668e85a4044 (in 17.3)
Handle better implicit transaction state of pipeline mode
When using a pipeline, a transaction starts from the first command and
is committed with a Sync message or when the pipeline ends.
Functions like IsInTransactionBlock() or PreventInTransactionBlock()
were already able to understand a pipeline as being in a transaction
block, but it was not the case of CheckTransactionBlock(). This
function is called for example to generate a WARNING for SET LOCAL,
complaining that it is used outside of a transaction block.
The current state of the code caused multiple problems, like:
- SET LOCAL executed at any stage of a pipeline issued a WARNING, even
if the command was at least second in line where the pipeline is in a
transaction state.
- LOCK TABLE failed when invoked at any step of a pipeline, even if it
should be able to work within a transaction block.
The pipeline protocol assumes that the first command of a pipeline is
not part of a transaction block, and that any follow-up commands is
considered as within a transaction block.
This commit changes the backend so as an implicit transaction block is
started each time the first Execute message of a pipeline has finished
processing, with this implicit transaction block ended once a sync is
processed. The checks based on XACT_FLAGS_PIPELINING in the routines
checking if we are in a transaction block are not necessary: it is
enough to rely on the existing ones.
Some tests are added to pgbench, that can be backpatched down to v17
when \syncpipeline is involved and down to v14 where \startpipeline and
\endpipeline are available. This is unfortunately limited regarding the
error patterns that can be checked, but it provides coverage for various
pipeline combinations to check if these succeed or fail. These tests
are able to capture the case of SET LOCAL's WARNING. The author has
proposed a different feature to improve the coverage by adding similar
meta-commands to psql where error messages could be checked, something
more useful for the cases where commands cannot be used in transaction
blocks, like REINDEX CONCURRENTLY or VACUUM. This is considered as
future work for v18~.
Author: Anthonin Bonnefoy
Reviewed-by: Jelte Fennema-Nio, Michael Paquier
Discussion: https://postgr.es/m/CAO6_XqrWO8uNBQrSu5r6jh+vTGi5Oiyk4y8yXDORdE2jbzw8xw@mail.gmail.com
Backpatch-through: 13
doc/src/sgml/protocol.sgml | 21 +++++++++++----------
src/backend/access/transam/xact.c | 13 -------------
src/backend/tcop/postgres.c | 18 ++++++++++++++++++
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 812c07e500f..9a5b7eeb4a8 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1088,16 +1088,17 @@ SELCT 1/0;<!-- this typo is intentional -->
<para>
If the client has not issued an explicit <command>BEGIN</command>,
- then each Sync ordinarily causes an implicit <command>COMMIT</command>
- if the preceding step(s) succeeded, or an
- implicit <command>ROLLBACK</command> if they failed. However, there
- are a few DDL commands (such as <command>CREATE DATABASE</command>)
- that cannot be executed inside a transaction block. If one of
- these is executed in a pipeline, it will fail unless it is the first
- command in the pipeline. Furthermore, upon success it will force an
- immediate commit to preserve database consistency. Thus a Sync
- immediately following one of these commands has no effect except to
- respond with ReadyForQuery.
+ then an implicit transaction block is started and each Sync ordinarily
+ causes an implicit <command>COMMIT</command> if the preceding step(s)
+ succeeded, or an implicit <command>ROLLBACK</command> if they failed.
+ This implicit transaction block will only be detected by the server
+ when the first command ends without a sync. There are a few DDL
+ commands (such as <command>CREATE DATABASE</command>) that cannot be
+ executed inside a transaction block. If one of these is executed in a
+ pipeline, it will fail unless it is the first command after a Sync.
+ Furthermore, upon success it will force an immediate commit to preserve
+ database consistency. Thus a Sync immediately following one of these
+ commands has no effect except to respond with ReadyForQuery.
</para>
<para>
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ffe26e26f66..423545e6038 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3405,16 +3405,6 @@ PreventInTransactionBlock(bool isTopLevel, const char *stmtType)
errmsg("%s cannot run inside a subtransaction",
stmtType)));
- /*
- * inside a pipeline that has started an implicit transaction?
- */
- if (MyXactFlags & XACT_FLAGS_PIPELINING)
- ereport(ERROR,
- (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
- /* translator: %s represents an SQL statement name */
- errmsg("%s cannot be executed within a pipeline",
- stmtType)));
-
/*
* inside a function call?
*/
@@ -3526,9 +3516,6 @@ IsInTransactionBlock(bool isTopLevel)
if (IsSubTransaction())
return true;
- if (MyXactFlags & XACT_FLAGS_PIPELINING)
- return true;
-
if (!isTopLevel)
return true;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index ec630b44916..d375d845b97 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2656,6 +2656,17 @@ start_xact_command(void)
xact_started = true;
}
+ else if (MyXactFlags & XACT_FLAGS_PIPELINING)
+ {
+ /*
+ * When the first Execute message is completed, following commands
+ * will be done in an implicit transaction block created via
+ * pipelining. The transaction state needs to be updated to an
+ * implicit block if we're not already in a transaction block (like
+ * one started by an explicit BEGIN).
+ */
+ BeginImplicitTransactionBlock();
+ }
/*
* Start statement timeout if necessary. Note that this'll intentionally
@@ -4605,6 +4616,13 @@ PostgresMain(int argc, char *argv[],
case 'S': /* sync */
pq_getmsgend(&input_message);
+
+ /*
+ * If pipelining was used, we may be in an implicit
+ * transaction block. Close it before calling
+ * finish_xact_command.
+ */
+ EndImplicitTransactionBlock();
finish_xact_command();
send_ready_for_query = true;
break;
[parent: 48a6cd1ae636]