Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection
Дата
Msg-id 32130.1504389456@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
I wrote:
> My thought is that what we need to do is find a way for isTopLevel
> to be false if we're processing a multi-command string.

Nah, that's backwards, the problem is exactly that isTopLevel is
false if we're processing a multi-command string.  That allows
DefineSavepoint to think that it's inside a function, and we don't
disallow savepoints inside functions.  (Or at least, xact.c doesn't
enforce any such prohibition; it's up to spi.c and the individual PLs
to decide if they could support that.)

After contemplating my navel for awhile, I think that this case proves
that the quick hack embodied in commit 4f896dac1 is inadequate.  Rather
than piling another quick hack on top and hoping that the result is OK,
I think it's time to bite the bullet and represent the behavior we want
explicitly in the transaction machinery.  Accordingly, PFA a patch
that invents a notion of an "implicit" transaction block.

I also added a bunch of test cases exercising the behavior.  Except
for the problem of FATAL exits for savepoint commands, all these
cases work exactly like they do in unpatched code.  However, now that
we have an explicit representation, it'd be easy to tweak the behavior
if we want to.  For instance, I'm not entirely sure whether we want
the behavior that COMMIT and ROLLBACK in this state print warnings.
Good luck changing that before; but now it'd be a straightforward
adjustment.

I'm inclined to complete the reversion of 4f896dac1 by also undoing
its error message text change in PreventTransactionChain,

-                 errmsg("%s cannot be executed from a function", stmtType)));
+                 errmsg("%s cannot be executed from a function or multi-command string",
+                        stmtType)));

but this patch doesn't include that change.

My feeling about this is that we don't need a back-patch.  Throwing
FATAL rather than ERROR for a misplaced savepoint command is a bit
unpleasant, but it doesn't break other sessions, and the upshot is
really the same: don't do that.

            regards, tom lane

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5e7e812..ba4b2da 100644
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** typedef enum TBlockState
*** 145,150 ****
--- 145,151 ----
      /* transaction block states */
      TBLOCK_BEGIN,                /* starting transaction block */
      TBLOCK_INPROGRESS,            /* live transaction */
+     TBLOCK_IMPLICIT_INPROGRESS, /* live transaction after implicit BEGIN */
      TBLOCK_PARALLEL_INPROGRESS, /* live transaction inside parallel worker */
      TBLOCK_END,                    /* COMMIT received */
      TBLOCK_ABORT,                /* failed xact, awaiting ROLLBACK */
*************** StartTransactionCommand(void)
*** 2700,2705 ****
--- 2701,2707 ----
               * previous CommitTransactionCommand.)
               */
          case TBLOCK_INPROGRESS:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_SUBINPROGRESS:
              break;

*************** CommitTransactionCommand(void)
*** 2790,2795 ****
--- 2792,2798 ----
               * counter and return.
               */
          case TBLOCK_INPROGRESS:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_SUBINPROGRESS:
              CommandCounterIncrement();
              break;
*************** AbortCurrentTransaction(void)
*** 3014,3023 ****
              break;

              /*
!              * if we aren't in a transaction block, we just do the basic abort
!              * & cleanup transaction.
               */
          case TBLOCK_STARTED:
              AbortTransaction();
              CleanupTransaction();
              s->blockState = TBLOCK_DEFAULT;
--- 3017,3028 ----
              break;

              /*
!              * If we aren't in a transaction block, we just do the basic abort
!              * & cleanup transaction.  For this purpose, we treat an implicit
!              * transaction block as if it were a simple statement.
               */
          case TBLOCK_STARTED:
+         case TBLOCK_IMPLICIT_INPROGRESS:
              AbortTransaction();
              CleanupTransaction();
              s->blockState = TBLOCK_DEFAULT;
*************** BeginTransactionBlock(void)
*** 3429,3434 ****
--- 3434,3448 ----
              break;

              /*
+              * BEGIN converts an implicit transaction block to a regular one.
+              * (Note that we allow this even if we've already done some
+              * commands, which is a bit odd but matches historical practice.)
+              */
+         case TBLOCK_IMPLICIT_INPROGRESS:
+             s->blockState = TBLOCK_BEGIN;
+             break;
+
+             /*
               * Already a transaction block in progress.
               */
          case TBLOCK_INPROGRESS:
*************** PrepareTransactionBlock(char *gid)
*** 3503,3509 ****
               * ignore case where we are not in a transaction;
               * EndTransactionBlock already issued a warning.
               */
