finally, yeah

4 files changed, 116 insertions, 20 deletions

diff --git a/mysql-test/suite/binlog/t/online_alter.test b/mysql-test/suite/binlog/t/online_alter.test
index f7ad2ea9c79..c0b4300fc3d 100644
--- a/mysql-test/suite/binlog/t/online_alter.test
+++ b/mysql-test/suite/binlog/t/online_alter.test
@@ -141,4 +141,86 @@ set debug_sync= 'now SIGNAL end';
 --connection default
 --reap
 select * from t1;
+
+--echo # test primary key change
+create or replace table t1 (a int primary key, b int) engine=innodb;
+insert t1 values (1, 11);
+insert t1 values (2, 22);
+
+--connection con2
+--send
+set debug_sync= 'now WAIT_FOR ended';
+
+--connection default
+set debug_sync= 'alter_table_copy_end SIGNAL ended WAIT_FOR end';
+
+--send
+alter table t1 drop primary key, add primary key(b),
+            algorithm= copy, lock= none;
+
+--connection con2
+--reap
+update t1 set b= 33 where a = 1;
+update t1 set b= 44 where a = 2;
+
+set debug_sync= 'now SIGNAL end';
+
+--connection default
+--reap
+select * from t1;
+
+--echo #
+--echo # Various tests, see below
+--echo #
+create or replace table t1 (a int primary key, b int) engine=innodb;
+insert t1 values (1, 11);
+insert t1 values (2, 22);
+insert t1 values (3, 33);
+insert t1 values (4, 44);
+insert t1 values (5, 55);
+insert t1 values (6, 66);
+
+--connection con2
+--send
+set debug_sync= 'now WAIT_FOR ended';
+
+--connection default
+set debug_sync= 'alter_table_copy_end SIGNAL ended WAIT_FOR end';
+
+--send
+alter table t1 drop primary key, add primary key(b),
+            algorithm= copy, lock= none;
+
+--connection con2
+--reap
+
+--echo # Double execution
+update t1 set b= 1001 where a = 1;
+update t1 set b= 2002 where a = 2;
+
+--echo # Double execution in transaction
+set autocommit = 0;
+start transaction;
+update t1 set b= 3003 where a = 3;
+update t1 set b= 4004 where a = 4;
+commit;
+set autocommit = 1;
+
+--echo # Second execution is rolled back
+update t1 set b= 5005 where a = 5;
+
+set autocommit = 0;
+start transaction;
+update t1 set b= 6006 where a = 6;  ## TODO bug
+rollback;
+set autocommit = 1;
+
+set debug_sync= 'now SIGNAL end';
+
+--connection default
+--reap
+select * from t1;
+
+
+--echo # Cleanup
 drop table t1, t2;
