fix savepoints in myisambb-10.8-online-alter

4 files changed, 42 insertions, 16 deletions

diff --git a/mysql-test/main/alter_table_online.result b/mysql-test/main/alter_table_online.result
index a1bbc4bb561..a9ca16b1442 100644
--- a/mysql-test/main/alter_table_online.result
+++ b/mysql-test/main/alter_table_online.result
@@ -456,6 +456,8 @@ create or replace table t1 (a int) engine=innodb;
 insert t1 values (1), (2);
 create or replace table t2 (a int) engine=innodb;
 insert t2 values (1), (2);
+create or replace table t3 (a int) engine=myisam;
+insert t3 values (1);
 connection con2;
 begin;
 update t2 set a= 222 where a = 2;
@@ -468,15 +470,22 @@ alter table t1 add b int NULL, algorithm= copy, lock= none;
 connection con2;
 savepoint whoopsie;
 update t1 set a= 123 where a = 1;
+insert t3 values (2);
 select * from t1;
 a
 123
 2
 rollback to savepoint whoopsie;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
 select * from t1;
 a
 1
 2
+select * from t3;
+a
+1
+2
 commit;
 set debug_sync= 'now SIGNAL end';
 connection default;
@@ -488,7 +497,12 @@ select * from t2;
 a
 111
 222
+select * from t3;
+a
+1
+2
 # Cleanup
 set debug_sync= 'reset';
 drop table t1;
 drop table t2;
+drop table t3;
diff --git a/mysql-test/main/alter_table_online.test b/mysql-test/main/alter_table_online.test
index 15440d130cc..815483d252d 100644
--- a/mysql-test/main/alter_table_online.test
+++ b/mysql-test/main/alter_table_online.test
@@ -550,6 +550,9 @@ insert t1 values (1), (2);
 create or replace table t2 (a int) engine=innodb;
 insert t2 values (1), (2);
 
+create or replace table t3 (a int) engine=myisam;
+insert t3 values (1);
+
 --connection con2
 begin;
 update t2 set a= 222 where a = 2;
@@ -569,10 +572,12 @@ alter table t1 add b int NULL, algorithm= copy, lock= none;
 --reap
 savepoint whoopsie;
 update t1 set a= 123 where a = 1;
+insert t3 values (2);
 
 select * from t1;
 rollback to savepoint whoopsie;
 select * from t1;
+select * from t3;
 commit;
 
 set debug_sync= 'now SIGNAL end';
@@ -582,8 +587,10 @@ set debug_sync= 'now SIGNAL end';
 
 select * from t1;
 select * from t2;
+select * from t3;
 
 --echo # Cleanup
 set debug_sync= 'reset';
 drop table t1;
 drop table t2;
+drop table t3;
diff --git a/sql/log.cc b/sql/log.cc
index 7e778c32856..9beab80773c 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -7595,6 +7595,9 @@ int online_alter_savepoint_set(THD *thd, LEX_CSTRING name)
 
   for (auto &cache: thd->online_alter_cache_list)
   {
+    if (cache.share->db_type()->savepoint_set == NULL)
+      continue;
+
     SAVEPOINT *sv= savepoint_add(thd, name, &cache.sv_list, NULL);
     if(unlikely(sv == NULL))
       DBUG_RETURN(1);
@@ -7614,6 +7617,9 @@ int online_alter_savepoint_rollback(THD *thd, LEX_CSTRING name)
 #ifdef HAVE_REPLICATION
   for (auto &cache: thd->online_alter_cache_list)
   {
+    if (cache.share->db_type()->savepoint_set == NULL)
+      continue;
+
     SAVEPOINT **sv= find_savepoint_in_list(thd, name, &cache.sv_list);
     // sv is null if savepoint was set up before online table was modified
     my_off_t pos= *sv ? *(my_off_t*)(*sv+1) : 0;
diff --git a/sql/transaction.cc b/sql/transaction.cc
index bc93b19fb74..1dc89259bdf 100644
--- a/sql/transaction.cc
+++ b/sql/transaction.cc
@@ -633,10 +633,6 @@ bool trans_savepoint(THD *thd, LEX_CSTRING name)
   if (newsv == NULL)
     DBUG_RETURN(TRUE);
 
-  int error= online_alter_savepoint_set(thd, name);
-  if (unlikely(error))
-    DBUG_RETURN(error);
-
   /*
     if we'll get an error here, don't add new savepoint to the list.
     we'll lose a little bit of memory in transaction mem_root, but it'll
@@ -645,6 +641,11 @@ bool trans_savepoint(THD *thd, LEX_CSTRING name)
   if (unlikely(ha_savepoint(thd, newsv)))
     DBUG_RETURN(TRUE);
 
+  int error= online_alter_savepoint_set(thd, name);
+  if (unlikely(error))
+    DBUG_RETURN(error);
+
+
   newsv->prev= thd->transaction->savepoints;
   thd->transaction->savepoints= newsv;
 
@@ -695,19 +696,17 @@ bool trans_rollback_to_savepoint(THD *thd, LEX_CSTRING name)
   if (thd->transaction->xid_state.check_has_uncommitted_xa())
     DBUG_RETURN(TRUE);
 
-  res= online_alter_savepoint_rollback(thd, name);
+  if (ha_rollback_to_savepoint(thd, sv))
+    res= TRUE;
+  else if (((thd->variables.option_bits & OPTION_KEEP_LOG) ||
+            thd->transaction->all.modified_non_trans_table) &&
+           !thd->slave_thread)
+    push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
+                 ER_WARNING_NOT_COMPLETE_ROLLBACK,
+                 ER_THD(thd, ER_WARNING_NOT_COMPLETE_ROLLBACK));
+
+  res= res || online_alter_savepoint_rollback(thd, name);
 
-  if (res == 0)
-  {
-    if (ha_rollback_to_savepoint(thd, sv))
-      res= TRUE;
-    else if (((thd->variables.option_bits & OPTION_KEEP_LOG) ||
-              thd->transaction->all.modified_non_trans_table) &&
-             !thd->slave_thread)
-      push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
-                   ER_WARNING_NOT_COMPLETE_ROLLBACK,
-                   ER_THD(thd, ER_WARNING_NOT_COMPLETE_ROLLBACK));
-  }
 
   thd->transaction->savepoints= sv;