!             Assert(s->blockState == TBLOCK_STARTED);
              /* Don't send back a PREPARE result tag... */
              result = false;
          }
--- 3517,3524 ----
               * ignore case where we are not in a transaction;
               * EndTransactionBlock already issued a warning.
               */
!             Assert(s->blockState == TBLOCK_STARTED ||
!                    s->blockState == TBLOCK_IMPLICIT_INPROGRESS);
              /* Don't send back a PREPARE result tag... */
              result = false;
          }
*************** EndTransactionBlock(void)
*** 3542,3547 ****
--- 3557,3574 ----
              break;

              /*
+              * In an implicit transaction block, commit, but issue a warning
+              * because there was no explicit BEGIN before this.
+              */
+         case TBLOCK_IMPLICIT_INPROGRESS:
+             ereport(WARNING,
+                     (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                      errmsg("there is no transaction in progress")));
+             s->blockState = TBLOCK_END;
+             result = true;
+             break;
+
+             /*
               * We are in a failed transaction block.  Tell
               * CommitTransactionCommand it's time to exit the block.
               */
*************** UserAbortTransactionBlock(void)
*** 3705,3712 ****
--- 3732,3745 ----
               * WARNING and go to abort state.  The upcoming call to
               * CommitTransactionCommand() will then put us back into the
               * default state.
+              *
+              * We do the same thing with ABORT inside an implicit transaction,
+              * although in this case we might be rolling back actual database
+              * state changes.  (It's debatable whether we should issue a
+              * WARNING in this case, but we have done so historically.)
               */
          case TBLOCK_STARTED:
+         case TBLOCK_IMPLICIT_INPROGRESS:
              ereport(WARNING,
                      (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
                       errmsg("there is no transaction in progress")));
*************** UserAbortTransactionBlock(void)
*** 3744,3749 ****
--- 3777,3834 ----
  }

  /*
+  * BeginImplicitTransactionBlock
+  *        Start an implicit transaction block if we're not already in one.
+  *
+  * Unlike BeginTransactionBlock, this is called directly from the main loop
+  * in postgres.c, not within a Portal.  So we can just change blockState
+  * without a lot of ceremony.  We do not expect caller to do
+  * CommitTransactionCommand/StartTransactionCommand.
+  */
+ void
+ BeginImplicitTransactionBlock(void)
+ {
+     TransactionState s = CurrentTransactionState;
+
+     /*
+      * If we are in STARTED state (that is, no transaction block is open),
+      * switch to IMPLICIT_INPROGRESS state, creating an implicit transaction
+      * block.
+      *
+      * For caller convenience, we consider all other transaction states as
+      * legal here; otherwise the caller would need its own state check, which
+      * seems rather pointless.
+      */
+     if (s->blockState == TBLOCK_STARTED)
+         s->blockState = TBLOCK_IMPLICIT_INPROGRESS;
+ }
+
+ /*
+  * EndImplicitTransactionBlock
+  *        End an implicit transaction block, if we're in one.
+  *
+  * Like EndTransactionBlock, we just make any needed blockState change here.
+  * The real work will be done in the upcoming CommitTransactionCommand().
+  */
+ void
+ EndImplicitTransactionBlock(void)
+ {
+     TransactionState s = CurrentTransactionState;
+
+     /*
+      * If we are in IMPLICIT_INPROGRESS state, switch back to STARTED state,
+      * allowing CommitTransactionCommand to commit whatever happened during
+      * the implicit transaction block as though it were a single statement.
+      *
+      * For caller convenience, we consider all other transaction states as
+      * legal here; otherwise the caller would need its own state check, which
+      * seems rather pointless.
+      */
+     if (s->blockState == TBLOCK_IMPLICIT_INPROGRESS)
+         s->blockState = TBLOCK_STARTED;
+ }
+
+ /*
   * DefineSavepoint
   *        This executes a SAVEPOINT command.
   */
*************** DefineSavepoint(char *name)
*** 3780,3785 ****
--- 3865,3879 ----
                  s->name = MemoryContextStrdup(TopTransactionContext, name);
              break;

+         case TBLOCK_IMPLICIT_INPROGRESS:
+             /* Disallow SAVEPOINT in an implicit transaction */
+             ereport(ERROR,
+                     (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+             /* translator: %s represents an SQL statement name */
+                      errmsg("%s can only be used in transaction blocks",
+                             "SAVEPOINT")));
+             break;
+
              /* These cases are invalid. */
          case TBLOCK_DEFAULT:
          case TBLOCK_STARTED:
*************** ReleaseSavepoint(List *options)
*** 3834,3841 ****
      switch (s->blockState)
      {
              /*
!              * We can't rollback to a savepoint if there is no savepoint
!              * defined.
               */
          case TBLOCK_INPROGRESS:
              ereport(ERROR,
--- 3928,3934 ----
      switch (s->blockState)
      {
              /*
!              * We can't release a savepoint if there is no savepoint defined.
               */
          case TBLOCK_INPROGRESS:
              ereport(ERROR,
*************** ReleaseSavepoint(List *options)
*** 3843,3848 ****
--- 3936,3950 ----
                       errmsg("no such savepoint")));
              break;

+         case TBLOCK_IMPLICIT_INPROGRESS:
+             /* Disallow RELEASE SAVEPOINT in an implicit transaction */
+             ereport(ERROR,
+                     (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+             /* translator: %s represents an SQL statement name */
+                      errmsg("%s can only be used in transaction blocks",
+                             "RELEASE SAVEPOINT")));
+             break;
+
              /*
               * We are in a non-aborted subtransaction.  This is the only valid
               * case.
*************** RollbackToSavepoint(List *options)
*** 3957,3962 ****
--- 4059,4073 ----
                       errmsg("no such savepoint")));
              break;

+         case TBLOCK_IMPLICIT_INPROGRESS:
+             /* Disallow ROLLBACK TO SAVEPOINT in an implicit transaction */
+             ereport(ERROR,
+                     (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+             /* translator: %s represents an SQL statement name */
+                      errmsg("%s can only be used in transaction blocks",
+                             "ROLLBACK TO SAVEPOINT")));
+             break;
+
              /*
               * There is at least one savepoint, so proceed.
               */
*************** RollbackToSavepoint(List *options)
*** 4046,4056 ****
  /*
   * BeginInternalSubTransaction
   *        This is the same as DefineSavepoint except it allows TBLOCK_STARTED,
!  *        TBLOCK_END, and TBLOCK_PREPARE states, and therefore it can safely be
!  *        used in functions that might be called when not inside a BEGIN block
!  *        or when running deferred triggers at COMMIT/PREPARE time.  Also, it
!  *        automatically does CommitTransactionCommand/StartTransactionCommand
!  *        instead of expecting the caller to do it.
   */
  void
  BeginInternalSubTransaction(char *name)
--- 4157,4168 ----
  /*
   * BeginInternalSubTransaction
   *        This is the same as DefineSavepoint except it allows TBLOCK_STARTED,
!  *        TBLOCK_IMPLICIT_INPROGRESS, TBLOCK_END, and TBLOCK_PREPARE states,
!  *        and therefore it can safely be used in functions that might be called
!  *        when not inside a BEGIN block or when running deferred triggers at
!  *        COMMIT/PREPARE time.  Also, it automatically does
!  *        CommitTransactionCommand/StartTransactionCommand instead of expecting
!  *        the caller to do it.
   */
  void
  BeginInternalSubTransaction(char *name)
*************** BeginInternalSubTransaction(char *name)
*** 4076,4081 ****
--- 4188,4194 ----
      {
          case TBLOCK_STARTED:
          case TBLOCK_INPROGRESS:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_END:
          case TBLOCK_PREPARE:
          case TBLOCK_SUBINPROGRESS:
*************** RollbackAndReleaseCurrentSubTransaction(
*** 4180,4185 ****
--- 4293,4299 ----
          case TBLOCK_DEFAULT:
          case TBLOCK_STARTED:
          case TBLOCK_BEGIN:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_PARALLEL_INPROGRESS:
          case TBLOCK_SUBBEGIN:
          case TBLOCK_INPROGRESS:
*************** RollbackAndReleaseCurrentSubTransaction(
*** 4211,4216 ****
--- 4325,4331 ----
      s = CurrentTransactionState;    /* changed by pop */
      AssertState(s->blockState == TBLOCK_SUBINPROGRESS ||
                  s->blockState == TBLOCK_INPROGRESS ||
+                 s->blockState == TBLOCK_IMPLICIT_INPROGRESS ||
                  s->blockState == TBLOCK_STARTED);
  }

*************** AbortOutOfAnyTransaction(void)
*** 4259,4264 ****
--- 4374,4380 ----
              case TBLOCK_STARTED:
              case TBLOCK_BEGIN:
              case TBLOCK_INPROGRESS:
+             case TBLOCK_IMPLICIT_INPROGRESS:
              case TBLOCK_PARALLEL_INPROGRESS:
              case TBLOCK_END:
              case TBLOCK_ABORT_PENDING:
*************** TransactionBlockStatusCode(void)
*** 4369,4374 ****
--- 4485,4491 ----
          case TBLOCK_BEGIN:
          case TBLOCK_SUBBEGIN:
          case TBLOCK_INPROGRESS:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_PARALLEL_INPROGRESS:
          case TBLOCK_SUBINPROGRESS:
          case TBLOCK_END:
*************** BlockStateAsString(TBlockState blockStat
*** 5036,5041 ****
--- 5153,5160 ----
              return "BEGIN";
          case TBLOCK_INPROGRESS:
              return "INPROGRESS";
+         case TBLOCK_IMPLICIT_INPROGRESS:
+             return "IMPLICIT_INPROGRESS";
          case TBLOCK_PARALLEL_INPROGRESS:
              return "PARALLEL_INPROGRESS";
          case TBLOCK_END:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8d3fecf..b27ca24 100644
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** exec_simple_query(const char *query_stri
*** 883,892 ****
      ListCell   *parsetree_item;
      bool        save_log_statement_stats = log_statement_stats;
      bool        was_logged = false;
-     bool        isTopLevel;
      char        msec_str[32];

-
      /*
       * Report query to various monitoring facilities.
       */
--- 883,890 ----
*************** exec_simple_query(const char *query_stri
*** 947,966 ****
      MemoryContextSwitchTo(oldcontext);

      /*
-      * We'll tell PortalRun it's a top-level command iff there's exactly one
-      * raw parsetree.  If more than one, it's effectively a transaction block
-      * and we want PreventTransactionChain to reject unsafe commands. (Note:
-      * we're assuming that query rewrite cannot add commands that are
-      * significant to PreventTransactionChain.)
-      */
-     isTopLevel = (list_length(parsetree_list) == 1);
-
-     /*
       * Run through the raw parsetree(s) and process each one.
       */
      foreach(parsetree_item, parsetree_list)
      {
          RawStmt    *parsetree = lfirst_node(RawStmt, parsetree_item);
          bool        snapshot_set = false;
          const char *commandTag;
          char        completionTag[COMPLETION_TAG_BUFSIZE];
--- 945,956 ----
      MemoryContextSwitchTo(oldcontext);

      /*
       * Run through the raw parsetree(s) and process each one.
       */
      foreach(parsetree_item, parsetree_list)
      {
          RawStmt    *parsetree = lfirst_node(RawStmt, parsetree_item);
+         bool        is_last_tree = (lnext(parsetree_item) == NULL);
          bool        snapshot_set = false;
          const char *commandTag;
          char        completionTag[COMPLETION_TAG_BUFSIZE];
*************** exec_simple_query(const char *query_stri
*** 1001,1006 ****
--- 991,1011 ----
          /* Make sure we are in a transaction command */
          start_xact_command();

+         /*
+          * For historical reasons, if multiple SQL statements are given in a
+          * single "simple Query" message, we execute them as a single
+          * transaction, unless explicit transaction control commands are
+          * included.  To represent this behavior properly in the transaction
+          * machinery, we use an "implicit" transaction block.  If this isn't
+          * the last statement, and we're not already in a transaction block,
+          * start an implicit transaction block to force this statement to be
+          * grouped together with the following ones.  (Thus, putting a COMMIT
+          * midway through the list doesn't allow the following statements to
+          * execute normally; they'll get grouped together anyway.)
+          */
+         if (!is_last_tree)
+             BeginImplicitTransactionBlock();
+
          /* If we got a cancel signal in parsing or prior command, quit */
          CHECK_FOR_INTERRUPTS();

*************** exec_simple_query(const char *query_stri
*** 1098,1104 ****
           */
          (void) PortalRun(portal,
                           FETCH_ALL,
!                          isTopLevel,
                           true,
                           receiver,
                           receiver,
--- 1103,1109 ----
           */
          (void) PortalRun(portal,
                           FETCH_ALL,
!                          true,
                           true,
                           receiver,
                           receiver,
*************** exec_simple_query(const char *query_stri
*** 1108,1132 ****

          PortalDrop(portal, false);

!         if (IsA(parsetree->stmt, TransactionStmt))
          {
              /*
!              * If this was a transaction control statement, commit it. We will
!              * start a new xact command for the next command (if any).
               */
              finish_xact_command();
          }
!         else if (lnext(parsetree_item) == NULL)
          {
              /*
!              * If this is the last parsetree of the query string, close down
!              * transaction statement before reporting command-complete.  This
!              * is so that any end-of-transaction errors are reported before
!              * the command-complete message is issued, to avoid confusing
!              * clients who will expect either a command-complete message or an
!              * error, not one and then the other.  But for compatibility with
!              * historical Postgres behavior, we do not force a transaction
!              * boundary between queries appearing in a single query string.
               */
              finish_xact_command();
          }
--- 1113,1139 ----

          PortalDrop(portal, false);

!         if (is_last_tree)
          {
              /*
!              * If this is the last parsetree of the query string, close down
!              * transaction statement, and any implicit transaction block,
!              * before reporting command-complete.  This is so that any
!              * end-of-transaction errors are reported before the
!              * command-complete message is issued, to avoid confusing clients
!              * who will expect either a command-complete message or an error,
!              * not one and then the other.  But for compatibility with
!              * historical Postgres behavior, we do not force a transaction
!              * boundary between queries appearing in a single query string.
               */
+             EndImplicitTransactionBlock();
              finish_xact_command();
          }
!         else if (IsA(parsetree->stmt, TransactionStmt))
          {
              /*
!              * If this was a transaction control statement, commit it. We will
!              * start a new xact command for the next command.
               */
              finish_xact_command();
          }
*************** exec_simple_query(const char *query_stri
*** 1149,1155 ****
      }                            /* end loop over parsetrees */

      /*
!      * Close down transaction statement, if one is open.
       */
      finish_xact_command();

--- 1156,1164 ----
      }                            /* end loop over parsetrees */

      /*
!      * Close down transaction statement, if one is open.  (This will only do
!      * something if the parsetree list was empty; otherwise the is_last_tree
!      * case above covered it.)
       */
      finish_xact_command();

diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index ad5aad9..f2c10f9 100644
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** extern void BeginTransactionBlock(void);
*** 352,357 ****
--- 352,359 ----
  extern bool EndTransactionBlock(void);
  extern bool PrepareTransactionBlock(char *gid);
  extern void UserAbortTransactionBlock(void);
+ extern void BeginImplicitTransactionBlock(void);
+ extern void EndImplicitTransactionBlock(void);
  extern void ReleaseSavepoint(List *options);
  extern void DefineSavepoint(char *name);
  extern void RollbackToSavepoint(List *options);
diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index d9b702d..bc0b15c 100644
*** a/src/test/regress/expected/transactions.out
--- b/src/test/regress/expected/transactions.out
*************** ERROR:  portal "ctt" cannot be run
*** 659,664 ****
--- 659,742 ----
  COMMIT;
  DROP FUNCTION create_temp_tab();
  DROP FUNCTION invert(x float8);
+ -- Test assorted behaviors around the implicit transaction block created
+ -- when multiple SQL commands are sent in a single Query message.  These
+ -- tests rely on the fact that psql will not break SQL commands apart at a
+ -- backslash-quoted semicolon, but will send them as one Query.
+ create temp table i_table (f1 int);
+ -- psql will show only the last result in a multi-statement Query
+ SELECT 1\; SELECT 2\; SELECT 3;
+  ?column?
+ ----------
+         3
+ (1 row)
+
+ -- this implicitly commits:
+ insert into i_table values(1)\; select * from i_table;
+  f1
+ ----
+   1
+ (1 row)
+
+ -- 1/0 error will cause rolling back the whole implicit transaction
+ insert into i_table values(2)\; select * from i_table\; select 1/0;
+ ERROR:  division by zero
+ select * from i_table;
+  f1
+ ----
+   1
+ (1 row)
+
+ rollback;  -- we are not in a transaction at this point
+ WARNING:  there is no transaction in progress
+ -- can use regular begin/commit/rollback within a single Query
+ begin\; insert into i_table values(3)\; commit;
+ rollback;  -- we are not in a transaction at this point
+ WARNING:  there is no transaction in progress
+ begin\; insert into i_table values(4)\; rollback;
+ rollback;  -- we are not in a transaction at this point
+ WARNING:  there is no transaction in progress
+ -- begin converts implicit transaction into a regular one that
+ -- can extend past the end of the Query
+ select 1\; begin\; insert into i_table values(5);
+ commit;
+ select 1\; begin\; insert into i_table values(6);
+ rollback;
+ -- commit in implicit-transaction state commits but issues a warning.
+ insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0;
+ WARNING:  there is no transaction in progress
+ ERROR:  division by zero
+ -- similarly, rollback aborts but issues a warning.
+ insert into i_table values(9)\; rollback\; select 2;
+ WARNING:  there is no transaction in progress
+  ?column?
+ ----------
+         2
+ (1 row)
+
+ select * from i_table;
+  f1
+ ----
+   1
+   3
+   5
+   7
+ (4 rows)
+
+ rollback;  -- we are not in a transaction at this point
+ WARNING:  there is no transaction in progress
+ -- implicit transaction block is still a transaction block, for e.g. VACUUM
+ SELECT 1\; VACUUM;
+ ERROR:  VACUUM cannot run inside a transaction block
+ -- we disallow savepoint-related commands in implicit-transaction state
+ SELECT 1\; SAVEPOINT sp;
+ ERROR:  SAVEPOINT can only be used in transaction blocks
+ ROLLBACK TO SAVEPOINT sp\; SELECT 2;
+ ERROR:  ROLLBACK TO SAVEPOINT can only be used in transaction blocks
+ SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+ ERROR:  RELEASE SAVEPOINT can only be used in transaction blocks
+ -- but this is OK, because the BEGIN converts it to a regular xact
+ SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
  -- Test for successful cleanup of an aborted transaction at session exit.
  -- THIS MUST BE THE LAST TEST IN THIS FILE.
  begin;
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index bf9cb05..3ebde73 100644
*** a/src/test/regress/sql/transactions.sql
--- b/src/test/regress/sql/transactions.sql
*************** DROP FUNCTION create_temp_tab();
*** 419,424 ****
--- 419,476 ----
  DROP FUNCTION invert(x float8);


+ -- Test assorted behaviors around the implicit transaction block created
+ -- when multiple SQL commands are sent in a single Query message.  These
+ -- tests rely on the fact that psql will not break SQL commands apart at a
+ -- backslash-quoted semicolon, but will send them as one Query.
+
+ create temp table i_table (f1 int);
+
+ -- psql will show only the last result in a multi-statement Query
+ SELECT 1\; SELECT 2\; SELECT 3;
+
+ -- this implicitly commits:
+ insert into i_table values(1)\; select * from i_table;
+ -- 1/0 error will cause rolling back the whole implicit transaction
+ insert into i_table values(2)\; select * from i_table\; select 1/0;
+ select * from i_table;
+
+ rollback;  -- we are not in a transaction at this point
+
+ -- can use regular begin/commit/rollback within a single Query
+ begin\; insert into i_table values(3)\; commit;
+ rollback;  -- we are not in a transaction at this point
+ begin\; insert into i_table values(4)\; rollback;
+ rollback;  -- we are not in a transaction at this point
+
+ -- begin converts implicit transaction into a regular one that
+ -- can extend past the end of the Query
+ select 1\; begin\; insert into i_table values(5);
+ commit;
+ select 1\; begin\; insert into i_table values(6);
+ rollback;
+
+ -- commit in implicit-transaction state commits but issues a warning.
+ insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0;
+ -- similarly, rollback aborts but issues a warning.
+ insert into i_table values(9)\; rollback\; select 2;
+
+ select * from i_table;
+
+ rollback;  -- we are not in a transaction at this point
+
+ -- implicit transaction block is still a transaction block, for e.g. VACUUM
+ SELECT 1\; VACUUM;
+
+ -- we disallow savepoint-related commands in implicit-transaction state
+ SELECT 1\; SAVEPOINT sp;
+ ROLLBACK TO SAVEPOINT sp\; SELECT 2;
+ SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
+
+ -- but this is OK, because the BEGIN converts it to a regular xact
+ SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+
+
  -- Test for successful cleanup of an aborted transaction at session exit.
  -- THIS MUST BE THE LAST TEST IN THIS FILE.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Alik Khilazhev
Дата:
Сообщение: Re: [HACKERS] [WIP] Zipfian distribution in pgbench
Следующее
От: Daniel Gustafsson
Дата:
Сообщение: Re: [HACKERS] adding the commit to a patch's thread