diff --git a/sql/handler.cc b/sql/handler.cc
index e7887d9e192..4072a82641f 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -6665,8 +6665,9 @@ static int binlog_log_row_online_alter(TABLE* table,
   if (!table->online_alter_cache)
     table->online_alter_cache= thd->binlog_setup_cache_data(&table->mem_root);
 
-  // We need to log all columns for the caase if alter table chanes primary key
+  // We need to log all columns for the case if alter table changes primary key.
   table->use_all_columns();
+  bitmap_set_all(table->rpl_write_set);
 
   int error= (*log_func)(thd, table, table->s->online_ater_binlog,
                          table->online_alter_cache, has_trans,
diff --git a/sql/log.cc b/sql/log.cc
index 8fffa3f003e..9dd3a4c6cdc 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -2147,12 +2147,8 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all)
   PSI_stage_info org_stage;
   DBUG_ENTER("binlog_commit");
 
-  /* 
-    cache_mngr can be NULL in case if logging is disabled.
-    Still we can have online alter in process
-  */
-
-  for (TABLE *table= thd->open_tables; table;
+  bool is_ending_transaction= ending_trans(thd, all);
+  for (TABLE *table= thd->open_tables; is_ending_transaction && table;
        table= table->next)
   {
     if (!table->online_alter_cache)
@@ -2178,6 +2174,10 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all)
       error= binlog->flush_and_sync(NULL);
     mysql_mutex_unlock(binlog->get_log_lock());
 
+    table->online_alter_cache->reset();
+    delete table->online_alter_cache;
+    table->online_alter_cache= NULL;
+
     if (error)
     {
       my_error(ER_ERROR_ON_WRITE, MYF(ME_ERROR_LOG), binlog->get_name(), errno);
@@ -2186,7 +2186,9 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all)
   }
 
   binlog_cache_mngr *const cache_mngr= thd->binlog_get_cache_mngr();
-
+  /*
+    cache_mngr can be NULL in case if binlog logging is disabled.
+  */
   if (!cache_mngr)
   {
     DBUG_ASSERT(WSREP(thd) ||
@@ -2229,7 +2231,7 @@ static int binlog_commit(handlerton *hton, THD *thd, bool all)
      - We are in a transaction and a full transaction is committed.
     Otherwise, we accumulate the changes.
   */
-  if (likely(!error) && ending_trans(thd, all))
+  if (likely(!error) && is_ending_transaction)
   {
     error= is_preparing_xa(thd) ?
       binlog_commit_flush_xa_prepare(thd, all, cache_mngr) :
@@ -2259,6 +2261,13 @@ static int binlog_rollback(handlerton *hton, THD *thd, bool all)
 {
   DBUG_ENTER("binlog_rollback");
 
+  for (TABLE *table= thd->open_tables; table; table= table->next)
+  {
+    table->online_alter_cache->reset();
+    delete table->online_alter_cache;
+    table->online_alter_cache= NULL;
+  }
+
   int error= 0;
   binlog_cache_mngr *const cache_mngr= thd->binlog_get_cache_mngr();
 
@@ -5770,8 +5779,10 @@ bool stmt_has_updated_non_trans_table(const THD* thd)
 
 binlog_cache_data *THD::binlog_setup_cache_data(MEM_ROOT *mem_root) 
 {
-  void *cache_buf= alloc_root(mem_root, sizeof(binlog_cache_data));
-  auto *cache= new (cache_buf)binlog_cache_data();
+  auto *cache= new binlog_cache_data();
+  cache->set_binlog_cache_info(max_binlog_stmt_cache_size,
+                               &binlog_stmt_cache_use,
+                               &binlog_stmt_cache_disk_use);
   if (!cache ||
       open_cached_file(&cache->cache_log, mysql_tmpdir,
                        LOG_PREFIX, (size_t)binlog_cache_size, MYF(MY_WME)))
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index b55ad031fb7..baa255f7d39 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -11705,6 +11705,15 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     error= 1;
   }
 
+  THD_STAGE_INFO(thd, stage_enabling_keys);
+  thd_progress_next_stage(thd);
+
+  if (!ignore)
+    to->file->extra(HA_EXTRA_END_ALTER_COPY);
+
+  cleanup_done= 1;
+  to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
+
   if (online && error < 0)
   {
     Table_map_log_event table_event(thd, from, from->s->table_map_id,
@@ -11732,7 +11741,8 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     rli.relay_log.description_event_for_exec=
                                             new Format_description_log_event(4);
 
-
+    // We restore bitmaps, because update event is going to mess up with them.
+    to->default_column_bitmaps();
     online_alter_read_from_binlog(thd, &rgi, from->s->online_ater_binlog, &log);
 
 
@@ -11744,16 +11754,8 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     from->s->online_ater_binlog->~MYSQL_BIN_LOG();
     from->s->online_ater_binlog= NULL;
   }
-
   // TODO handle m_vers_from_plain
-  THD_STAGE_INFO(thd, stage_enabling_keys);
-  thd_progress_next_stage(thd);
 
-  if (!ignore)
-    to->file->extra(HA_EXTRA_END_ALTER_COPY);
-
-  cleanup_done= 1;
-  to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
   DEBUG_SYNC(thd, "copy_data_between_tables_before_reset_backup_lock");
   if (backup_reset_alter_copy_lock(thd))