Hi, Aleksey, On Feb 15, Aleksey Midenkov wrote: > > On Feb 12, Aleksey Midenkov wrote: > > > commit 0402f6dcb67 > > > Author: Aleksey Midenkov <[email protected]> > > > Date: Tue Feb 8 22:54:03 2022 +0300 > > > > > > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > > > index 0403d6e73c3..06068290247 100644 > > > --- a/sql/sql_delete.cc > > > +++ b/sql/sql_delete.cc > > > @@ -507,6 +508,18 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, > > > COND *conds, > > > if (thd->lex->describe || thd->lex->analyze_stmt) > > > goto produce_explain_and_leave; > > > > > > + if (thd->log_current_statement) > > > + { > > > + thd->reset_unsafe_warnings(); > > > + if (thd->binlog_query(THD::STMT_QUERY_TYPE, > > > + thd->query(), thd->query_length(), > > > + transactional_table, FALSE, FALSE, 0) > 0) > > > + { > > > + my_error(ER_ERROR_ON_WRITE, MYF(0), "binary log", -1); > > > + DBUG_RETURN(1); > > > + } > > > + } > > > > perhaps, move this block into a function? It's repeated four times here. > > > > Moved.
Thanks! The new method, binlog_for_noop_dml() looks very puzzling. I understand what it does, for now, but somebody who wasn't involved in this bugfixing, likely won't have any idea. The name is correct, I agree, it says what, but it doesn't explain why. Could you add a comment there? To say that "noop dml" is "an insert(odku)/update/delete that doesn't change the table" and that it is normally not logged, but needs to be logged if it auto-created a partition as a side effect. > > by the way, you don't seem to have a test for a zero-rows insert. > > That is INSERT ... SELECT with an impossible where or limit 0. > > > > In fact, you can have a zero-row insert with a normal insert > > INSERT t1 VALUES (1); if this (1) is a unique key violation. > > I believe your code handles this case, thought, just a test case is > > missing. > > INSERT does not trigger history. But INSERT .. ODKU does. Added test > cases for INSERT .. ODKU and INSERT .. SELECT .. ODKU. That required > to add THD::vers_created_partitions as log_current_statement is also > used in CREATE TABLE (and select_insert::prepare_eof() is used for > CREATE TABLE .. SELECT) and that broke some replication tests: > > rpl.create_or_replace_mix > rpl.create_or_replace_row > rpl.create_or_replace_statement Indeed, I see. Can you use OPTION_KEEP_LOG instead? Or transaction->stmt.modified_non_trans_table ? I hate it that there're so many ways to force binlogging a statement, and they're all, of course, are subtly different. > > > > + > > > my_ok(thd, 0); > > > DBUG_RETURN(0); > > > } Regards, Sergei VP of MariaDB Server Engineering and [email protected] _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
