Clean-up, give better names, add comments to

thd->in_multi_stmt_transaction() and thd->active_transaction().


include/mysql_com.h:
  Comment SERVER_STATUS_IN_TRANS flag.
sql/ha_ndbcluster.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/handler.cc:
  Add comments.
sql/log.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/log_event.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/sql_base.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/sql_cache.cc:
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().
sql/sql_class.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/sql_class.h:
  Rename and comment two transaction processing- related methods.
sql/sql_parse.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().
sql/sql_prepare.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
sql/sql_rename.cc:
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().
sql/sql_table.cc:
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().
sql/sys_vars.cc:
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().
sql/transaction.cc:
  Rename: thd->in_multi_stmt_transaction() -> 
  thd->in_multi_stmt_transaction_mode().
  Rename: thd->active_transaction() ->
  thd->in_active_multi_stmt_transaction().

Author: Konstantin Osipov

include/mysql_com.h

@@ -197,7 +197,14 @@ enum enum_server_command
                                                & ~CLIENT_COMPRESS) \
                                                & ~CLIENT_SSL_VERIFY_SERVER_CERT)
 
-#define SERVER_STATUS_IN_TRANS     1	/* Transaction has started */
+/**
+  Is raised when a multi-statement transaction
+  has been started, either explicitly, by means
+  of BEGIN or COMMIT AND CHAIN, or
+  implicitly, by the first transactional
+  statement, when autocommit=off.
+*/
+#define SERVER_STATUS_IN_TRANS     1
 #define SERVER_STATUS_AUTOCOMMIT   2	/* Server in auto_commit mode */
 #define SERVER_MORE_RESULTS_EXISTS 8    /* Multi query - next query exists */
 #define SERVER_QUERY_NO_GOOD_INDEX_USED 16

sql/ha_ndbcluster.cc

