maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On Apr 13, Michael Widenius wrote:
> revision-id: 3cbe15bd78c (mariadb-10.5.2-120-g3cbe15bd78c)
> parent(s): d4d332d196d
> author: Michael Widenius <monty@xxxxxxxxxxx>
> committer: Michael Widenius <monty@xxxxxxxxxxx>
> timestamp: 2020-04-09 01:37:01 +0300
> message:
> 
> Fixed core dump in alter table if ADD PARTITION fails
> 
> I didn't add a test case as to reproduce this we need to
> have a failed write in on of the engines. Bug found and bug fix
> verified while debugging S3 and partitions.
> 
> ---
>  sql/sql_partition.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
> index ef8ef5114a8..4e984fa775d 100644
> --- a/sql/sql_partition.cc
> +++ b/sql/sql_partition.cc
> @@ -6817,7 +6817,8 @@ static void handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt,
>    DBUG_ENTER("handle_alter_part_error");
>    DBUG_ASSERT(table->m_needs_reopen);
>  
> -  if (close_table)
> +  /* The table may not be open if ha_partition::change_partitions() failed */
> +  if (close_table && !table->file->is_open())

This looks *very* confusing, like you close the table only if it's *not*
open. But then the whole if() is very confusing, as it closes in both
branches, so the meaning of "close_table" is totally not clear.

May be instead of this your fix you'd better cherry-pick
9c02b7d6670b069866 from MySQL? It removes close_table completely,
so I expect it to work for S3 just as you wanted.

>    {
>      /*
>        All instances of this table needs to be closed.
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx