Hi!

On Fri, 29 Sept 2023 at 21:41, Sergei Golubchik <serg@mariadb.org> wrote:
> +using Online_alter_cache_list= ilist<online_alter_cache_data>;

please, use typedef for that

Fine, but can you explain what's your reasoning behind this? 

>  class online_alter_cache_data: public Sql_alloc, public ilist_node<>,
>                                 public binlog_cache_data
> @@ -246,7 +268,11 @@ int online_alter_savepoint_rollback(THD *thd, LEX_CSTRING name)

>  static int online_alter_close_connection(handlerton *hton, THD *thd)
>  {
> -  DBUG_ASSERT(thd->online_alter_cache_list.empty());
> +  Ha_data &ha= thd->ha_data[online_alter_hton->slot];
> +  auto *cache_list= (Online_alter_cache_list*)ha.ha_ptr;

why not thd_get_ha_data()?

I suppose that this simple wrapper is used for dependency inversion, i.e. to access thd data outside `sql` scope, like innodb does it.

--
Yours truly,
Nikita Malyavin