Draft patch that fixes and a sketches test cases for:

Bug#20837 Apparent change of isolation level during transaction,
Bug#46527 COMMIT AND CHAIN RELEASE does not make sense,
Bug#53343 completion_type=1, COMMIT/ROLLBACK AND CHAIN don't 
preserve the isolation level
Bug#53346 completion_type has strange effect in a stored 
procedure/prepared statement

Make thd->tx_isolation mean strictly "current transaction 
isolation level"
Make thd->variables.tx_isolation mean "current session isolation
level".
The current transaction isolation level is now established
at transaction start. If there was a SET TRANSACTION
ISOLATION LEVEL statement, the value is taken from it.
Otherwise, the session value is used.
A change in a session value, made while a transaction is active,
whereas still allowed, no longer has any effect on the
current transaction isolation level. This is an incompatible
change.
A change in a session isolation level, made while there is
no active transaction, overrides SET TRANSACTION statement,
if there was any.
Changed the impelmentation to not look at @@session.completion_type
in the parser, and thus fixed Bug#53346.
Changed the parser to not allow AND NO CHAIN RELEASE,
and thus fixed Bug#46527.
Changed the transaction API to take the current transaction
isolation level into account:
- BEGIN/COMMIT now do preserve the current transaction
isolation level if chaining is on.
- implicit commit, XA COMMIT or XA ROLLBACK or autocommit don't.

10 files changed, 164 insertions, 34 deletions

diff --git a/sql/handler.cc b/sql/handler.cc
index c0a5e2ff55c..34590bd3b42 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -1297,7 +1297,6 @@ int ha_commit_one_phase(THD *thd, bool all)
       if (thd->transaction.changed_tables)
         query_cache.invalidate(thd->transaction.changed_tables);
 #endif
-      thd->variables.tx_isolation=thd->session_tx_isolation;
     }
   }
   /* Free resources and perform other cleanup even for 'empty' transactions. */
@@ -1374,8 +1373,6 @@ int ha_rollback_trans(THD *thd, bool all)
     if (is_real_trans && thd->transaction_rollback_request &&
         thd->transaction.xid_state.xa_state != XA_NOTR)
       thd->transaction.xid_state.rm_error= thd->stmt_da->sql_errno();
-    if (all)
-      thd->variables.tx_isolation=thd->session_tx_isolation;
   }
   /* Always cleanup. Even if nht==0. There may be savepoints. */
   if (is_real_trans)
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 3144192a971..7db70c36d73 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -320,7 +320,7 @@ int thd_sql_command(const THD *thd)
 extern "C"
 int thd_tx_isolation(const THD *thd)
 {
-  return (int) thd->variables.tx_isolation;
+  return (int) thd->tx_isolation;
 }
 
 extern "C"
@@ -922,7 +922,7 @@ void THD::init(void)
   update_lock_default= (variables.low_priority_updates ?
 			TL_WRITE_LOW_PRIORITY :
 			TL_WRITE);
-  session_tx_isolation= (enum_tx_isolation) variables.tx_isolation;
+  tx_isolation= (enum_tx_isolation) variables.tx_isolation;
   update_charset();
   reset_current_stmt_binlog_format_row();
   bzero((char *) &status_var, sizeof(status_var));
diff --git a/sql/sql_class.h b/sql/sql_class.h
index ac1a10f57b9..ca20849dc89 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -2043,8 +2043,31 @@ public:
   uint	     server_status,open_options;
   enum enum_thread_type system_thread;
   uint       select_number;             //number of select (used for EXPLAIN)
-  /* variables.transaction_isolation is reset to this after each commit */
-  enum_tx_isolation session_tx_isolation;
+  /*
+    Current or next transaction isolation level.
+    When a connection is established, the value is taken from
+    @@session.tx_isolation (default transaction isolation for
+    the session), which is in turn taken from @@global.tx_isolation
+    (the global value).
+    If there is no transaction started, this variable
+    holds the value of the next transaction's isolation level.
+    When a transaction starts, the value stored in this variable
+    becomes "actual".
+    At transaction commit or rollback, we assign this variable
+    again from @@session.tx_isolation.
+    The only statement that can otherwise change the value
+    of this variable is SET TRANSACTION ISOLATION LEVEL.
+    Its purpose is to effect the isolation level of the next
+    transaction in this session. When this statement is executed,
+    the value in this variable is changed. However, since
+    this statement is only allowed when there is no active
+    transaction, this assignment (naturally) only affects the
+    upcoming transaction.
+    At the end of the current active transaction the value is
+    be reset again from @@session.tx_isolation, as described
+    above.
+  */
+  enum_tx_isolation tx_isolation;
   enum_check_fields count_cuted_fields;
 
   DYNAMIC_ARRAY user_var_events;        /* For user variables replication */
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
index 6f7acc4a609..1e83b221dac 100644
--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -1971,7 +1971,7 @@ struct LEX: public Query_tables_list
   bool autocommit;
   bool verbose, no_write_to_binlog;
 
-  bool tx_chain, tx_release;
+  enum enum_yes_no_unknown tx_chain, tx_release;
   /*
     Special JOIN::prepare mode: changing of query is prohibited.
     When creating a view, we need to just check its syntax omitting
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index c8faac7c91d..2443b71dbc9 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -4033,33 +4033,65 @@ end_with_restore_list:
     my_ok(thd);
     break;
   case SQLCOM_COMMIT:
+  {
     DBUG_ASSERT(thd->lock == NULL ||
                 thd->locked_tables_mode == LTM_LOCK_TABLES);
+    bool tx_chain= (lex->tx_chain == TVL_YES ||
+                    (thd->variables.completion_type == 1 &&
+                     lex->tx_chain != TVL_NO));
+    bool tx_release= (lex->tx_release == TVL_YES ||
+                      (thd->variables.completion_type == 2 &&
+                       lex->tx_release != TVL_NO));
     if (trans_commit(thd))
       goto error;
     thd->mdl_context.release_transactional_locks();
     /* Begin transaction with the same isolation level. */
-    if (lex->tx_chain && trans_begin(thd))
+    if (tx_chain)
+    {
+      if (trans_begin(thd))
       goto error;
+    }
+    else
+    {
+      /* Reset the isolation level if no chaining transaction. */
+      thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
+    }
     /* Disconnect the current client connection. */
-    if (lex->tx_release)
+    if (tx_release)
       thd->killed= THD::KILL_CONNECTION;
     my_ok(thd);
     break;
+  }
   case SQLCOM_ROLLBACK:
+  {
     DBUG_ASSERT(thd->lock == NULL ||
                 thd->locked_tables_mode == LTM_LOCK_TABLES);
+    bool tx_chain= (lex->tx_chain == TVL_YES ||
+                    (thd->variables.completion_type == 1 &&
+                     lex->tx_chain != TVL_NO));
+    bool tx_release= (lex->tx_release == TVL_YES ||
+                      (thd->variables.completion_type == 2 &&
+                       lex->tx_release != TVL_NO));
     if (trans_rollback(thd))
       goto error;
     thd->mdl_context.release_transactional_locks();
     /* Begin transaction with the same isolation level. */
-    if (lex->tx_chain && trans_begin(thd))
-      goto error;
+    if (tx_chain)
+    {
+      if (trans_begin(thd))
+        goto error;
+    }
+    else
+    {
+      /* Reset the isolation level if no chaining transaction. */
+      thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
+    }
     /* Disconnect the current client connection. */
-    if (lex->tx_release)
+    if (tx_release)
       thd->killed= THD::KILL_CONNECTION;
     my_ok(thd);
     break;
+  }
   case SQLCOM_RELEASE_SAVEPOINT:
     if (trans_release_savepoint(thd, lex->ident))
       goto error;
@@ -4561,12 +4593,22 @@ create_sp_error:
     if (trans_xa_commit(thd))
       goto error;
     thd->mdl_context.release_transactional_locks();
+    /*
+      We've just done a commit, reset transaction
+      isolation level to the session default.
+    */
+    thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
     my_ok(thd);
     break;
   case SQLCOM_XA_ROLLBACK:
     if (trans_xa_rollback(thd))
       goto error;
     thd->mdl_context.release_transactional_locks();
+    /*
+      We've just done a rollback, reset transaction
+      isolation level to the session default.
+    */
+    thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
     my_ok(thd);
     break;
   case SQLCOM_XA_RECOVER:
diff --git a/sql/sql_priv.h b/sql/sql_priv.h
index 20893e0caa8..7e53935700c 100644
--- a/sql/sql_priv.h
+++ b/sql/sql_priv.h
@@ -215,6 +215,11 @@ enum enum_var_type
 
 class sys_var;
 
