maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey,

This is a review of `git diff 5fbbdbc85ba3 62d3496969db`

Basically, no new comments, see below:

On Apr 07, Aleksey Midenkov wrote:

> diff --git a/libmariadb b/libmariadb
> index f6c3d9fd2af..735a7299dba 160000
> --- a/libmariadb
> +++ b/libmariadb
> @@ -1 +1 @@
> -Subproject commit f6c3d9fd2af5d17db64cc996574aa312efd70fcf
> +Subproject commit 735a7299dbae19cc2b82b9697becaf90e9b43047

you've mistakenly checked in a submodule

> diff --git a/sql/field.cc b/sql/field.cc
> index f18fb25ebe3..04c492e6c69 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -2416,6 +2416,13 @@ int Field::set_default()
>    if (default_value)
>    {
>      Query_arena backup_arena;
> +    /*
> +      TODO: this imposes memory leak until table flush when save_in_field()
> +            does expr_arena allocation. F.ex. case from main.default:
> +
> +            CREATE TABLE t1 (a INT DEFAULT CONCAT('1 '));
> +            INSERT INTO t1 VALUES (DEFAULT);

I wasn't able to repeat that. In your branch (didn't try in vanilla 10.3) and
for the above test case I see no additional allocations in table->mem_root
after the table is opened.

> +    */
>      table->in_use->set_n_backup_active_arena(table->expr_arena, &backup_arena);
>      int rc= default_value->expr->save_in_field(this, 0);
>      table->in_use->restore_active_arena(table->expr_arena, &backup_arena);
> diff --git a/sql/table.cc b/sql/table.cc
> index 08d91678b25..d22da17c31a 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -2970,36 +2988,133 @@ static bool fix_vcol_expr(THD *thd, Virtual_column_info *vcol)
...
> +bool Vcol_expr_context::init()
> +{
> +  /*
> +      As this is vcol expression we must narrow down name resolution to
> +      single table.
> +  */
> +  if (init_lex_with_single_table(thd, table, &lex))

I've already asked about it in a separate email,
so here I'm just marking a line that we haven't fully agreed on yet

> +  {
> +    my_error(ER_OUT_OF_RESOURCES, MYF(0));
> +    table->map= old_map;
> +    return true;
> +  }
> +
> +  lex.sql_command= old_lex->sql_command;
> +  thd->variables.sql_mode= 0;
> +
> +  TABLE_LIST const *tl= table->pos_in_table_list;
> +  DBUG_ASSERT(table->pos_in_table_list);
> +
> +  if (table->pos_in_table_list->security_ctx)
>      thd->security_ctx= tl->security_ctx;
> -  bool res= fix_session_vcol_expr(thd, vcol);
> +
> +  inited= true;
> +  return false;
> +}
> +
 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx