Hi, Monty! Just a couple of minor comments Note, the below is from the combined diff of this commit and your earlier MDEV-21953 commit, so you can see some lines here that are not part of the fc07306120f.
diff --git a/sql/handler.cc b/sql/handler.cc index cc7eedd18f2..402db310ed6 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -133,6 +133,23 @@ static plugin_ref ha_default_tmp_plugin(THD *thd) return ha_default_plugin(thd); }
+#if defined(WITH_ARIA_STORAGE_ENGINE) && MYSQL_VERSION_ID < 100500 +void ha_maria_implicit_commit(THD *thd, bool new_trn) +{ + if (ha_maria::has_active_transaction(thd)) + { + int error; + MDL_request mdl_request; + mdl_request.init(MDL_key::BACKUP, "", "", MDL_BACKUP_COMMIT, MDL_EXPLICIT); + error= thd->mdl_context.acquire_lock(&mdl_request, + thd->variables.lock_wait_timeout); + ha_maria::implicit_commit(thd, new_trn); + if (!error) + thd->mdl_context.release_lock(mdl_request.ticket);
This looks rather fishy. You commit in aria unconditionally? whether you got the lock or not?
+ } +} +#endif +
/** @brief Return the default storage engine handlerton for thread @@ -1753,13 +1777,14 @@ int ha_commit_one_phase(THD *thd, bool all) if ((res= thd->wait_for_prior_commit())) DBUG_RETURN(res); } - res= commit_one_phase_2(thd, all, trans, is_real_trans); + res= commit_one_phase_2(thd, all, trans, is_real_trans, rw_trans); DBUG_RETURN(res); }
static int -commit_one_phase_2(THD *thd, bool all, THD_TRANS *trans, bool is_real_trans) +commit_one_phase_2(THD *thd, bool all, THD_TRANS *trans, bool is_real_trans, + bool rw_trans)
rw_trans seems to be unused here (also in ha_commit_one_phase())
{ int error= 0; uint count= 0; diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 40e606425c5..71b447fb920 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1383,7 +1384,11 @@ void THD::update_all_stats() void THD::init_for_queries() { set_time(); - ha_enable_transaction(this,TRUE); + /* + We don't need to call ha_enable_transaction() as we can't have + any active transactions that has to be commited + */ + transaction.on= TRUE;
I just commented on the similar code in another commit of yours. don't just say "can't have any active transactions", add an assert instead, please.
reset_root_defaults(mem_root, variables.query_alloc_block_size, variables.query_prealloc_size);
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org