Hi serg!

On Tue, Jan 30, 2018 at 12:06 AM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sachin!

On Jan 29, Sachin Setiya wrote:
> commit 905806689979ae91c89115f18b107ebacbc489e2
> Author: Sachin Setiya <sachin.setiya@maridb.com>
> Date:   Fri Jan 26 22:58:34 2018 +0530
>
>     Mdev-15085 Invisible Column Non-constant Default value results...
>
>     Problem:- If we create table field with dynamic default value then that
>      field always gets NULL value.
>
>     Analyze:- This is because in fill_record we simple continue at Invisible
>      column because we though that share->default_values(default value is
>      always copied into table->record[0] before insert) will have a default
>      value for them(which is true for constant defaults , but not for dynamic
>      defaults).
>
>     Solution:- In case of invisible field we will assign default value to 'value'
>      variable, but we do this only for columns which have non dynamic default value.
>      We do not have to worry for Auto_increment field , storage engine will take
>      of it.
>
> diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> index 45efa63..17ff7eb 100644
> --- a/sql/sql_base.cc
> +++ b/sql/sql_base.cc
> @@ -8382,7 +8382,12 @@ fill_record(THD *thd, TABLE *table, Field **ptr,
> List<Item> &values,
>
>      if (field->invisible)
>      {
> -      continue;
> +      if(field->flags & AUTO_INCREMENT_FLAG ||
> +          !field->default_value)
> +        continue;
> +      else
> +        value= new (thd->mem_root) Item_default_value(thd,
> +                            &thd->lex->select_lex.context);

No-no, you cannot create items in fill_record(). It would create a new
item for every inserted row.

I think the only thing you need to do here (and it doesn't depend on
AUTO_INCREMENT_FLAG or field->default_value), you need to set
all_fields_have_values to false. Like

  if (field->invisible)
  {
    all_fields_have_values= false;
    continue;
  }

Indeed, hidden columns did not get a value, so all_fields_have_values
should be false. I'd expect the rest will work automatically :)

Thanks for nice trick.
Patch updated. 
>      }
>      else
>        value=v++;

Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org



--
Regards
Sachin Setiya
Software Engineer at  MariaDB