maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Aug 30, Alexander Barkov wrote:
> revision-id: 899c0b3ec6e (mariadb-10.2.26-51-g899c0b3ec6e)
> parent(s): e4415549e53
> author: Alexander Barkov <bar@xxxxxxxxxxx>
> committer: Alexander Barkov <bar@xxxxxxxxxxx>
> timestamp: 2019-08-29 12:35:19 +0400
> message:
> 
> Part2: MDEV-18156 Assertion `0' failed or `btr_validate_index(index, 0, false)' in row_upd_sec_index_entry or error code 126: Index is corrupted upon DELETE with PAD_CHAR_TO_FULL_LENGTH
> 
> This patch allows the server to open old tables that have
> "bad" generated columns (i.e. indexed virtual generated columns,
> persistent generated columns) that depend on sql_mode,
> for general things like SELECT, INSERT, DROP, etc.
> Warning are issued in such cases.
> 
> Only these commands are now disallowed and return an error:
> - CREATE TABLE introducing a "bad" generated column
> - ALTER TABLE introducing a "bad" generated column
> - CREATE INDEX introdicing a "bad" generated column
>   (i.e. adding an index on a virtual generated column
>    that depends on sql_mode).
> 
> Note, these commands are allowed:
> - ALTER TABLE removing a "bad" generate column
> - ALTER TABLE removing an index from a "bad" virtual generated column
> - DROP INDEX removing an index from a "bad" virtual generated column
> but only if the table does not have any "bad" columns as a result.
> 
> diff --git a/sql/field.cc b/sql/field.cc
> index e2b745743d2..b720db19ebf 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -1428,16 +1428,24 @@ void Field::load_data_set_value(const char *pos, uint length,
>  }
>  
>  
> -void Field::error_generated_column_function_is_not_allowed(THD *thd) const
> +void Field::error_generated_column_function_is_not_allowed(THD *thd,
> +                                                           bool error) const
>  {
>    StringBuffer<64> tmp;
>    vcol_info->expr->print(&tmp, (enum_query_type)
>                                 (QT_TO_SYSTEM_CHARSET |
>                                  QT_ITEM_IDENT_SKIP_DB_NAMES |
>                                  QT_ITEM_IDENT_SKIP_TABLE_NAMES));
> -  my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0),
> -           tmp.c_ptr(), vcol_info->get_vcol_type_name(),
> -           const_cast<const char*>(field_name));
> +  if (error)
> +    my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0),
> +             tmp.c_ptr(), vcol_info->get_vcol_type_name(),
> +             const_cast<const char*>(field_name));
> +  else
> +    push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> +                        ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED,
> +                        ER_THD(thd, ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED),
> +                        tmp.c_ptr(), vcol_info->get_vcol_type_name(),
> +                        const_cast<const char*>(field_name));

an easier way of doing it would be

    my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED,
             MYF(error ? 0 : ME_WARNING),
             tmp.c_ptr(), vcol_info->get_vcol_type_name(),
             const_cast<const char*>(field_name));

>  }
>  
>  
> diff --git a/sql/table.h b/sql/table.h
> index 7786679982f..23a61345242 100644
> --- a/sql/table.h
> +++ b/sql/table.h
> @@ -325,6 +325,20 @@ enum tmp_table_type
>  };
>  enum release_type { RELEASE_NORMAL, RELEASE_WAIT_FOR_DROP };
>  
> +
> +enum vcol_init_mode
> +{
> +  VCOL_INIT_DEPENDENCY_FAILURE_IS_WARNING= 1,
> +  VCOL_INIT_DEPENDENCY_FAILURE_IS_ERROR= 2
> +  /*
> +    There will be a new flags soon,

better say "There may be new flags here"
because may be there won't be any new flags soon after all,
if this your fix will be sufficient :)

> +    e.g. to automatically remove sql_mode dependency:
> +      GENERATED ALWAYS AS (char_col) ->
> +      GENERATED ALWAYS AS (RTRIM(char_col))
> +  */
> +};
> +
> +
>  enum enum_vcol_update_mode
>  {
>    VCOL_UPDATE_FOR_READ= 0,

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx