Re: [Maria-developers] 903ae1c03ae: Fix running without binlog
Hi, Nikita! On Nov 15, Nikita Malyavin wrote:
revision-id: 903ae1c03ae (mariadb-10.5.2-479-g903ae1c03ae) parent(s): a5fca9a6e30 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2021-01-27 17:28:05 +1000 message:
Fix running without binlog
Strange. With such a comment I'd expect you add a test for ALTER TABLE with disabled binlog. Instead you change an error to a warning in one of the existing tests. This test change doesn't match the commit comment.
diff --git a/mysql-test/suite/binlog/r/online_alter.result b/mysql-test/suite/binlog/r/online_alter.result index 9bca94b8a87..bb3fa85e76c 100644 --- a/mysql-test/suite/binlog/r/online_alter.result +++ b/mysql-test/suite/binlog/r/online_alter.result @@ -34,9 +34,9 @@ insert into t1 values (123), (456), (789); set debug_sync= 'now SIGNAL end'; connection default; Warnings: -Error 1364 Field 'b' doesn't have a default value -Error 1364 Field 'b' doesn't have a default value -Error 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value select * from t1; a b 5 0 diff --git a/sql/sql_table.cc b/sql/sql_table.cc index f0d1167ad61..e348056646d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11308,9 +11308,12 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, break;
ev->thd= thd; + bool abort_on_warning= thd->abort_on_warning; + thd->abort_on_warning= false; thd->set_n_backup_active_arena(&event_arena, &backup_arena); error= ev->apply_event(rgi); thd->restore_active_arena(&event_arena, &backup_arena); + thd->abort_on_warning= abort_on_warning;
event_arena.free_items(); free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC));
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
diff --git a/sql/handler.cc b/sql/handler.cc index 8b95653e69f..8b22167e729 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -6660,7 +6660,16 @@ static int binlog_log_row_online_alter(TABLE*
THD *thd= table->in_use;
if (!table->online_alter_cache) - table->online_alter_cache= thd->binlog_setup_cache_data(); + { + auto *cache_mngr= online_alter_binlog_get_cache_mngr(thd, table); + // Use transaction cache directly, if it is not multi-transaction mode + table->online_alter_cache= binlog_get_cache_data(cache_mngr, + !thd->in_multi_stmt_
indeed, maybe something's mixed up I really think this should be a part of this fix (from "[6/8] store cache managers in a list") : table, transaction_mode());
+ + trans_register_ha(thd, false, binlog_hton, 0); + if (thd->in_multi_stmt_transaction_mode()) + trans_register_ha(thd, true, binlog_hton, 0);
But anyway it is going to be a fixup, so whatever. I am only sorry for confusion. The way to test with and without binlog is to rename .test in .inc and make a new test: one that sources have_binlog.inc, and one that doesn't. I don't like putting tests in .inc, so decided to leave only one version of it. Maybe that's wrong But what about sourcing a .test file? On Mon, 15 Nov 2021 at 13:56, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nikita!
On Nov 15, Nikita Malyavin wrote:
revision-id: 903ae1c03ae (mariadb-10.5.2-479-g903ae1c03ae) parent(s): a5fca9a6e30 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2021-01-27 17:28:05 +1000 message:
Fix running without binlog
Strange. With such a comment I'd expect you add a test for ALTER TABLE with disabled binlog. Instead you change an error to a warning in one of the existing tests. This test change doesn't match the commit comment.
diff --git a/mysql-test/suite/binlog/r/online_alter.result b/mysql-test/suite/binlog/r/online_alter.result index 9bca94b8a87..bb3fa85e76c 100644 --- a/mysql-test/suite/binlog/r/online_alter.result +++ b/mysql-test/suite/binlog/r/online_alter.result @@ -34,9 +34,9 @@ insert into t1 values (123), (456), (789); set debug_sync= 'now SIGNAL end'; connection default; Warnings: -Error 1364 Field 'b' doesn't have a default value -Error 1364 Field 'b' doesn't have a default value -Error 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value +Warning 1364 Field 'b' doesn't have a default value select * from t1; a b 5 0 diff --git a/sql/sql_table.cc b/sql/sql_table.cc index f0d1167ad61..e348056646d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11308,9 +11308,12 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, break;
ev->thd= thd; + bool abort_on_warning= thd->abort_on_warning; + thd->abort_on_warning= false; thd->set_n_backup_active_arena(&event_arena, &backup_arena); error= ev->apply_event(rgi); thd->restore_active_arena(&event_arena, &backup_arena); + thd->abort_on_warning= abort_on_warning;
event_arena.free_items(); free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC));
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
-- Yours truly, Nikita Malyavin
Hi, Nikita! On Nov 24, Nikita Malyavin wrote:
The way to test with and without binlog is to rename .test in .inc and make a new test: one that sources have_binlog.inc, and one that doesn't.
I don't like putting tests in .inc, so decided to leave only one version of it. Maybe that's wrong
But what about sourcing a .test file?
That's okay. There're tests in mysql-test that use inc, there're tests that include other tests. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Nikita Malyavin
-
Sergei Golubchik