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

I wrote:
> ... PFA a patch
> that invents a notion of an "implicit" transaction block.

On further consideration, I think the control logic I added in
exec_simple_query() is a shade bogus.  I set it up to only force
an implicit transaction block when there are at least two statements
remaining to execute.  However, that has the result of allowing, eg,

    begin\; select 1\; commit\; vacuum;

Now in principle it's perfectly OK to allow that, since the vacuum
is alone in its transaction.  But it feels more like an implementation
artifact than a good design.  The existing code doesn't allow it,
and we might have a hard time duplicating this behavior if we ever
significantly rewrote the transaction infrastructure.  Plus I'd hate
to have to explain it to users.  I think we'd be better off enforcing
transaction block restrictions on every statement in a multi-command
string, regardless of the location of any COMMIT/ROLLBACK within the
string.

Hence, attached a v2 that does it like that.  I also fully reverted
4f896dac1 by undoing its changes to PreventTransactionChain; other
than that, the changes in xact.c are the same as before.

            regards, tom lane

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5e7e812..8b33676 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;
*************** AbortCurrentTransaction(void)
*** 3148,3156 ****
   *    completes).  Subtransactions are verboten too.
   *
   *    isTopLevel: passed down from ProcessUtility to determine whether we are
!  *    inside a function or multi-query querystring.  (We will always fail if
!  *    this is false, but it's convenient to centralize the check here instead of
!  *    making callers do it.)
   *    stmtType: statement type name, for error messages.
   */
  void
--- 3153,3160 ----
   *    completes).  Subtransactions are verboten too.
   *
   *    isTopLevel: passed down from ProcessUtility to determine whether we are
!  *    inside a function.  (We will always fail if this is false, but it's
!  *    convenient to centralize the check here instead of making callers do it.)
   *    stmtType: statement type name, for error messages.
   */
  void
*************** PreventTransactionChain(bool isTopLevel,
*** 3183,3190 ****
          ereport(ERROR,
                  (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
          /* translator: %s represents an SQL statement name */
!                  errmsg("%s cannot be executed from a function or multi-command string",
!                         stmtType)));

      /* If we got past IsTransactionBlock test, should be in default state */
      if (CurrentTransactionState->blockState != TBLOCK_DEFAULT &&
--- 3187,3193 ----
          ereport(ERROR,
                  (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
          /* translator: %s represents an SQL statement name */
!                  errmsg("%s cannot be executed from a function", stmtType)));

      /* If we got past IsTransactionBlock test, should be in default state */
      if (CurrentTransactionState->blockState != TBLOCK_DEFAULT &&
*************** BeginTransactionBlock(void)
*** 3429,3434 ****
--- 3432,3446 ----
              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;
          }
--- 3515,3522 ----
               * 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 ****
--- 3555,3572 ----
              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 ****
--- 3730,3743 ----
               * 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 ****
--- 3775,3832 ----
  }

  /*
+  * 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 ****
--- 3863,3877 ----
                  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,
--- 3926,3932 ----
      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 ****
--- 3934,3948 ----
                       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 ****
--- 4057,4071 ----
                       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)
--- 4155,4166 ----
  /*
   * 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 ****
--- 4186,4192 ----
      {
          case TBLOCK_STARTED:
          case TBLOCK_INPROGRESS:
+         case TBLOCK_IMPLICIT_INPROGRESS:
          case TBLOCK_END:
          case TBLOCK_PREPARE:
          case TBLOCK_SUBINPROGRESS:
*************** RollbackAndReleaseCurrentSubTransaction(
*** 4180,4185 ****
--- 4291,4297 ----
          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 ****
--- 4323,4329 ----
      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 ****
--- 4372,4378 ----
              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 ****
--- 4483,4489 ----
          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 ****
--- 5151,5158 ----
              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..c10d891 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,891 ----
      ListCell   *parsetree_item;
      bool        save_log_statement_stats = log_statement_stats;
      bool        was_logged = false;
!     bool        use_implicit_block;
      char        msec_str[32];

      /*
       * Report query to various monitoring facilities.
       */
*************** exec_simple_query(const char *query_stri
*** 947,959 ****
      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.
--- 946,959 ----
      MemoryContextSwitchTo(oldcontext);

      /*
!      * 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 make
!      * portions of the list be separate transactions.  To represent this
!      * behavior properly in the transaction machinery, we use an "implicit"
!      * transaction block.
       */
!     use_implicit_block = (list_length(parsetree_list) > 1);

      /*
       * Run through the raw parsetree(s) and process each one.
*************** exec_simple_query(const char *query_stri
*** 1001,1006 ****
--- 1001,1016 ----
          /* Make sure we are in a transaction command */
          start_xact_command();

+         /*
+          * If using an implicit transaction block, and we're not already in a
+          * transaction block, start an implicit block to force this statement
+          * to be grouped together with any following ones.  (We must do this
+          * each time through the loop; otherwise, a COMMIT/ROLLBACK in the
+          * list would cause later statements to not be grouped.)
+          */
+         if (use_implicit_block)
+             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,
--- 1108,1114 ----
           */
          (void) PortalRun(portal,
                           FETCH_ALL,
!                          true,    /* always top level */
                           true,
                           receiver,
                           receiver,
*************** exec_simple_query(const char *query_stri
*** 1108,1122 ****

          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
--- 1118,1124 ----

          PortalDrop(portal, false);

!         if (lnext(parsetree_item) == NULL)
          {
              /*
               * If this is the last parsetree of the query string, close down
*************** exec_simple_query(const char *query_stri
*** 1124,1132 ****
               * 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();
          }
--- 1126,1143 ----
               * 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.  Also, if we're using an
!              * implicit transaction block, we must close that out first.
!              */
!             if (use_implicit_block)
!                 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();

--- 1160,1168 ----
      }                            /* 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 last loop
!      * iteration already did 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..a7fdcf4 100644
*** a/src/test/regress/expected/transactions.out
--- b/src/test/regress/expected/transactions.out
*************** ERROR:  portal "ctt" cannot be run
*** 659,664 ****
--- 659,748 ----
  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
+ SELECT 1\; COMMIT\; VACUUM;
+ WARNING:  there is no transaction in progress
+ 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
+ SELECT 1\; COMMIT\; SAVEPOINT sp;
+ WARNING:  there is no transaction in progress
+ 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..82661ab 100644
*** a/src/test/regress/sql/transactions.sql
--- b/src/test/regress/sql/transactions.sql
*************** DROP FUNCTION create_temp_tab();
*** 419,424 ****
--- 419,478 ----
  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;
+ SELECT 1\; COMMIT\; VACUUM;
+
+ -- we disallow savepoint-related commands in implicit-transaction state
+ SELECT 1\; SAVEPOINT sp;
+ SELECT 1\; COMMIT\; 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