@@ -4628,7 +4628,7 @@ int ha_ndbcluster::start_statement(THD *thd,
   trans_register_ha(thd, FALSE, ndbcluster_hton);
   if (!thd_ndb->trans)
   {
-    if (thd->in_multi_stmt_transaction())
+    if (thd->in_multi_stmt_transaction_mode())
       trans_register_ha(thd, TRUE, ndbcluster_hton);
     DBUG_PRINT("trans",("Starting transaction"));      
     thd_ndb->trans= ndb->startTransaction();
@@ -4698,7 +4698,7 @@ int ha_ndbcluster::init_handler_for_statement(THD *thd, Thd_ndb *thd_ndb)
   }
 #endif
 
-  if (thd->in_multi_stmt_transaction())
+  if (thd->in_multi_stmt_transaction_mode())
   {
     const void *key= m_table;
     HASH_SEARCH_STATE state;
@@ -4782,7 +4782,7 @@ int ha_ndbcluster::external_lock(THD *thd, int lock_type)
     if (opt_ndb_cache_check_time && m_rows_changed)
     {
       DBUG_PRINT("info", ("Rows has changed and util thread is running"));
-      if (thd->in_multi_stmt_transaction())
+      if (thd->in_multi_stmt_transaction_mode())
       {
         DBUG_PRINT("info", ("Add share to list of tables to be invalidated"));
         /* NOTE push_back allocates memory using transactions mem_root! */
@@ -4801,7 +4801,7 @@ int ha_ndbcluster::external_lock(THD *thd, int lock_type)
       DBUG_PRINT("trans", ("Last external_lock"));
       PRINT_OPTION_FLAGS(thd);
 
-      if (!thd->in_multi_stmt_transaction())
+      if (!thd->in_multi_stmt_transaction_mode())
       {
         if (thd_ndb->trans)
         {
@@ -4911,7 +4911,7 @@ static int ndbcluster_commit(handlerton *hton, THD *thd, bool all)
   PRINT_OPTION_FLAGS(thd);
   DBUG_PRINT("enter", ("Commit %s", (all ? "all" : "stmt")));
   thd_ndb->start_stmt_count= 0;
-  if (trans == NULL || (!all && thd->in_multi_stmt_transaction()))
+  if (trans == NULL || (!all && thd->in_multi_stmt_transaction_mode()))
   {
     /*
       An odditity in the handler interface is that commit on handlerton
@@ -4981,7 +4981,7 @@ static int ndbcluster_rollback(handlerton *hton, THD *thd, bool all)
   DBUG_ASSERT(ndb);
   thd_ndb->start_stmt_count= 0;
   if (trans == NULL || (!all &&
-      thd->in_multi_stmt_transaction()))
+      thd->in_multi_stmt_transaction_mode()))
   {
     /* Ignore end-of-statement until real rollback or commit is called */
     DBUG_PRINT("info", ("Rollback before start or end-of-statement only"));
@@ -8271,7 +8271,7 @@ ndbcluster_cache_retrieval_allowed(THD *thd,
   DBUG_ENTER("ndbcluster_cache_retrieval_allowed");
   DBUG_PRINT("enter", ("dbname: %s, tabname: %s", dbname, tabname));
 
-  if (thd->in_multi_stmt_transaction())
+  if (thd->in_multi_stmt_transaction_mode())
   {
     DBUG_PRINT("exit", ("No, don't use cache in transaction"));
     DBUG_RETURN(FALSE);
@@ -8339,7 +8339,7 @@ ha_ndbcluster::register_query_cache_table(THD *thd,
   DBUG_ENTER("ha_ndbcluster::register_query_cache_table");
   DBUG_PRINT("enter",("dbname: %s, tabname: %s", m_dbname, m_tabname));
 
-  if (thd->in_multi_stmt_transaction())
+  if (thd->in_multi_stmt_transaction_mode())
   {
     DBUG_PRINT("exit", ("Can't register table during transaction"));
     DBUG_RETURN(FALSE);

sql/handler.cc

@@ -1245,17 +1245,30 @@ int ha_commit_trans(THD *thd, bool all)
 /**
   @note
   This function does not care about global read lock. A caller should.
+
+  @param[in]  all  Is set in case of explicit commit
+                   (COMMIT statement), or implicit commit
+                   issued by DDL. Is not set when called
+                   at the end of statement, even if
+                   autocommit=1.
 */
+
 int ha_commit_one_phase(THD *thd, bool all)
 {
   int error=0;
   THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt;
   /*
     "real" is a nick name for a transaction for which a commit will
     make persistent changes. E.g. a 'stmt' transaction inside a 'all'
-    transation is not 'real': even though it's possible to commit it,
+    transaction is not 'real': even though it's possible to commit it,
     the changes are not durable as they might be rolled back if the
     enclosing 'all' transaction is rolled back.
+    We establish the value of 'is_real_trans' by checking
+    if it's an explicit COMMIT/BEGIN statement, or implicit
+    commit issued by DDL (all == TRUE), or if we're running
+    in autocommit mode (it's only in the autocommit mode
+    ha_commit_one_phase() can be called with an empty
+    transaction.all.ha_list, see why in trans_register_ha()).
   */
   bool is_real_trans=all || thd->transaction.all.ha_list == 0;
   Ha_trx_info *ha_info= trans->ha_list, *ha_info_next;
@@ -1303,9 +1316,15 @@ int ha_rollback_trans(THD *thd, bool all)
   /*
     "real" is a nick name for a transaction for which a commit will
     make persistent changes. E.g. a 'stmt' transaction inside a 'all'
-    transation is not 'real': even though it's possible to commit it,
+    transaction is not 'real': even though it's possible to commit it,
     the changes are not durable as they might be rolled back if the
     enclosing 'all' transaction is rolled back.
+    We establish the value of 'is_real_trans' by checking
+    if it's an explicit COMMIT or BEGIN statement, or implicit
+    commit issued by DDL (in these cases all == TRUE),
+    or if we're running in autocommit mode (it's only in the autocommit mode
+    ha_commit_one_phase() is called with an empty
+    transaction.all.ha_list, see why in trans_register_ha()).
   */
   bool is_real_trans=all || thd->transaction.all.ha_list == 0;
   DBUG_ENTER("ha_rollback_trans");
@@ -1358,7 +1377,7 @@ int ha_rollback_trans(THD *thd, bool all)
     if (all)
       thd->variables.tx_isolation=thd->session_tx_isolation;
   }
-  /* Always cleanup. Even if there nht==0. There may be savepoints. */
+  /* Always cleanup. Even if nht==0. There may be savepoints. */
   if (is_real_trans)
     thd->transaction.cleanup();
   if (all)

sql/log.cc

@@ -1686,7 +1686,7 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all)
   DBUG_PRINT("debug",
              ("all: %d, in_transaction: %s, all.modified_non_trans_table: %s, stmt.modified_non_trans_table: %s",
               all,
-              YESNO(thd->in_multi_stmt_transaction()),
+              YESNO(thd->in_multi_stmt_transaction_mode()),
               YESNO(thd->transaction.all.modified_non_trans_table),
               YESNO(thd->transaction.stmt.modified_non_trans_table)));
 
@@ -4267,7 +4267,7 @@ bool use_trans_cache(const THD* thd, bool is_transactional)
 */
 bool ending_trans(THD* thd, const bool all)
 {
-  return (all || (!all && !thd->in_multi_stmt_transaction()));
+  return (all || (!all && !thd->in_multi_stmt_transaction_mode()));
 }
 
 /**
@@ -4370,7 +4370,7 @@ THD::binlog_start_trans_and_stmt()
       cache_mngr->trx_cache.get_prev_position() == MY_OFF_T_UNDEF)
   {
     this->binlog_set_stmt_begin();
-    if (in_multi_stmt_transaction())
+    if (in_multi_stmt_transaction_mode())
       trans_register_ha(this, TRUE, binlog_hton);
     trans_register_ha(this, FALSE, binlog_hton);
     /*

sql/log_event.cc

@@ -2485,13 +2485,13 @@ Query_log_event::Query_log_event(THD* thd_arg, const char* query_arg,
       implicit_commit= TRUE;
       break;
     case SQLCOM_DROP_TABLE:
-      force_trans= lex->drop_temporary && thd->in_multi_stmt_transaction();
+      force_trans= lex->drop_temporary && thd->in_multi_stmt_transaction_mode();
       implicit_commit= !force_trans;
       break;
     case SQLCOM_ALTER_TABLE:
     case SQLCOM_CREATE_TABLE:
       force_trans= (lex->create_info.options & HA_LEX_CREATE_TMP_TABLE) &&
-                    thd->in_multi_stmt_transaction();
+                    thd->in_multi_stmt_transaction_mode();
       implicit_commit= !force_trans &&
                        !(lex->select_lex.item_list.elements &&
                          thd->is_current_stmt_binlog_format_row());

sql/sql_base.cc

@@ -1558,7 +1558,7 @@ void close_thread_tables(THD *thd)
     - If in autocommit mode, or outside a transactional context,
     automatically release metadata locks of the current statement.
   */
-  if (! thd->in_multi_stmt_transaction() &&
+  if (! thd->in_multi_stmt_transaction_mode() &&
       ! (thd->state_flags & Open_tables_state::BACKUPS_AVAIL))
   {
     thd->mdl_context.release_transactional_locks();
@@ -3783,7 +3783,7 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list)
 Open_table_context::Open_table_context(THD *thd, ulong timeout)
   :m_action(OT_NO_ACTION),
    m_start_of_statement_svp(thd->mdl_context.mdl_savepoint()),
-   m_has_locks((thd->in_multi_stmt_transaction() &&
+   m_has_locks((thd->in_multi_stmt_transaction_mode() &&
                 thd->mdl_context.has_locks()) ||
                 thd->mdl_context.trans_sentinel()),
    m_global_mdl_request(NULL),

sql/sql_cache.cc

@@ -1177,7 +1177,7 @@ void Query_cache::store_query(THD *thd, TABLE_LIST *tables_used)
     DBUG_ASSERT(flags.protocol_type != (unsigned int) Protocol::PROTOCOL_LOCAL);
     flags.more_results_exists= test(thd->server_status &
                                     SERVER_MORE_RESULTS_EXISTS);
-    flags.in_trans= test(thd->server_status & SERVER_STATUS_IN_TRANS);
+    flags.in_trans= thd->in_active_multi_stmt_transaction();
     flags.autocommit= test(thd->server_status & SERVER_STATUS_AUTOCOMMIT);
     flags.pkt_nr= net->pkt_nr;
     flags.character_set_client_num=
@@ -1470,7 +1470,7 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
   flags.protocol_type= (unsigned int) thd->protocol->type();
   flags.more_results_exists= test(thd->server_status &
                                   SERVER_MORE_RESULTS_EXISTS);
-  flags.in_trans= test(thd->server_status & SERVER_STATUS_IN_TRANS);
+  flags.in_trans= thd->in_active_multi_stmt_transaction();
   flags.autocommit= test(thd->server_status & SERVER_STATUS_AUTOCOMMIT);
   flags.pkt_nr= thd->net.pkt_nr;
   flags.character_set_client_num= thd->variables.character_set_client->number;
@@ -1541,7 +1541,7 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
   }
   DBUG_PRINT("qcache", ("Query have result 0x%lx", (ulong) query));
 
-  if (thd->in_multi_stmt_transaction() &&
+  if (thd->in_multi_stmt_transaction_mode() &&
       (query->tables_type() & HA_CACHE_TBL_TRANSACT))
   {
     DBUG_PRINT("qcache",
@@ -1698,7 +1698,7 @@ void Query_cache::invalidate(THD *thd, TABLE_LIST *tables_used,
   if (is_disabled())
     DBUG_VOID_RETURN;
 
-  using_transactions= using_transactions && thd->in_multi_stmt_transaction();
+  using_transactions= using_transactions && thd->in_multi_stmt_transaction_mode();
   for (; tables_used; tables_used= tables_used->next_local)
   {
     DBUG_ASSERT(!using_transactions || tables_used->table!=0);
@@ -1782,7 +1782,7 @@ void Query_cache::invalidate(THD *thd, TABLE *table,
   if (is_disabled())
     DBUG_VOID_RETURN;
 
-  using_transactions= using_transactions && thd->in_multi_stmt_transaction();
+  using_transactions= using_transactions && thd->in_multi_stmt_transaction_mode();
   if (using_transactions && 
       (table->file->table_cache_type() == HA_CACHE_TBL_TRANSACT))
     thd->add_changed_table(table);
@@ -1800,7 +1800,7 @@ void Query_cache::invalidate(THD *thd, const char *key, uint32  key_length,
   if (is_disabled())
    DBUG_VOID_RETURN;
 
-  using_transactions= using_transactions && thd->in_multi_stmt_transaction();
+  using_transactions= using_transactions && thd->in_multi_stmt_transaction_mode();
   if (using_transactions) // used for innodb => has_transactions() is TRUE
     thd->add_changed_table(key, key_length);
   else
@@ -3572,7 +3572,7 @@ Query_cache::is_cacheable(THD *thd, size_t query_len, const char *query,
                                                 tables_type)))
       DBUG_RETURN(0);
 
-    if (thd->in_multi_stmt_transaction() &&
+    if (thd->in_multi_stmt_transaction_mode() &&
 	((*tables_type)&HA_CACHE_TBL_TRANSACT))
     {
       DBUG_PRINT("qcache", ("not in autocommin mode"));

sql/sql_class.cc

@@ -1440,7 +1440,7 @@ void THD::add_changed_table(TABLE *table)
 {
   DBUG_ENTER("THD::add_changed_table(table)");
 
-  DBUG_ASSERT(in_multi_stmt_transaction() && table->file->has_transactions());
+  DBUG_ASSERT(in_multi_stmt_transaction_mode() && table->file->has_transactions());
   add_changed_table(table->s->table_cache_key.str,
                     (long) table->s->table_cache_key.length);
   DBUG_VOID_RETURN;

sql/sql_class.h

@@ -2309,10 +2309,6 @@ class THD :public Statement,
   {
     return limit_found_rows;
   }
-  inline bool active_transaction()
-  {
-    return server_status & SERVER_STATUS_IN_TRANS;
-  }
   /**
     Returns TRUE if session is in a multi-statement transaction mode.
 
@@ -2323,11 +2319,60 @@ class THD :public Statement,
     OPTION_BEGIN: Regardless of the autocommit status, a multi-statement
     transaction can be explicitly started with the statements "START
     TRANSACTION", "BEGIN [WORK]", "[COMMIT | ROLLBACK] AND CHAIN", etc.
+
+    Note: this doesn't tell you whether a transaction is active.
+    A session can be in multi-statement transaction mode, and yet
+    have no active transaction, e.g., in case of:
+    set @@autocommit=0;
+    set @a= 3;                                     <-- these statements don't
+    set transaction isolation level serializable;  <-- start an active
+    flush tables;                                  <-- transaction
+
+    I.e. for the above scenario this function returns TRUE, even
+    though no active transaction has begun.
+    @sa in_active_multi_stmt_transaction()
   */
-  inline bool in_multi_stmt_transaction()
+  inline bool in_multi_stmt_transaction_mode()
   {
     return variables.option_bits & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN);
   }
+  /**
+    TRUE if the session is in a multi-statement transaction mode
+    (@sa in_multi_stmt_transaction_mode()) *and* there is an
+    active transaction, i.e. there is an explicit start of a
+    transaction with BEGIN statement, or implicit with a
+    statement that uses a transactional engine.
+
+    For example, these scenarios don't start an active transaction
+    (even though the server is in multi-statement transaction mode):
+
+    set @@autocommit=0;
+    select * from nontrans_table;
+    set @var=TRUE;
+    flush tables;
+
+    Note, that even for a statement that starts a multi-statement
+    transaction (i.e. select * from trans_table), this
+    flag won't be set until we open the statement's tables
+    and the engines register themselves for the transaction
+    (see trans_register_ha()),
+    hence this method is reliable to use only after
+    open_tables() has completed.
+
+    Why do we need a flag?
+    ----------------------
+    We need to maintain a (at first glance redundant)
+    session flag, rather than looking at thd->transaction.all.ha_list
+    because of explicit start of a transaction with BEGIN. 
+
+    I.e. in case of
+    BEGIN;
+    select * from nontrans_t1; <-- in_active_multi_stmt_transaction() is true
+  */
+  inline bool in_active_multi_stmt_transaction()
+  {
+    return server_status & SERVER_STATUS_IN_TRANS;
+  }
   inline bool fill_derived_tables()
   {
     return !stmt_arena->is_stmt_prepare() && !lex->only_view_structure();

sql/sql_parse.cc

@@ -2765,7 +2765,7 @@ case SQLCOM_PREPARE:
     client thread has locked tables
   */
   if (thd->locked_tables_mode ||
-      thd->active_transaction() || thd->global_read_lock.is_acquired())
+      thd->in_active_multi_stmt_transaction() || thd->global_read_lock.is_acquired())
   {
     my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
                ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
@@ -3273,7 +3273,7 @@ case SQLCOM_PREPARE:
       Don't allow this within a transaction because we want to use
       re-generate table
     */
-    if (thd->active_transaction())
+    if (thd->in_active_multi_stmt_transaction())
     {
       my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
                  ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
@@ -4711,6 +4711,9 @@ case SQLCOM_PREPARE:
     thd->global_read_lock.start_waiting_global_read_lock(thd);
   }
 
+  DBUG_ASSERT(!thd->in_active_multi_stmt_transaction() ||
+               thd->in_multi_stmt_transaction_mode());
+
   if (stmt_causes_implicit_commit(thd, CF_IMPLICIT_COMMIT_END))
   {
     /* If commit fails, we should be able to reset the OK status. */
@@ -5524,7 +5527,7 @@ void THD::reset_for_next_command()
     OPTION_STATUS_NO_TRANS_UPDATE | OPTION_KEEP_LOG to not get warnings
     in ha_rollback_trans() about some tables couldn't be rolled back.
   */
-  if (!thd->in_multi_stmt_transaction())
+  if (!thd->in_multi_stmt_transaction_mode())
   {
     thd->variables.option_bits&= ~OPTION_KEEP_LOG;
     thd->transaction.all.modified_non_trans_table= FALSE;