A fix and a test case for Bug#12713 "Error in a stored function called from

a SELECT doesn't cause ROLLBACK of statem".

The idea of the fix is to ensure that we always commit the current
statement at the end of dispatch_command(). In order to not issue
redundant disc syncs, an optimization of the two-phase commit
protocol is implemented to bypass the two phase commit if
the transaction is read-only.


mysql-test/suite/binlog/r/binlog_row_mix_innodb_myisam.result:
  Update test results.
mysql-test/suite/binlog/r/binlog_stm_mix_innodb_myisam.result:
  Update test results.
mysql-test/suite/rpl_ndb/t/disabled.def:
  Disable the tests, for which this changeset reveals a bug:
  the injector thread does not always add 'statement commit' to the
  rows injected in circular replication set up.
  To be investigated separately.
sql/ha_ndbcluster_binlog.cc:
  Add close_thread_tables() to run_query: this ensures
  that all tables are closed and there is no pending statement transaction.
sql/handler.cc:
  Implement optimisation of read-only transactions.
  If a transaction consists only of DML statements that do not change
  data, we do not perform a two-phase commit for it 
  (run one phase commit only).
sql/handler.h:
  Implement optimisation of read-only transactions.
  If a transaction consists only of DML statements that do not change
  data, we do not perform a two-phase commit for it 
  (run one phase commit only).
sql/log.cc:
  Mark the binlog transaction read-write whenever it's started.
  We never read from binlog, so it's safe and least intrusive to add
  this mark up here.
sql/log_event.cc:
  Update to the new layout of thd->transaction.
sql/rpl_injector.cc:
  Always commit statement transaction before committing the global one.
sql/sp.cc:
  Ad comments.
sql/sp_head.cc:
  Add comments.
sql/sql_base.cc:
  Commit transaction at the end of the statement. Always.
sql/sql_class.cc:
  Update thd_ha_data to return the right pointer in the new layout.
  
  Fix select_dumpvar::send_data to properly return operation status.
  A test case from commit.inc would lead to an assertion failure in the 
  diagnostics area (double assignment). Not test otherwise by the test suite.
sql/sql_class.h:
  Implement a new layout of storage engine transaction info in which 
  it is easy to access all members related to the handlerton only
  based on ht->slot.
sql/sql_cursor.cc:
  Update to the new layout of thd->transaction.
sql/sql_delete.cc:
  Remove wrong and now redundant calls to ha_autocommit_or_rollback.
  The transaction is committed in one place, at the end of the statement.
  Remove calls to mysql_unlock_tables, since some engines count locks
  and commit statement transaction in unlock_tables(), which essentially
  equates mysql_unlock_tables to ha_autocommit_or_rollback.
  Previously it was necessary to unlock tables soon because we wanted
  to avoid sending of 'ok' packet to the client under locked tables.
  This is no longer necessary, since OK packet is also sent from one place
  at the end of transaction.
sql/sql_do.cc:
  Add DO always clears the error, we must rollback the current
  statement before this happens. Otherwise the statement will be committed,
  and not rolled back in the end.
sql/sql_insert.cc:
  Remove wrong and now redundant calls to ha_autocommit_or_rollback.
  The transaction is committed in one place, at the end of the statement.
  Remove calls to mysql_unlock_tables, since some engines count locks
  and commit statement transaction in unlock_tables(), which essentially
  equates mysql_unlock_tables to ha_autocommit_or_rollback.
  Previously it was necessary to unlock tables soon because we wanted
  to avoid sending of 'ok' packet to the client under locked tables.
  This is no longer necessary, since OK packet is also sent from one place
  at the end of transaction.
sql/sql_load.cc:
  Remove wrong and now redundant calls to ha_autocommit_or_rollback.
  The transaction is committed in one place, at the end of the statement.
  Remove calls to mysql_unlock_tables, since some engines count locks
  and commit statement transaction in unlock_tables(), which essentially
  equates mysql_unlock_tables to ha_autocommit_or_rollback.
  Previously it was necessary to unlock tables soon because we wanted
  to avoid sending of 'ok' packet to the client under locked tables.
  This is no longer necessary, since OK packet is also sent from one place
  at the end of transaction.
sql/sql_parse.cc:
  Implement optimisation of read-only transactions: bypass 2-phase
  commit for them.
  Always commit statement transaction before commiting the global one.
  Fix an unrelated crash in check_table_access, when called from 
  information_schema.
sql/sql_partition.cc:
  Partitions commit at the end of a DDL operation.
  Make sure that send_ok() is done only if the commit has succeeded.
sql/sql_table.cc:
  Use ha_autocommit_or_rollback and end_active_trans everywhere.
  Add end_trans to mysql_admin_table, so that it leaves no pending
  transaction.
sql/sql_udf.cc:
  Remvove a redundant call to close_thread_tables()
sql/sql_update.cc:
  Remove wrong and now redundant calls to ha_autocommit_or_rollback.
  The transaction is committed in one place, at the end of the statement.
  Remove calls to mysql_unlock_tables, since some engines count locks
  and commit statement transaction in unlock_tables(), which essentially
  equates mysql_unlock_tables to ha_autocommit_or_rollback.
  Previously it was necessary to unlock tables soon because we wanted
  to avoid sending of 'ok' packet to the client under locked tables.
  This is no longer necessary, since OK packet is also sent from one place
  at the end of transaction.
mysql-test/include/commit.inc:
  New BitKeeper file ``mysql-test/include/commit.inc''
mysql-test/r/commit_1innodb.result:
  New BitKeeper file ``mysql-test/r/commit_1innodb.result''
