maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei!

On Tue, Apr 12, 2022 at 12:58 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey,
>
> Basically, it's fine, but I still didn't like that there's a new flag in
> THD to mark a statement that has to be logged. So I tried to change to
> log_current_statement. I know you tried that and you told me what tests
> fails, rpl.create_or_replace_*. I debugged that a bit, and I think the
> problem is that select_create inherits from select_insert, so
> CREATE ... SELECT runs your code that is only supposed to be run for
> INSERT ODKU or REPLACE. This change makes all tests pass (after removing
> vers_created_partitions):

I attached the patch and this still fails rpl.create_or_replace_row.
Why don't you like vers_created_partitions? It might be useful info
for debugging and for new features.

>
> @@ -4225,7 +4225,8 @@ bool select_insert::prepare_eof()
>        thd->clear_error();
>      else
>        errcode= query_error_code(thd, killed_status == NOT_KILLED);
> -    StatementBinlog stmt_binlog(thd, thd->binlog_need_stmt_format(trans_table));
> +    StatementBinlog stmt_binlog(thd, !can_rollback_data() &&
> +                                thd->binlog_need_stmt_format(trans_table));
>      res= thd->binlog_query(THD::ROW_QUERY_TYPE,
>                             thd->query(), thd->query_length(),
>                             trans_table, FALSE, FALSE, errcode);
>
> On Apr 10, Aleksey Midenkov wrote:
> > revision-id: 17548c8a8b6 (mariadb-10.6.1-250-g17548c8a8b6)
> > parent(s): 6282e025a0e
> > author: Aleksey Midenkov
> > committer: Aleksey Midenkov
> > timestamp: 2022-03-07 23:49:35 +0300
> > message:
> >
> > MDEV-25477 Auto-create breaks replication when triggering event was not replicated
> >
> > diff --git a/sql/sql_class.h b/sql/sql_class.h
> > index 686e6e70766..4fed1bf7355 100644
> > --- a/sql/sql_class.h
> > +++ b/sql/sql_class.h
> > @@ -2914,6 +2914,13 @@ class THD: public THD_count, /* this must be first */
> >    int binlog_flush_pending_rows_event(bool stmt_end, bool is_transactional);
> >    int binlog_remove_pending_rows_event(bool clear_maps, bool is_transactional);
> >
> > +  bool binlog_need_stmt_format(bool is_transactional) const
> > +  {
> > +    return vers_created_partitions && !binlog_get_pending_rows_event(is_transactional);
> > +  }
> > +
> > +  bool binlog_for_noop_dml(bool transactional_table);
> > +
> >    /**
> >      Determine the binlog format of the current statement.
> >
> > @@ -3557,6 +3564,7 @@ class THD: public THD_count, /* this must be first */
> >    bool       tablespace_op;  /* This is TRUE in DISCARD/IMPORT TABLESPACE */
> >    /* True if we have to log the current statement */
> >    bool            log_current_statement;
> > +  uint       vers_created_partitions;
> >    /**
> >      True if a slave error. Causes the slave to stop. Not the same
> >      as the statement execution error (is_error()), since
> > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
> > index 707b8a0d3bf..25551dfa197 100644
> > --- a/sql/sql_insert.cc
> > +++ b/sql/sql_insert.cc
> > @@ -4223,6 +4225,7 @@ bool select_insert::prepare_eof()
> >        thd->clear_error();
> >      else
> >        errcode= query_error_code(thd, killed_status == NOT_KILLED);
> > +    StatementBinlog stmt_binlog(thd, thd->binlog_need_stmt_format(trans_table));
> >      res= thd->binlog_query(THD::ROW_QUERY_TYPE,
> >                             thd->query(), thd->query_length(),
> >                             trans_table, FALSE, FALSE, errcode);
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



-- 
@midenok

diff --git a/sql/partition_info.cc b/sql/partition_info.cc
index 3a4d690bd46..f8e8369fedc 100644
--- a/sql/partition_info.cc
+++ b/sql/partition_info.cc
@@ -1028,7 +1028,6 @@ bool vers_create_partitions(THD *thd, TABLE_LIST* tl, uint num_parts)
   thd->get_stmt_da()->reset_diagnostics_area();
   thd->variables.option_bits|= OPTION_KEEP_LOG;
   thd->log_current_statement= true;
-  thd->vers_created_partitions= num_parts;
 
 exit:
   thd->work_part_info= save_part_info;
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 4fed1bf7355..29bcfe98b08 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -2916,7 +2916,7 @@ class THD: public THD_count, /* this must be first */
 
   bool binlog_need_stmt_format(bool is_transactional) const
   {
-    return vers_created_partitions && !binlog_get_pending_rows_event(is_transactional);
+    return !binlog_get_pending_rows_event(is_transactional);
   }
 
   bool binlog_for_noop_dml(bool transactional_table);
@@ -3564,7 +3564,6 @@ class THD: public THD_count, /* this must be first */
   bool       tablespace_op;	/* This is TRUE in DISCARD/IMPORT TABLESPACE */
   /* True if we have to log the current statement */
   bool	     log_current_statement;
-  uint       vers_created_partitions;
   /**
     True if a slave error. Causes the slave to stop. Not the same
     as the statement execution error (is_error()), since
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 25551dfa197..36a55c44539 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -4225,7 +4225,8 @@ bool select_insert::prepare_eof()
       thd->clear_error();
     else
       errcode= query_error_code(thd, killed_status == NOT_KILLED);
-    StatementBinlog stmt_binlog(thd, thd->binlog_need_stmt_format(trans_table));
+    StatementBinlog stmt_binlog(thd, !can_rollback_data() &&
+                                thd->binlog_need_stmt_format(trans_table));
     res= thd->binlog_query(THD::ROW_QUERY_TYPE,
                            thd->query(), thd->query_length(),
                            trans_table, FALSE, FALSE, errcode);
@@ -4346,7 +4347,8 @@ void select_insert::abort_result_set()
 
         if(WSREP_EMULATE_BINLOG(thd) || mysql_bin_log.is_open())
         {
-          StatementBinlog stmt_binlog(thd, thd->binlog_need_stmt_format(transactional_table));
+          StatementBinlog stmt_binlog(thd, !can_rollback_data() &&
+                                      thd->binlog_need_stmt_format(transactional_table));
           int errcode= query_error_code(thd, thd->killed == NOT_KILLED);
           int res;
           /* error of writing binary log is ignored */
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 6935a8d5fe7..078ea0dae9b 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -7570,7 +7570,6 @@ void THD::reset_for_next_command(bool do_clear_error)
   query_start_sec_part_used= 0;
   is_fatal_error= time_zone_used= 0;
   log_current_statement= 0;
-  vers_created_partitions= 0;
 
   /*
     Clear the status flag that are expected to be cleared at the