From 2f2d4eec311f8bd7c208de33894c0db9f269dcc1 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 16 Feb 2018 20:57:06 -0500 Subject: [PATCH v1 3/8] 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. --- 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 e8a4412671..06a7e108d7 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3905,13 +3905,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 @@ -3975,16 +3973,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) @@ -4026,13 +4014,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 @@ -4096,16 +4082,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 266a3ef8ef..b013539ee9 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3601,6 +3601,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 bbffc87842..ba5ebdf408 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1512,6 +1512,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 d99f2be2c9..ecb2494226 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -9872,40 +9872,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 65fc947fc6..2d29778787 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 ac292bc6e7..e3a63e9db4 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2964,7 +2964,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.16.2