+enum enum_yes_no_unknown
+{
+  TVL_YES, TVL_NO, TVL_UNKNOWN
+};
+
 #ifdef MYSQL_SERVER
 
 #endif /* MYSQL_SERVER */
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index aa336f3c072..b49e271c06c 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -766,6 +766,7 @@ static bool add_create_index (LEX *lex, Key::Keytype type,
   enum index_hint_type index_hint;
   enum enum_filetype filetype;
   enum Foreign_key::fk_option m_fk_option;
+  enum enum_yes_no_unknown m_yes_no_unk;
   Diag_condition_item_name diag_condition_item_name;
 }
 
@@ -1433,12 +1434,15 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
         table_option opt_if_not_exists opt_no_write_to_binlog
         opt_temporary all_or_any opt_distinct
         opt_ignore_leaves fulltext_options spatial_type union_option
-        start_transaction_opts opt_chain opt_release
+        start_transaction_opts
         union_opt select_derived_init option_type2
         opt_natural_language_mode opt_query_expansion
         opt_ev_status opt_ev_on_completion ev_on_completion opt_ev_comment
         ev_alter_on_schedule_completion opt_ev_rename_to opt_ev_sql_stmt
 
+%type <m_yes_no_unk>
+        opt_chain opt_release
+
 %type <m_fk_option>
         delete_option
 
@@ -13546,16 +13550,16 @@ opt_work:
 
 opt_chain:
           /* empty */
-          { $$= (YYTHD->variables.completion_type == 1); }
-        | AND_SYM NO_SYM CHAIN_SYM { $$=0; }
-        | AND_SYM CHAIN_SYM        { $$=1; }
+          { $$= TVL_UNKNOWN; }
+        | AND_SYM NO_SYM CHAIN_SYM { $$= TVL_NO; }
+        | AND_SYM CHAIN_SYM        { $$= TVL_YES; }
         ;
 
 opt_release:
           /* empty */
-          { $$= (YYTHD->variables.completion_type == 2); }
-        | RELEASE_SYM        { $$=1; }
-        | NO_SYM RELEASE_SYM { $$=0; }
+          { $$= TVL_UNKNOWN; }
+        | RELEASE_SYM        { $$= TVL_YES; }
+        | NO_SYM RELEASE_SYM { $$= TVL_NO; }
 ;
 
 opt_savepoint:
@@ -13568,7 +13572,9 @@ commit:
           {
             LEX *lex=Lex;
             lex->sql_command= SQLCOM_COMMIT;
-            lex->tx_chain= $3; 
+            /* Don't allow AND CHAIN RELEASE. */
+            MYSQL_YYABORT_UNLESS($3 != TVL_YES || $4 != TVL_YES);
+            lex->tx_chain= $3;
             lex->tx_release= $4;
           }
         ;
@@ -13578,7 +13584,9 @@ rollback:
           {
             LEX *lex=Lex;
             lex->sql_command= SQLCOM_ROLLBACK;
-            lex->tx_chain= $3; 
+            /* Don't allow AND CHAIN RELEASE. */
+            MYSQL_YYABORT_UNLESS($3 != TVL_YES || $4 != TVL_YES);
+            lex->tx_chain= $3;
             lex->tx_release= $4;
           }
         | ROLLBACK_SYM opt_work
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index b8312fc3255..2062af88b1a 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -2022,24 +2022,38 @@ static bool check_tx_isolation(sys_var *self, THD *thd, set_var *var)
   return FALSE;
 }
 
-/*
-  If one doesn't use the SESSION modifier, the isolation level
-  is only active for the next command.
-*/
-static bool fix_tx_isolation(sys_var *self, THD *thd, enum_var_type type)
+
+bool Sys_var_tx_isolation::session_update(THD *thd, set_var *var)
 {
-  if (type == OPT_SESSION)
-    thd->session_tx_isolation= (enum_tx_isolation)thd->variables.tx_isolation;
-  return false;
+  if (var->type == OPT_SESSION && Sys_var_enum::session_update(thd, var))
+    return TRUE;
+  if (var->type == OPT_DEFAULT || !thd->in_active_multi_stmt_transaction())
+  {
+    /*
+      Update the isolation level of the next transaction.
+      I.e. if one did:
+      COMMIT;
+      SET SESSION ISOLATION LEVEL ...
+      BEGIN; <-- this transaction has the new isolation
+      Note, that in case of:
+      COMMIT;
+      SET TRANSACTION ISOLATION LEVEL ...
+      SET SESSION ISOLATION LEVEL ...
+      BEGIN; <-- the session isolation level is used, not the
+      result of SET TRANSACTION statement.
+     */
+    thd->tx_isolation= (enum_tx_isolation) var->save_result.ulonglong_value;
+  }
+  return FALSE;
 }
 
