Hi Sergei, On Sun, Apr 17, 2022 at 8:48 PM Sergei Golubchik wrote:

Hi, Aleksey,

...

I attached the patch and this still fails rpl.create_or_replace_row.

small mistake in the patch:

...

--- 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);

if you put here

log_current_statement && !binlog_get_pending_rows_event(is_transactional)

then all tests pass.

...

}

bool binlog_for_noop_dml(bool transactional_table);

...

Why don't you like vers_created_partitions? It might be useful info for debugging and for new features.

I have a second patch that renames flags to have a very well-defined meaning and name.

OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX log_current_statement -> OPTION_BINLOG_THIS_STMT

Where is your patch?

the first flag means that when a transaction is rolled back, it's written to binlog with ROLLBACK at the end, not discarded.

The second means that the statement is written to binlog (or binlog trx cache) even if it otherwise wouldn't be.

So, two flags. Clear names and semantics. "vers_created_partitions" fits into the second use case, so one should use OPTION_BINLOG_THIS_STMT for it.

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

-- @midenok

Hi Sergei! On Tue, Apr 19, 2022 at 11:13 AM Sergei Golubchik wrote:

Hi, Aleksey,

On Apr 18, Aleksey Midenkov wrote:

...

...

I have a second patch that renames flags to have a very well-defined meaning and name.

OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX log_current_statement -> OPTION_BINLOG_THIS_STMT

Where is your patch?

It's really just renaming, almost nothing else. But here it is, attached.

I'd keep log_current_statement() like this: --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3562,4 +3562,8 @@ class THD: public THD_count, /* this must be first */ /* set during loop of derived table processing */ bool derived_tables_processing; bool tablespace_op; /* This is TRUE in DISCARD/IMPORT TABLESPACE */ + bool log_current_statement() const + { + return variables.option_bits & OPTION_BINLOG_THIS_STMT; + } /**

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

-- @midenok

Sergei, I updated the branch accordingly. There is your patch fa444975d0f where I added log_current_statement(). On Wed, Apr 20, 2022 at 1:12 PM Sergei Golubchik wrote:

Hi, Aleksey,

Okay, sure. I was going to push it after the release. And in 10.3 to simplify future merges.

On Apr 20, Aleksey Midenkov wrote:

...

Hi Sergei!

On Tue, Apr 19, 2022 at 11:13 AM Sergei Golubchik wrote:

...

Hi, Aleksey,

On Apr 18, Aleksey Midenkov wrote:

...

...

I have a second patch that renames flags to have a very well-defined meaning and name.

OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX log_current_statement -> OPTION_BINLOG_THIS_STMT

Where is your patch?

It's really just renaming, almost nothing else. But here it is, attached.

I'd keep log_current_statement() like this:

--- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3562,4 +3562,8 @@ class THD: public THD_count, /* this must be first */ /* set during loop of derived table processing */ bool derived_tables_processing; bool tablespace_op; /* This is TRUE in DISCARD/IMPORT TABLESPACE */ + bool log_current_statement() const + { + return variables.option_bits & OPTION_BINLOG_THIS_STMT; + } /**

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

-- @midenok