mysql-test/t/commit_1innodb.test:
  New BitKeeper file ``mysql-test/t/commit_1innodb.test''

1 files changed, 113 insertions, 9 deletions

diff --git a/sql/handler.h b/sql/handler.h
index b3a4b408589..9800f4974c3 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -721,14 +721,14 @@ struct handlerton
 #define HTON_SUPPORT_LOG_TABLES      (1 << 7) //Engine supports log tables
 #define HTON_NO_PARTITION            (1 << 8) //You can not partition these tables
 
-typedef struct st_thd_trans
+class Ha_trx_info;
+
+struct THD_TRANS
 {
-  /* number of entries in the ht[] */
-  uint        nht;
   /* true is not all entries in the ht[] support 2pc */
   bool        no_2pc;
-  /* storage engines that registered themselves for this transaction */
-  handlerton *ht[MAX_HA];
+  /* storage engines that registered in this transaction */
+  Ha_trx_info *ha_list;
   /* 
     The purpose of this flag is to keep track of non-transactional
     tables that were modified in scope of:
@@ -758,7 +758,106 @@ typedef struct st_thd_trans
     saved value.
   */
   bool modified_non_trans_table;
-} THD_TRANS;
+
+  void reset() { no_2pc= FALSE; modified_non_trans_table= FALSE; }
+};
+
+
+/**
+  Either statement transaction or normal transaction - related
+  thread-specific storage engine data.
+
+  If a storage engine participates in a statement/transaction,
+  an instance of this class is present in
+  thd->transaction.{stmt|all}.ha_list. The addition to
+  {stmt|all}.ha_list is made by trans_register_ha().
+
+  When it's time to commit or rollback, each element of ha_list
+  is used to access storage engine's prepare()/commit()/rollback()
+  methods, and also to evaluate if a full two phase commit is
+  necessary.
+
+  @sa General description of transaction handling in handler.cc.
+*/
+
+class Ha_trx_info
+{
+public:
+  /** Register this storage engine in the given transaction context. */
+  void register_ha(THD_TRANS *trans, handlerton *ht_arg)
+  {
+    DBUG_ASSERT(m_flags == 0);
+    DBUG_ASSERT(m_ht == NULL);
+    DBUG_ASSERT(m_next == NULL);
+
+    m_ht= ht_arg;
+    m_flags= (int) TRX_READ_ONLY; /* Assume read-only at start. */
+
+    m_next= trans->ha_list;
+    trans->ha_list= this;
+  }
+
+  /** Clear, prepare for reuse. */
+  void reset()
+  {
+    m_next= NULL;
+    m_ht= NULL;
+    m_flags= 0;
+  }
+
+  Ha_trx_info() { reset(); }
+
+  void set_trx_read_write()
+  {
+    DBUG_ASSERT(is_started());
+    m_flags|= (int) TRX_READ_WRITE;
+  }
+  bool is_trx_read_write() const
+  {
+    DBUG_ASSERT(is_started());
+    return m_flags & (int) TRX_READ_WRITE;
+  }
+  bool is_started() const { return m_ht != NULL; }
+  /** Mark this transaction read-write if the argument is read-write. */
+  void coalesce_trx_with(const Ha_trx_info *stmt_trx)
+  {
+    /*
+      Must be called only after the transaction has been started.
+      Can be called many times, e.g. when we have many
+      read-write statements in a transaction.
+    */
+    DBUG_ASSERT(is_started());
+    if (stmt_trx->is_trx_read_write())
+      set_trx_read_write();
+  }
+  Ha_trx_info *next() const
+  {
+    DBUG_ASSERT(is_started());
+    return m_next;
+  }
+  handlerton *ht() const
+  {
+    DBUG_ASSERT(is_started());
+    return m_ht;
+  }
+private:
+  enum { TRX_READ_ONLY= 0, TRX_READ_WRITE= 1 };
+  /** Auxiliary, used for ha_list management */
+  Ha_trx_info *m_next;
+  /**
+    Although a given Ha_trx_info instance is currently always used
+    for the same storage engine, 'ht' is not-NULL only when the
+    corresponding storage is a part of a transaction.
+  */
+  handlerton *m_ht;
+  /**
+    Transaction flags related to this engine.
+    Not-null only if this instance is a part of transaction.
+    May assume a combination of enum values above.
+  */
+  uchar       m_flags;
+};
+
 
 enum enum_tx_isolation { ISO_READ_UNCOMMITTED, ISO_READ_COMMITTED,
 			 ISO_REPEATABLE_READ, ISO_SERIALIZABLE};
@@ -1640,8 +1739,15 @@ protected:
     provide useful functionality.
   */
   virtual int rename_table(const char *from, const char *to);
+  /**
+    Delete a table in the engine. Called for base as well as temporary
+    tables.
+  */
   virtual int delete_table(const char *name);
 private:
+  /* Private helpers */
+  inline void mark_trx_read_write();
+private:
   /*
     Low-level primitives for storage engines.  These should be
     overridden by the storage engine class. To call these methods, use
@@ -1821,9 +1927,7 @@ extern TYPELIB tx_isolation_typelib;
 extern TYPELIB myisam_stats_method_typelib;
 extern ulong total_ha, total_ha_2pc;
 
-	/* Wrapper functions */
-#define ha_commit_stmt(thd) (ha_commit_trans((thd), FALSE))
-#define ha_rollback_stmt(thd) (ha_rollback_trans((thd), FALSE))
+       /* Wrapper functions */
 #define ha_commit(thd) (ha_commit_trans((thd), TRUE))
 #define ha_rollback(thd) (ha_rollback_trans((thd), TRUE))