Hi Sergei,

On Fri, Oct 25, 2019 at 9:44 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Aleksey!

On Oct 25, Aleksey Midenkov wrote:
> revision-id: d5352b8154d (mariadb-10.2.25-54-gd5352b8154d)
> parent(s): 1153950ad0a
> author: Aleksey Midenkov <midenok@gmail.com>
> committer: Aleksey Midenkov <midenok@gmail.com>
> timestamp: 2019-07-22 15:40:06 +0300
> message:
>
> MDEV-20015 Assertion `!in_use->is_error()' failed in TABLE::update_virtual_field
>
> Preserve and restore statement DA.

This is strange. Diagnostics areas aren't supposed to be temporarily
created on the frame, they aren't arenas.

There are already several samples of this pattern in the code. This is the main (if not the only) usage of set_stmt_da().
 

Why TABLE::update_virtual_field() is called at all if there's already
an error?

update_virtual_field() is called as part of  REPAIR (MDEV-5800) which is done on bulk insert finish. I doubt it should consider error status in this case as no matter how SQL command is finished it must update the index.

#0  TABLE::update_virtual_field (this=0x7f85b4068388, vf=0x7f85b4066948) at /home/midenok/src/mariadb/10.2/src/sql/table.cc:7685
#1  0x0000000001105d2b in compute_vcols (info=0x7f85b406a5d8, record=0x7f85b4006948 "\377\001", keynum=1) at /home/midenok/src/mariadb/10.2/src/storage/myisam/ha_myisam.cc:683
#2  0x00000000011142bd in sort_get_next_record (sort_param=0x7f860c05edd8) at /home/midenok/src/mariadb/10.2/src/storage/myisam/mi_check.c:3657
#3  0x0000000001118a33 in sort_key_read (sort_param=0x7f860c05edd8, key=0x7f85b4039808) at /home/midenok/src/mariadb/10.2/src/storage/myisam/mi_check.c:3121
#4  0x00000000011650a9 in find_all_keys (info=0x7f860c05edd8, keys=2, sort_keys=0x7f85b40397f8, buffpek=0x7f860c05ea20, maxbuffer=0x7f860c05ea54, tempfile=0x7f860c05e898, tempfile_for_exceptions=0x7f860c05e728) at /home/midenok/src/mariadb/10.2/src/storage/myisam/sort.c:312
#5  0x0000000001164b8d in _create_index_by_sort (info=0x7f860c05edd8, no_messages=1 '\001', sortbuff_size=134216704) at /home/midenok/src/mariadb/10.2/src/storage/myisam/sort.c:228
#6  0x000000000111763b in mi_repair_by_sort (param=0x7f85b406e300, info=0x7f85b406a5d8, name=0x7f860c05f7c0 "./test/t2", rep_quick=1) at /home/midenok/src/mariadb/10.2/src/storage/myisam/mi_check.c:2401
#7  0x0000000001107097 in ha_myisam::repair (this=0x7f85b4068f20, thd=0x7f85b4000cf8, param=..., do_optimize=false) at /home/midenok/src/mariadb/10.2/src/storage/myisam/ha_myisam.cc:1271
#8  0x00000000011085d3 in ha_myisam::enable_indexes (this=0x7f85b4068f20, mode=2) at /home/midenok/src/mariadb/10.2/src/storage/myisam/ha_myisam.cc:1610
#9  0x0000000001108db2 in ha_myisam::end_bulk_insert (this=0x7f85b4068f20) at /home/midenok/src/mariadb/10.2/src/storage/myisam/ha_myisam.cc:1760
#10 0x00000000006d0d23 in handler::ha_end_bulk_insert (this=0x7f85b4068f20) at /home/midenok/src/mariadb/10.2/src/sql/handler.h:2918
#11 0x00000000006cd120 in select_insert::abort_result_set (this=0x7f85b4012818) at /home/midenok/src/mariadb/10.2/src/sql/sql_insert.cc:3959
#12 0x00000000007335a7 in handle_select (thd=0x7f85b4000cf8, lex=0x7f85b4004820, result=0x7f85b4012818, setup_tables_done_option=1073741824) at /home/midenok/src/mariadb/10.2/src/sql/sql_select.cc:383
#13 0x00000000006eedad in mysql_execute_command (thd=0x7f85b4000cf8) at /home/midenok/src/mariadb/10.2/src/sql/sql_parse.cc:4284
#14 0x00000000006e8880 in mysql_parse (thd=0x7f85b4000cf8, rawbuf=0x7f85b40117f0 "insert into t2 (pk) select a from t1", length=36, parser_state=0x7f860c0625f0, is_com_multi=false, is_next_command=false) at /home/midenok/src/mariadb/10.2/src/sql/sql_parse.cc:7760
 

> diff --git a/sql/table.cc b/sql/table.cc
> index f5b5bad99cc..65611d78bde 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -7682,15 +7682,25 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode)
> 
>  int TABLE::update_virtual_field(Field *vf)
>  {
> -  DBUG_ASSERT(!in_use->is_error());
> +  Diagnostics_area *stmt_da= NULL;
> +  Diagnostics_area tmp_stmt_da(in_use->query_id, false, true);
> +  bool error;
>    Query_arena backup_arena;
>    DBUG_ENTER("TABLE::update_virtual_field");
> +  if (unlikely(in_use->is_error()))
> +  {
> +    stmt_da= in_use->get_stmt_da();
> +    in_use->set_stmt_da(&tmp_stmt_da);
> +  }
>    in_use->set_n_backup_active_arena(expr_arena, &backup_arena);
>    bitmap_clear_all(&tmp_set);
>    vf->vcol_info->expr->walk(&Item::update_vcol_processor, 0, &tmp_set);
>    vf->vcol_info->expr->save_in_field(vf, 0);
>    in_use->restore_active_arena(expr_arena, &backup_arena);
> -  DBUG_RETURN(in_use->is_error());
> +  error= in_use->is_error();
> +  if (stmt_da)
> +    in_use->set_stmt_da(stmt_da);
> +  DBUG_RETURN(error);
>  }

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


--
All the best,

Aleksey Midenkov
@midenok