From ec87efde8dbc670467468860c5d577dc805dae5e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 16 Feb 2018 20:57:06 -0500 Subject: [PATCH] Simplify parse representation of savepoint commands Instead of embedding the savepoint name in a list and then requiring complex code to unpack it, just add another struct field to store it directly. Reviewed-by: Alvaro Herrera --- src/backend/access/transam/xact.c | 28 ++-------------------------- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 15 +++++---------- src/backend/tcop/utility.c | 24 ++++-------------------- src/include/access/xact.h | 4 ++-- src/include/nodes/parsenodes.h | 3 ++- 7 files changed, 17 insertions(+), 59 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 49d4decc71..d16102a1e4 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3908,13 +3908,11 @@ DefineSavepoint(const char *name) * As above, we don't actually do anything here except change blockState. */ void -ReleaseSavepoint(List *options) +ReleaseSavepoint(const char *name) { TransactionState s = CurrentTransactionState; TransactionState target, xact; - ListCell *cell; - char *name = NULL; /* * Workers synchronize transaction state at the beginning of each parallel @@ -3978,16 +3976,6 @@ ReleaseSavepoint(List *options) break; } - foreach(cell, options) - { - DefElem *elem = lfirst(cell); - - if (strcmp(elem->defname, "savepoint_name") == 0) - name = strVal(elem->arg); - } - - Assert(PointerIsValid(name)); - for (target = s; PointerIsValid(target); target = target->parent) { if (PointerIsValid(target->name) && strcmp(target->name, name) == 0) @@ -4029,13 +4017,11 @@ ReleaseSavepoint(List *options) * As above, we don't actually do anything here except change blockState. */ void -RollbackToSavepoint(List *options) +RollbackToSavepoint(const char *name) { TransactionState s = CurrentTransactionState; TransactionState target, xact; - ListCell *cell; - char *name = NULL; /* * Workers synchronize transaction state at the beginning of each parallel @@ -4099,16 +4085,6 @@ RollbackToSavepoint(List *options) break; } - foreach(cell, options) - { - DefElem *elem = lfirst(cell); - - if (strcmp(elem->defname, "savepoint_name") == 0) - name = strVal(elem->arg); - } - - Assert(PointerIsValid(name)); - for (target = s; PointerIsValid(target); target = target->parent) { if (PointerIsValid(target->name) && strcmp(target->name, name) == 0) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index f84da801c6..3ad4da64aa 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3602,6 +3602,7 @@ _copyTransactionStmt(const TransactionStmt *from) COPY_SCALAR_FIELD(kind); COPY_NODE_FIELD(options); + COPY_STRING_FIELD(savepoint_name); COPY_STRING_FIELD(gid); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index ee8d925db1..765b1be74b 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1513,6 +1513,7 @@ _equalTransactionStmt(const TransactionStmt *a, const TransactionStmt *b) { COMPARE_SCALAR_FIELD(kind); COMPARE_NODE_FIELD(options); + COMPARE_STRING_FIELD(savepoint_name); COMPARE_STRING_FIELD(gid); return true; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 06c03dff3c..cd5ba2d4d8 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -9876,40 +9876,35 @@ TransactionStmt: { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_SAVEPOINT; - n->options = list_make1(makeDefElem("savepoint_name", - (Node *)makeString($2), @1)); + n->savepoint_name = $2; $$ = (Node *)n; } | RELEASE SAVEPOINT ColId { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_RELEASE; - n->options = list_make1(makeDefElem("savepoint_name", - (Node *)makeString($3), @1)); + n->savepoint_name = $3; $$ = (Node *)n; } | RELEASE ColId { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_RELEASE; - n->options = list_make1(makeDefElem("savepoint_name", - (Node *)makeString($2), @1)); + n->savepoint_name = $2; $$ = (Node *)n; } | ROLLBACK opt_transaction TO SAVEPOINT ColId { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_ROLLBACK_TO; - n->options = list_make1(makeDefElem("savepoint_name", - (Node *)makeString($5), @1)); + n->savepoint_name = $5; $$ = (Node *)n; } | ROLLBACK opt_transaction TO ColId { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_ROLLBACK_TO; - n->options = list_make1(makeDefElem("savepoint_name", - (Node *)makeString($4), @1)); + n->savepoint_name = $4; $$ = (Node *)n; } | PREPARE TRANSACTION Sconst diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 81881be1d5..ed55521a0c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -469,34 +469,18 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case TRANS_STMT_SAVEPOINT: - { - ListCell *cell; - char *name = NULL; - - RequireTransactionBlock(isTopLevel, "SAVEPOINT"); - - foreach(cell, stmt->options) - { - DefElem *elem = lfirst(cell); - - if (strcmp(elem->defname, "savepoint_name") == 0) - name = strVal(elem->arg); - } - - Assert(PointerIsValid(name)); - - DefineSavepoint(name); - } + RequireTransactionBlock(isTopLevel, "SAVEPOINT"); + DefineSavepoint(stmt->savepoint_name); break; case TRANS_STMT_RELEASE: RequireTransactionBlock(isTopLevel, "RELEASE SAVEPOINT"); - ReleaseSavepoint(stmt->options); + ReleaseSavepoint(stmt->savepoint_name); break; case TRANS_STMT_ROLLBACK_TO: RequireTransactionBlock(isTopLevel, "ROLLBACK TO SAVEPOINT"); - RollbackToSavepoint(stmt->options); + RollbackToSavepoint(stmt->savepoint_name); /* * CommitTransactionCommand is in charge of diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 4a1307a4f0..87ae2cd4df 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -354,9 +354,9 @@ extern bool PrepareTransactionBlock(const char *gid); extern void UserAbortTransactionBlock(void); extern void BeginImplicitTransactionBlock(void); extern void EndImplicitTransactionBlock(void); -extern void ReleaseSavepoint(List *options); +extern void ReleaseSavepoint(const char *name); extern void DefineSavepoint(const char *name); -extern void RollbackToSavepoint(List *options); +extern void RollbackToSavepoint(const char *name); extern void BeginInternalSubTransaction(const char *name); extern void ReleaseCurrentSubTransaction(void); extern void RollbackAndReleaseCurrentSubTransaction(void); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index f668cbad34..92082b3a7a 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2966,7 +2966,8 @@ typedef struct TransactionStmt { NodeTag type; TransactionStmtKind kind; /* see above */ - List *options; /* for BEGIN/START and savepoint commands */ + List *options; /* for BEGIN/START commands */ + char *savepoint_name; /* for savepoint commands */ char *gid; /* for two-phase-commit related commands */ } TransactionStmt; -- 2.40.0