From 423aba24036d681d1e65016179ae851e90812fc1 Mon Sep 17 00:00:00 2001 From: Gurjeet Singh Date: Fri, 2 Jun 2023 19:19:45 -0700 Subject: [PATCH v1] Allow user to mark a transaction uncommittable. In some cases, at the beginning of a transaction the user knows that the transaction is not supposed to be committed. In such cases the user can start the transaction with NOT COMMITTABLE attribute, and Postgres will refuse to commit the transaction. While a transaction is in progress, upon seeing an undesirable state, the user may want to continue the transaction to troubleshoot or learn more, but they may want to ensure that the transaction does not get committed. For such cases, the user can use SET TRANSACTION NOT COMMITTABLE command, and Postgres will refuse to commit the transaction. The user may use SET SESSION CHARACTERISTICS commmand to change the default COMMITTABLE attribute of all future transactions. Once marked as NOT COMMITTABLE, a transaction cannot then be marked as COMMITTABLE. --- src/backend/access/transam/xact.c | 79 +++++++++++++++++++++++++---- src/backend/commands/variable.c | 20 ++++++++ src/backend/parser/gram.y | 10 +++- src/backend/tcop/utility.c | 12 ++--- src/backend/utils/misc/guc.c | 2 + src/backend/utils/misc/guc_funcs.c | 6 +++ src/backend/utils/misc/guc_tables.c | 19 +++++++ src/bin/psql/tab-complete.c | 8 +-- src/include/access/xact.h | 7 +++ src/include/parser/kwlist.h | 1 + src/include/utils/guc_hooks.h | 1 + 11 files changed, 145 insertions(+), 20 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 8daaa535ed..e58d04a3ae 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -84,6 +84,9 @@ bool XactReadOnly; bool DefaultXactDeferrable = false; bool XactDeferrable; +bool DefaultXactCommittable = true; +bool XactCommittable = true; + int synchronous_commit = SYNCHRONOUS_COMMIT_ON; /* @@ -222,6 +225,7 @@ typedef struct SerializedTransactionState { int xactIsoLevel; bool xactDeferrable; + bool xactCommittable; FullTransactionId topFullTransactionId; FullTransactionId currentFullTransactionId; CommandId currentCommandId; @@ -2059,6 +2063,7 @@ StartTransaction(void) XactReadOnly = DefaultXactReadOnly; } XactDeferrable = DefaultXactDeferrable; + XactCommittable = DefaultXactCommittable; XactIsoLevel = DefaultXactIsoLevel; forceSyncCommit = false; MyXactFlags = 0; @@ -3004,7 +3009,7 @@ StartTransactionCommand(void) /* * Simple system for saving and restoring transaction characteristics - * (isolation level, read only, deferrable). We need this for transaction + * (isolation level, read only, deferrable, committable). We need this for transaction * chaining, so that we can set the characteristics of the new transaction to * be the same as the previous one. (We need something like this because the * GUC system resets the characteristics at transaction end, so for example @@ -3016,6 +3021,7 @@ SaveTransactionCharacteristics(SavedTransactionCharacteristics *s) s->save_XactIsoLevel = XactIsoLevel; s->save_XactReadOnly = XactReadOnly; s->save_XactDeferrable = XactDeferrable; + s->save_XactCommittable = XactCommittable; } void @@ -3024,6 +3030,7 @@ RestoreTransactionCharacteristics(const SavedTransactionCharacteristics *s) XactIsoLevel = s->save_XactIsoLevel; XactReadOnly = s->save_XactReadOnly; XactDeferrable = s->save_XactDeferrable; + XactCommittable = s->save_XactCommittable; } @@ -3884,6 +3891,10 @@ PrepareTransactionBlock(const char *gid) * The real work will be done in the upcoming CommitTransactionCommand(). * We do it this way because it's not convenient to change memory context, * resource owner, etc while executing inside a Portal. + * + * When ending a live transaction, if the user has previously declared the + * transaction uncommittable, we emit a warning and treat it as a ROLLBACK, + * instead. */ bool EndTransactionBlock(bool chain) @@ -3898,8 +3909,19 @@ EndTransactionBlock(bool chain) * to COMMIT. */ case TBLOCK_INPROGRESS: - s->blockState = TBLOCK_END; - result = true; + if (XactCommittable) + { + s->blockState = TBLOCK_END; + result = true; + } + else + { + ereport(WARNING, + (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), + errmsg("transaction is not committable"))); + + s->blockState = TBLOCK_ABORT_PENDING; + } break; /* @@ -3918,8 +3940,20 @@ EndTransactionBlock(bool chain) ereport(WARNING, (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), errmsg("there is no transaction in progress"))); - s->blockState = TBLOCK_END; - result = true; + + if (XactCommittable) + { + s->blockState = TBLOCK_END; + result = true; + } + else + { + ereport(WARNING, + (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), + errmsg("transaction is not committable"))); + + s->blockState = TBLOCK_ABORT_PENDING; + } break; /* @@ -3944,12 +3978,26 @@ EndTransactionBlock(bool chain) BlockStateAsString(s->blockState)); s = s->parent; } + if (s->blockState == TBLOCK_INPROGRESS) - s->blockState = TBLOCK_END; + { + if (XactCommittable) + { + s->blockState = TBLOCK_END; + result = true; + } + else + { + ereport(WARNING, + (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), + errmsg("transaction is not committable"))); + + s->blockState = TBLOCK_ABORT_PENDING; + } + } else elog(FATAL, "EndTransactionBlock: unexpected state %s", BlockStateAsString(s->blockState)); - result = true; break; /* @@ -3997,7 +4045,18 @@ EndTransactionBlock(bool chain) ereport(WARNING, (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), errmsg("there is no transaction in progress"))); - result = true; + if (XactCommittable) + { + result = true; + } + else + { + ereport(WARNING, + (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), + errmsg("transaction is not committable"))); + + s->blockState = TBLOCK_ABORT_PENDING; + } break; /* @@ -5360,7 +5419,7 @@ EstimateTransactionStateSpace(void) * Write out relevant details of our transaction state that will be * needed by a parallel worker. * - * We need to save and restore XactDeferrable, XactIsoLevel, and the XIDs + * We need to save and restore XactDeferrable, XactCommittable, XactIsoLevel, and the XIDs * associated with this transaction. These are serialized into a * caller-supplied buffer big enough to hold the number of bytes reported by * EstimateTransactionStateSpace(). We emit the XIDs in sorted order for the @@ -5379,6 +5438,7 @@ SerializeTransactionState(Size maxsize, char *start_address) result->xactIsoLevel = XactIsoLevel; result->xactDeferrable = XactDeferrable; + result->xactCommittable = XactCommittable; result->topFullTransactionId = XactTopFullTransactionId; result->currentFullTransactionId = CurrentTransactionState->fullTransactionId; @@ -5448,6 +5508,7 @@ StartParallelWorkerTransaction(char *tstatespace) tstate = (SerializedTransactionState *) tstatespace; XactIsoLevel = tstate->xactIsoLevel; XactDeferrable = tstate->xactDeferrable; + XactCommittable = tstate->xactCommittable; XactTopFullTransactionId = tstate->topFullTransactionId; CurrentTransactionState->fullTransactionId = tstate->currentFullTransactionId; diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index f0f2e07655..78aa1cab71 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -635,6 +635,26 @@ check_transaction_deferrable(bool *newval, void **extra, GucSource source) return true; } +/* + * SET TRANSACTION [NOT] COMMITTABLE + */ + +bool +check_transaction_committable(bool *newval, void **extra, GucSource source) +{ + /* + * If a transaction has been marked uncommittable, it cannot be marked as + * committable. + * + * TODO: Should we emit a warning here, notifying the user as to why their + * attempt to set transaction_committable = true was rejected? + */ + if (XactCommittable == false && *newval == true) + return false; + + return true; +} + /* * Random number seed * diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 39ab7eac0d..ef00123fe9 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -698,7 +698,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); CACHE CALL CALLED CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE CLUSTER COALESCE COLLATE COLLATION COLUMN COLUMNS COMMENT COMMENTS COMMIT - COMMITTED COMPRESSION CONCURRENTLY CONFIGURATION CONFLICT + COMMITTABLE COMMITTED COMPRESSION CONCURRENTLY CONFIGURATION CONFLICT CONNECTION CONSTRAINT CONSTRAINTS CONTENT_P CONTINUE_P CONVERSION_P COPY COST CREATE CROSS CSV CUBE CURRENT_P CURRENT_CATALOG CURRENT_DATE CURRENT_ROLE CURRENT_SCHEMA @@ -10978,6 +10978,12 @@ transaction_mode_item: | NOT DEFERRABLE { $$ = makeDefElem("transaction_deferrable", makeIntConst(false, @1), @1); } + | COMMITTABLE + { $$ = makeDefElem("transaction_committable", + makeIntConst(true, @1), @1); } + | NOT COMMITTABLE + { $$ = makeDefElem("transaction_committable", + makeIntConst(false, @1), @1); } ; /* Syntax with commas is SQL-spec, without commas is Postgres historical */ @@ -16976,6 +16982,7 @@ unreserved_keyword: | COMMENT | COMMENTS | COMMIT + | COMMITTABLE | COMMITTED | COMPRESSION | CONFIGURATION @@ -17515,6 +17522,7 @@ bare_label_keyword: | COMMENT | COMMENTS | COMMIT + | COMMITTABLE | COMMITTED | COMPRESSION | CONCURRENTLY diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 30b51bf4d3..b2b659cf2f 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -620,16 +620,16 @@ standard_ProcessUtility(PlannedStmt *pstmt, if (strcmp(item->defname, "transaction_isolation") == 0) SetPGVariable("transaction_isolation", - list_make1(item->arg), - true); + list_make1(item->arg), true); else if (strcmp(item->defname, "transaction_read_only") == 0) SetPGVariable("transaction_read_only", - list_make1(item->arg), - true); + list_make1(item->arg), true); else if (strcmp(item->defname, "transaction_deferrable") == 0) SetPGVariable("transaction_deferrable", - list_make1(item->arg), - true); + list_make1(item->arg), true); + else if (strcmp(item->defname, "transaction_committable") == 0) + SetPGVariable("transaction_committable", + list_make1(item->arg), true); } } break; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index a9033b7a54..dae153a2ef 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1533,6 +1533,8 @@ InitializeGUCOptions(void) PGC_POSTMASTER, PGC_S_OVERRIDE); SetConfigOption("transaction_deferrable", "no", PGC_POSTMASTER, PGC_S_OVERRIDE); + SetConfigOption("transaction_committable", "yes", + PGC_POSTMASTER, PGC_S_OVERRIDE); /* * For historical reasons, some GUC parameters can receive defaults from diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c index 0903ba43a9..0aa00a16e9 100644 --- a/src/backend/utils/misc/guc_funcs.c +++ b/src/backend/utils/misc/guc_funcs.c @@ -93,6 +93,9 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) else if (strcmp(item->defname, "transaction_deferrable") == 0) SetPGVariable("transaction_deferrable", list_make1(item->arg), stmt->is_local); + else if (strcmp(item->defname, "transaction_committable") == 0) + SetPGVariable("transaction_committable", + list_make1(item->arg), stmt->is_local); else elog(ERROR, "unexpected SET TRANSACTION element: %s", item->defname); @@ -115,6 +118,9 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) else if (strcmp(item->defname, "transaction_deferrable") == 0) SetPGVariable("default_transaction_deferrable", list_make1(item->arg), stmt->is_local); + else if (strcmp(item->defname, "transaction_committable") == 0) + SetPGVariable("default_transaction_committable", + list_make1(item->arg), stmt->is_local); else elog(ERROR, "unexpected SET SESSION element: %s", item->defname); diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 68aecad66f..d51a324f3f 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1587,6 +1587,25 @@ struct config_bool ConfigureNamesBool[] = false, check_transaction_deferrable, NULL, NULL }, + { + {"default_transaction_committable", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the default committable status of new transactions."), + NULL + }, + &DefaultXactCommittable, + true, + NULL, NULL, NULL + }, + { + {"transaction_committable", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Whether to allow committing a transaction."), + NULL, + GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + }, + &XactCommittable, + true, + check_transaction_committable, NULL, NULL + }, { {"row_security", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Enable row security."), diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 677847e434..ac8d1de61e 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2675,7 +2675,7 @@ psql_completion(const char *text, int start, int end) /* BEGIN */ else if (Matches("BEGIN")) - COMPLETE_WITH("WORK", "TRANSACTION", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); + COMPLETE_WITH("WORK", "TRANSACTION", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE", "COMMITTABLE", "NOT COMMITTABLE"); /* END, ABORT */ else if (Matches("END|ABORT")) COMPLETE_WITH("AND", "WORK", "TRANSACTION"); @@ -4401,16 +4401,16 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("AUTHORIZATION"); /* Complete "SET TRANSACTION" */ else if (Matches("SET", "TRANSACTION")) - COMPLETE_WITH("SNAPSHOT", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); + COMPLETE_WITH("SNAPSHOT", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE", "COMMITTABLE", "NOT COMMITTABLE"); else if (Matches("BEGIN|START", "TRANSACTION") || Matches("BEGIN", "WORK") || Matches("BEGIN") || Matches("SET", "SESSION", "CHARACTERISTICS", "AS", "TRANSACTION")) - COMPLETE_WITH("ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); + COMPLETE_WITH("ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE", "COMMITTABLE", "NOT COMMITTABLE"); else if (Matches("SET|BEGIN|START", "TRANSACTION|WORK", "NOT") || Matches("BEGIN", "NOT") || Matches("SET", "SESSION", "CHARACTERISTICS", "AS", "TRANSACTION", "NOT")) - COMPLETE_WITH("DEFERRABLE"); + COMPLETE_WITH("DEFERRABLE", "COMMITTABLE"); else if (Matches("SET|BEGIN|START", "TRANSACTION|WORK", "ISOLATION") || Matches("BEGIN", "ISOLATION") || Matches("SET", "SESSION", "CHARACTERISTICS", "AS", "TRANSACTION", "ISOLATION")) diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 7d3b9446e6..187432919a 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -65,6 +65,12 @@ extern PGDLLIMPORT bool xact_is_sampled; extern PGDLLIMPORT bool DefaultXactDeferrable; extern PGDLLIMPORT bool XactDeferrable; +/* + * Xact is committable + */ +extern PGDLLIMPORT bool DefaultXactCommittable; +extern PGDLLIMPORT bool XactCommittable; + typedef enum { SYNCHRONOUS_COMMIT_OFF, /* asynchronous commit */ @@ -154,6 +160,7 @@ typedef struct SavedTransactionCharacteristics int save_XactIsoLevel; bool save_XactReadOnly; bool save_XactDeferrable; + bool save_XactCommittable; } SavedTransactionCharacteristics; diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index f5b2e61ca5..1d8a1eb1f8 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -90,6 +90,7 @@ PG_KEYWORD("columns", COLUMNS, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("comment", COMMENT, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("comments", COMMENTS, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("commit", COMMIT, UNRESERVED_KEYWORD, BARE_LABEL) +PG_KEYWORD("committable", COMMITTABLE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("committed", COMMITTED, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("compression", COMPRESSION, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("concurrently", CONCURRENTLY, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL) diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 2ecb9fc086..a8930c70ae 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -149,6 +149,7 @@ extern bool check_timezone_abbreviations(char **newval, void **extra, GucSource source); extern void assign_timezone_abbreviations(const char *newval, void *extra); extern bool check_transaction_deferrable(bool *newval, void **extra, GucSource source); +extern bool check_transaction_committable(bool *newval, void **extra, GucSource source); extern bool check_transaction_isolation(int *newval, void **extra, GucSource source); extern bool check_transaction_read_only(bool *newval, void **extra, GucSource source); extern const char *show_unix_socket_permissions(void); -- 2.35.1