Hi, Nikita,

On Aug 08, Nikita Malyavin wrote:
> yes, extra safety can be worth it. I'd also add an assertion on top of
> that:
>
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -12181,6 +12181,7 @@ copy_data_between_tables(THD *thd, TABLE *from,
> TABLE *>
>      if (error)
>        from->s->tdc->flush_unused(1); // to free the binlog
>      to->pos_in_table_list= NULL; // Safety
> +    DBUG_ASSERT(thd->lex->sql_command == SQLCOM_ALTER_TABLE);
> +    thd->lex->sql_command= SQLCOM_ALTER_TABLE;
>    }
>    else if (online) // error was on copy stage
>    {
>
> That is, if we'll catch it while testing, we'll fix it. And users will
> be protected form the possible corner cases we didn't think of

Well, that's not exactly what I meant. I meant that row event
changing thd->lex->sql_command might be intentional and not something
that needs to be fixed.

So just reset it back to SQLCOM_ALTER_TABLE in
copy_data_between_tables() and that's it. Without assert and without
avoiding SQLCOM_REPLACE.