MDEV-16710, Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transactionbb-10.2-mdev16428

rollback_to_stmt_savepoint() calls do_rollback_to_savepoint(). This removes
the changes made by this statement, and also removes the set savepoint.

Before we start processing the next statement, we need to set the new
savepoint, so we will have something to rollback to if the next statement
fails.

Since rollback_to_stmt_savepoint always sets a new savepoint now,
m_n_savepoints is now redundant and is removed.

3 files changed, 48 insertions, 8 deletions

diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc
index bd2e0bfde4f..25bab1d97a9 100644
--- a/storage/rocksdb/ha_rocksdb.cc
+++ b/storage/rocksdb/ha_rocksdb.cc
@@ -1918,8 +1918,6 @@ protected:
   bool m_is_two_phase = false;
 
 private:
-  /* Number of RockDB savepoints taken */
-  int m_n_savepoints;
   /*
     Number of write operations this transaction had when we took the last
     savepoint (the idea is not to take another savepoint if we haven't made
@@ -2493,7 +2491,6 @@ public:
       entire transaction.
     */
     do_set_savepoint();
-    m_n_savepoints= 1;
     m_writes_at_last_savepoint= m_write_count;
   }
 
@@ -2510,7 +2507,6 @@ public:
     {
       do_set_savepoint();
       m_writes_at_last_savepoint= m_write_count;
-      m_n_savepoints++;
     }
   }
 
@@ -2521,10 +2517,14 @@ public:
   void rollback_to_stmt_savepoint() {
     if (m_writes_at_last_savepoint != m_write_count) {
       do_rollback_to_savepoint();
-      if (!--m_n_savepoints) {
-        do_set_savepoint();
-        m_n_savepoints= 1;
-      }
+      /*
+        RollbackToSavePoint "removes the most recent SetSavePoint()", so
+        we need to set it again so that next statement can roll back to this
+        stage.
+        It's ok to do it here at statement end (instead of doing it at next
+        statement start) because setting a savepoint is cheap.
+      */
+      do_set_savepoint();
       m_writes_at_last_savepoint= m_write_count;
     }
   }
diff --git a/storage/rocksdb/mysql-test/rocksdb/r/transaction.result b/storage/rocksdb/mysql-test/rocksdb/r/transaction.result
index 006baaf1339..7664a11e50b 100644
--- a/storage/rocksdb/mysql-test/rocksdb/r/transaction.result
+++ b/storage/rocksdb/mysql-test/rocksdb/r/transaction.result
@@ -958,3 +958,20 @@ a
 rollback;
 drop function func;
 drop table t1,t2,t3;
+#
+# MDEV-16710: Slave SQL: Could not execute Update_rows_v1 event with RocksDB and triggers
+# Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transaction
+#
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=RocksDB;
+INSERT INTO t1 VALUES (1);
+CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=RocksDB;
+CREATE TRIGGER tr AFTER INSERT ON t2 FOR EACH ROW INSERT INTO non_existing_table VALUES (NULL);
+BEGIN;
+DELETE FROM t1;
+INSERT INTO t2 VALUES (1);
+INSERT INTO t2 VALUES (2);
+# Must return empty result:
+SELECT * FROM t1;
+a
+COMMIT;
+drop table t1,t2;
diff --git a/storage/rocksdb/mysql-test/rocksdb/t/transaction.test b/storage/rocksdb/mysql-test/rocksdb/t/transaction.test
index 3350db99dab..2503cbc8215 100644
--- a/storage/rocksdb/mysql-test/rocksdb/t/transaction.test
+++ b/storage/rocksdb/mysql-test/rocksdb/t/transaction.test
@@ -133,3 +133,26 @@ rollback;
 drop function func;
 drop table t1,t2,t3;
 
+--echo #
+--echo # MDEV-16710: Slave SQL: Could not execute Update_rows_v1 event with RocksDB and triggers
+--echo # Issue#857: MyRocks: Incorrect behavior when miltiple statements fail inside a transaction
+--echo #
+CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=RocksDB;
+INSERT INTO t1 VALUES (1);
+ 
+CREATE TABLE t2 (b INT PRIMARY KEY) ENGINE=RocksDB;
+ 
+CREATE TRIGGER tr AFTER INSERT ON t2 FOR EACH ROW INSERT INTO non_existing_table VALUES (NULL);
+ 
+BEGIN;
+DELETE FROM t1;
+--error 0,ER_NO_SUCH_TABLE
+INSERT INTO t2 VALUES (1);
+--error 0,ER_NO_SUCH_TABLE
+INSERT INTO t2 VALUES (2);
+--echo # Must return empty result:
+SELECT * FROM t1;
+COMMIT;
+
+drop table t1,t2;
+