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 <alvherre@alvh.no-ip.org>

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 49d4decc71083bab42e7559c0f7e0e0a8cf3c715..d16102a1e4dd22c6f2dd1c463577478596660207 100644 (file)
--- 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 f84da801c6d27c212bae8bf95dcff7a8432e5d93..3ad4da64aafd8ce4ada56652ddf2009dfb826968 100644 (file)
--- 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 ee8d925db14daa9bb0337b19e9f333ac3abfff69..765b1be74b342a2acdc50d360392392d7117f45e 100644 (file)
--- 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 06c03dff3cea1b69fe15322cd87f86ece9597cb1..cd5ba2d4d8d47acc40c0daa9a25a5b041fec67dc 100644 (file)
--- 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 81881be1d515822f3742cf8386e7697aeba79b33..ed55521a0cdd027fa24d390cbbd46dc29c530c7d 100644 (file)
--- 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 4a1307a4f01d748cb926154d60f554205c24f3ec..87ae2cd4df66cb916111683211d22d872cf744e0 100644 (file)
--- 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 f668cbad346f27ce3f3148be9c705f0554948d45..92082b3a7a21b02136f607a45a4d5a543c3251ed 100644 (file)
--- 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;