maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

On Jul 29, Oleksandr Byelkin wrote:
> revision-id: 7311586ca25 (mariadb-10.2.31-317-g7311586ca25)
> parent(s): a1e52e7f32c
> author: Oleksandr Byelkin <sanja@xxxxxxxxxxx>
> committer: Oleksandr Byelkin <sanja@xxxxxxxxxxx>
> timestamp: 2020-07-17 13:47:26 +0200
> message:
> 
> MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT
> 
> Part 1: make better asserts.
> 
> diff --git a/sql/field.cc b/sql/field.cc
> index 65bd9d22857..cf0cc655888 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -11138,6 +11138,16 @@ bool Field::save_in_field_default_value(bool view_error_processing)
>  {
>    THD *thd= table->in_use;
>  
> +  if ( // table is not opened properly
> +      table == NULL || table->pos_in_table_list == NULL ||
> +      table->pos_in_table_list->select_lex == NULL ||
> +      // we are in subquery/derived/CTE
> +      !table->pos_in_table_list->top_table()->select_lex->is_top_level_select())
> +  {
> +    DBUG_ASSERT(0); // shoud not happened
> +    my_error(ER_INVALID_DEFAULT_PARAM, MYF(0));
> +    return false;
> +  }

that's kind of questionable. if this condition can never be true, why do
you have my_error() there? Just do asserts, like

  DBUG_ASSERT(table);
  DBUG_ASSERT(table->pos_in_table_list);
  DBUG_ASSERT(table->pos_in_table_list->select_lex);
  DBUG_ASSERT(table->pos_in_table_list->top_table()->select_lex->is_top_level_select());

>    if (flags & NO_DEFAULT_VALUE_FLAG &&
>        real_type() != MYSQL_TYPE_ENUM)
>    {
> @@ -11177,12 +11187,29 @@ bool Field::save_in_field_default_value(bool view_error_processing)
>  bool Field::save_in_field_ignore_value(bool view_error_processing)
>  {
>    enum_sql_command com= table->in_use->lex->sql_command;
> +
> +  if ( // table is not opened properly
> +      table == NULL || table->pos_in_table_list == NULL ||
> +      table->pos_in_table_list->select_lex == NULL ||
> +      // we are in subquery/derived/CTE
> +      !table->pos_in_table_list->top_table()->select_lex->is_top_level_select())
> +  {
> +    DBUG_ASSERT(0); // shoud not happened
> +    my_error(ER_INVALID_DEFAULT_PARAM, MYF(0));
> +    return false;
> +  }

ditto

>    // All insert-like commands
>    if (com == SQLCOM_INSERT || com == SQLCOM_REPLACE ||
>        com == SQLCOM_INSERT_SELECT || com == SQLCOM_REPLACE_SELECT ||
>        com == SQLCOM_LOAD)
>      return save_in_field_default_value(view_error_processing);
> -  return 0; // ignore
> +  if (com == SQLCOM_UPDATE || com == SQLCOM_UPDATE_MULTI)
> +    return 0; // ignore
> +
> +  // unexpected command
> +  DBUG_ASSERT(0); // shoud not happened

ditto

but why it should not happen? What if one does SELECT DEFAULT(f) FROM t?

> +  my_error(ER_INVALID_DEFAULT_PARAM, MYF(0));
> +  return false;
>  }
>  
>  
> diff --git a/sql/item.cc b/sql/item.cc
> index 9451d4203ca..382eb3ac47a 100644
> --- a/sql/item.cc
> +++ b/sql/item.cc
> @@ -3930,13 +3930,19 @@ int Item_param::save_in_field(Field *field, bool no_conversions)
>    case NULL_VALUE:
>      return set_field_to_null_with_conversions(field, no_conversions);
>    case DEFAULT_VALUE:
> -    return field->save_in_field_default_value(field->table->pos_in_table_list->
> +    return field->save_in_field_default_value((field->table &&
> +                                               field->table->pos_in_table_list)?

in what case can field->table be NULL here?

> +                                              field->table->pos_in_table_list->
>                                                top_table() !=
> -                                              field->table->pos_in_table_list);
> +                                              field->table->pos_in_table_list:
> +                                              FALSE);
>    case IGNORE_VALUE:
> -    return field->save_in_field_ignore_value(field->table->pos_in_table_list->
> +    return field->save_in_field_ignore_value((field->table &&
> +                                               field->table->pos_in_table_list)?
> +                                             field->table->pos_in_table_list->
>                                               top_table() !=
> -                                             field->table->pos_in_table_list);
> +                                             field->table->pos_in_table_list:
> +                                             FALSE);
>    case NO_VALUE:
>      DBUG_ASSERT(0); // Should not be possible
>      return true;
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx