MDEV-33828 : Transactional commit not supported by involved engine(s)

Problem was too tight condition on ha_commit_trans to not
allow non transactional storage engines participate 2pc
in Galera case. This is required because transaction
using e.g. procedures might read mysql.proc table inside
a trasaction and these tables use at the moment Aria
storage engine that does not support 2pc.

Fixed by allowing read only transactions to storage
engines that do not support two phase commit to participate
2pc transaction. These will be committed later separately.

Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>

mysql-test/suite/galera/r/MDEV-33828.result

@@ -0,0 +1,41 @@
+connection node_2;
+connection node_1;
+SET AUTOCOMMIT=ON;
+SELECT @@autocommit;
+@@autocommit
+1
+SET LOCAL enforce_storage_engine=InnoDB;
+CREATE TABLE t1(id int not null primary key auto_increment, name varchar(64)) ENGINE=InnoDB;
+INSERT INTO t1(name) VALUES ('name1'),('name3'),('name6'),('name2');
+CREATE PROCEDURE sel_proc()
+BEGIN
+DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END;
+SELECT * FROM t1;
+END|
+CREATE PROCEDURE ins_proc()
+BEGIN
+DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END;
+INSERT INTO t1 VALUES ('name_proc');
+END|
+SET AUTOCOMMIT=OFF;
+SELECT @@autocommit;
+@@autocommit
+0
+START TRANSACTION;
+insert into t1(name) values('name10');
+select param_list, returns, db, type from mysql.proc where name='sel_proc';
+param_list	returns	db	type
+		test	PROCEDURE
+call ins_proc();
+COMMIT;
+SET AUTOCOMMIT=ON;
+SELECT * FROM t1;
+id	name
+1	name1
+3	name3
+5	name6
+7	name2
+9	name10
+DROP TABLE t1;
+DROP PROCEDURE sel_proc;
+DROP PROCEDURE ins_proc;

mysql-test/suite/galera/r/mdev-22063.result

@@ -79,7 +79,6 @@ INSERT INTO t3(id) SELECT seq FROM seq_1_to_1000;
 REPLACE INTO t4 SELECT * FROM t1;
 REPLACE INTO t5 SELECT * FROM t2;
 REPLACE INTO t6 SELECT * FROM t3;
-ERROR HY000: Transactional commit not supported by involved engine(s)
 REPLACE INTO t7 SELECT * FROM t2;
 REPLACE INTO t8 SELECT * FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t1;
@@ -97,9 +96,9 @@ EXPECT_1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
 EXPECT_1000
 1000
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
-EXPECT_0
-0
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
+EXPECT_1000
+1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 EXPECT_1000
 1000
@@ -122,9 +121,9 @@ EXPECT_1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
 EXPECT_1000
 1000
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
-EXPECT_0
-0
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
+EXPECT_1000
+1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 EXPECT_1000
 1000
@@ -148,7 +147,6 @@ INSERT INTO t3(id) SELECT seq FROM seq_1_to_1000;
 INSERT INTO t4 SELECT * FROM t1;
 INSERT INTO t5 SELECT * FROM t2;
 INSERT INTO t6 SELECT * FROM t3;
-ERROR HY000: Transactional commit not supported by involved engine(s)
 INSERT INTO t7 SELECT * FROM t2;
 INSERT INTO t8 SELECT * FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t1;
@@ -166,9 +164,9 @@ EXPECT_1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
 EXPECT_1000
 1000
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
-EXPECT_0
-0
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
+EXPECT_1000
+1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 EXPECT_1000
 1000
@@ -191,9 +189,9 @@ EXPECT_1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
 EXPECT_1000
 1000
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
-EXPECT_0
-0
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
+EXPECT_1000
+1000
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 EXPECT_1000
 1000

mysql-test/suite/galera/t/MDEV-33828.cnf

@@ -0,0 +1,4 @@
+!include ../galera_2nodes.cnf
+
+[mysqld]
+log-bin

mysql-test/suite/galera/t/MDEV-33828.test

@@ -0,0 +1,45 @@
+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+--source include/have_aria.inc
+
+SET AUTOCOMMIT=ON;
+SELECT @@autocommit;
+
+SET LOCAL enforce_storage_engine=InnoDB;
+
+CREATE TABLE t1(id int not null primary key auto_increment, name varchar(64)) ENGINE=InnoDB;
+INSERT INTO t1(name) VALUES ('name1'),('name3'),('name6'),('name2');
+
+DELIMITER |;
+CREATE PROCEDURE sel_proc()
+BEGIN
+  DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END;
+  SELECT * FROM t1;
+END|
+
+CREATE PROCEDURE ins_proc()
+BEGIN
+  DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END;
+  INSERT INTO t1 VALUES ('name_proc');
+END|
+DELIMITER ;|
+
+SET AUTOCOMMIT=OFF;
+SELECT @@autocommit;
+
+START TRANSACTION;
+
+insert into t1(name) values('name10');
+
+select param_list, returns, db, type from mysql.proc where name='sel_proc';
+
+call ins_proc();
+
+COMMIT;
+
+SET AUTOCOMMIT=ON;
+
+SELECT * FROM t1;
+DROP TABLE t1;
+DROP PROCEDURE sel_proc;
+DROP PROCEDURE ins_proc;

mysql-test/suite/galera/t/mdev-22063.test

@@ -81,8 +81,6 @@ INSERT INTO t3(id) SELECT seq FROM seq_1_to_1000;
 
 REPLACE INTO t4 SELECT * FROM t1;
 REPLACE INTO t5 SELECT * FROM t2;
-# For some reason Aria storage engine does register_ha
---error ER_ERROR_DURING_COMMIT
 REPLACE INTO t6 SELECT * FROM t3;
 REPLACE INTO t7 SELECT * FROM t2;
 REPLACE INTO t8 SELECT * FROM t3;
@@ -92,7 +90,7 @@ SELECT COUNT(*) AS EXPECT_1000 FROM t2;
 SELECT COUNT(*) AS EXPECT_1000 FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t4;
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 SELECT COUNT(*) AS EXPECT_1000 FROM t8;
 
@@ -107,7 +105,7 @@ SELECT COUNT(*) AS EXPECT_1000 FROM t2;
 SELECT COUNT(*) AS EXPECT_1000 FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t4;
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 SELECT COUNT(*) AS EXPECT_1000 FROM t8;
 
@@ -131,8 +129,6 @@ INSERT INTO t3(id) SELECT seq FROM seq_1_to_1000;
 
 INSERT INTO t4 SELECT * FROM t1;
 INSERT INTO t5 SELECT * FROM t2;
-# For some reason Aria storage engine does register_ha
---error ER_ERROR_DURING_COMMIT
 INSERT INTO t6 SELECT * FROM t3;
 INSERT INTO t7 SELECT * FROM t2;
 INSERT INTO t8 SELECT * FROM t3;
@@ -142,7 +138,7 @@ SELECT COUNT(*) AS EXPECT_1000 FROM t2;
 SELECT COUNT(*) AS EXPECT_1000 FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t4;
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 SELECT COUNT(*) AS EXPECT_1000 FROM t8;
 
@@ -157,7 +153,7 @@ SELECT COUNT(*) AS EXPECT_1000 FROM t2;
 SELECT COUNT(*) AS EXPECT_1000 FROM t3;
 SELECT COUNT(*) AS EXPECT_1000 FROM t4;
 SELECT COUNT(*) AS EXPECT_1000 FROM t5;
-SELECT COUNT(*) AS EXPECT_0 FROM t6;
+SELECT COUNT(*) AS EXPECT_1000 FROM t6;
 SELECT COUNT(*) AS EXPECT_1000 FROM t7;
 SELECT COUNT(*) AS EXPECT_1000 FROM t8;
 

sql/handler.cc

@@ -1531,6 +1531,29 @@ ha_check_and_coalesce_trx_read_only(THD *thd, Ha_trx_info *ha_list,
   return rw_ha_count;
 }
 
+#ifdef WITH_WSREP
+/**
+  Check if transaction contains storage engine not supporting
+  two-phase commit and transaction is read-write.
+
+  @retval
+    true Transaction contains storage engine not supporting
+         two phase commit and transaction is read-write
+  @retval
+    false otherwise
+*/
+static bool wsrep_have_no2pc_rw_ha(Ha_trx_info* ha_list)
+{
+  for (Ha_trx_info *ha_info=ha_list; ha_info; ha_info= ha_info->next())
+  {
+    handlerton *ht= ha_info->ht();
+    // Transaction is read-write and handler does not support 2pc
+    if (ha_info->is_trx_read_write() && ht->prepare==0)
+      return true;
+  }
+  return false;
+}
+#endif /* WITH_WSREP */
 
 /**
   @retval
@@ -1734,14 +1757,18 @@ int ha_commit_trans(THD *thd, bool all)
     */
     if (run_wsrep_hooks)
     {
-      // This commit involves more than one storage engine and requires
-      // two phases, but some engines don't support it.
-      // Issue a message to the client and roll back the transaction.
-      if (trans->no_2pc && rw_ha_count > 1)
+      // This commit involves storage engines that do not support two phases.
+      // We allow read only transactions to such storage engines but not
+      // read write transactions.
+      if (trans->no_2pc && rw_ha_count > 1 && wsrep_have_no2pc_rw_ha(trans->ha_list))
       {
-	// REPLACE|INSERT INTO ... SELECT uses TOI for MyISAM|Aria
-	if (WSREP(thd) && thd->wsrep_cs().mode() != wsrep::client_state::m_toi)
-	{
+        // This commit involves more than one storage engine and requires
+        // two phases, but some engines don't support it.
+        // Issue a message to the client and roll back the transaction.
+
+        // REPLACE|INSERT INTO ... SELECT uses TOI for MyISAM|Aria
+        if (WSREP(thd) && thd->wsrep_cs().mode() != wsrep::client_state::m_toi)
+        {
           my_message(ER_ERROR_DURING_COMMIT, "Transactional commit not supported "
                      "by involved engine(s)", MYF(0));
           error= 1;