+
 // NO_CMD_LINE - different name of the option
-static Sys_var_enum Sys_tx_isolation(
+static Sys_var_tx_isolation Sys_tx_isolation(
        "tx_isolation", "Default transaction isolation level",
        SESSION_VAR(tx_isolation), NO_CMD_LINE,
        tx_isolation_names, DEFAULT(ISO_REPEATABLE_READ),
-       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_tx_isolation),
-       ON_UPDATE(fix_tx_isolation));
+       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_tx_isolation));
 
 static Sys_var_ulonglong Sys_tmp_table_size(
        "tmp_table_size",
diff --git a/sql/sys_vars.h b/sql/sys_vars.h
index c4855ef4fd3..fbc48573487 100644
--- a/sql/sys_vars.h
+++ b/sql/sys_vars.h
@@ -1599,6 +1599,22 @@ public:
   { return type != STRING_RESULT; }
 };
 
+
+class Sys_var_tx_isolation: public Sys_var_enum
+{
+public:
+  Sys_var_tx_isolation(const char *name_arg,
+          const char *comment, int flag_args, ptrdiff_t off, size_t size,
+          CMD_LINE getopt,
+          const char *values[], uint def_val, PolyLock *lock,
+          enum binlog_status_enum binlog_status_arg,
+          on_check_function on_check_func)
+    :Sys_var_enum(name_arg, comment, flag_args, off, size, getopt,
+                  values, def_val, lock, binlog_status_arg, on_check_func)
+  {}
+  virtual bool session_update(THD *thd, set_var *var);
+};
+
 /****************************************************************************
   Used templates
 ****************************************************************************/
diff --git a/sql/transaction.cc b/sql/transaction.cc
index 5047de1ccdc..78551d6fcf7 100644
--- a/sql/transaction.cc
+++ b/sql/transaction.cc
@@ -96,7 +96,18 @@ bool trans_begin(THD *thd, uint flags)
 
   DBUG_ASSERT(!thd->locked_tables_mode);
 
-  if (trans_commit_implicit(thd))
+  if (thd->in_multi_stmt_transaction_mode() ||
+      (thd->variables.option_bits & OPTION_TABLE_LOCK))
+  {
+    thd->variables.option_bits&= ~OPTION_TABLE_LOCK;
+    thd->server_status&= ~SERVER_STATUS_IN_TRANS;
+    res= test(ha_commit_trans(thd, TRUE));
+  }
+
+  thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
+  thd->transaction.all.modified_non_trans_table= FALSE;
+
+  if (res)
     DBUG_RETURN(TRUE);
 
   /*
@@ -182,6 +193,14 @@ bool trans_commit_implicit(THD *thd)
   thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
   thd->transaction.all.modified_non_trans_table= FALSE;
 
+  /*
+    Upon implicit commit, reset the current transaction
+    isolation level. We do not care about
+    @@session.completion_type since it's documented
+    to not have any effect on implicit commit.
+  */
+  thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
+
   DBUG_RETURN(res);
 }
 
@@ -234,7 +253,11 @@ bool trans_commit_stmt(THD *thd)
   DBUG_ENTER("trans_commit_stmt");
   int res= FALSE;
   if (thd->transaction.stmt.ha_list)
+  {
     res= ha_commit_trans(thd, FALSE);
+    if (! thd->in_active_multi_stmt_transaction())
+      thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
+  }
 
   if (res)
     /*
@@ -265,6 +288,8 @@ bool trans_rollback_stmt(THD *thd)
     ha_rollback_trans(thd, FALSE);
     if (thd->transaction_rollback_request && !thd->in_sub_stmt)
       ha_rollback_trans(thd, TRUE);
+    if (! thd->in_active_multi_stmt_transaction())
+      thd->tx_isolation= (enum_tx_isolation) thd->variables.tx_isolation;
   }
 
   RUN_HOOK(transaction, after_rollback, (thd, FALSE));