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;