maria-developers team mailing list archive

[Vicențiu Ciorbaru <cvicentiu@xxxxxxxxx>]

Hi Varun!

I will take a second look at this but a first minor comment is to not save
two characters by shortening fields to flds, it makes reading unnatural.

Vicentiu

On Sat, 12 May 2018 at 21:45 Varun <varunraiko1803@xxxxxxxxx> wrote:

> revision-id: 4861977ce76761fefb9779a774984b6afa818f2e
> (mariadb-10.2.5-636-g4861977ce76)
> parent(s): 8b26fea83572cf5c043721b7835c3828937f9c27
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2018-05-13 00:09:15 +0530
> message:
>
> MDEV-14695: Assertion `n < m_size' failed in
> Bounds_checked_array<Element_type>::operator
>
> In this issue we hit the assert because we are adding addition fields to
> the field JOIN::all_fields list. This
> is done because  HEAP tables can't index BIT fields so  we need to use an
> additional hidden field for grouping because later it will be
> converted to a LONG field. Original field will remain of the BIT type and
> will be returned. This happens when we convert DISTINCT to
> GROUP BY.
>
> The solution is to take into account the number of such hidden fields that
> would be added to the field JOIN::all_fields list while calculating
> the size of the ref_pointer_array.
>
> ---
>  sql/sql_base.cc   | 9 ++++++---
>  sql/sql_base.h    | 4 ++--
>  sql/sql_delete.cc | 3 ++-
>  sql/sql_lex.cc    | 8 +++++++-
>  sql/sql_lex.h     | 5 +++++
>  sql/sql_select.cc | 4 +++-
>  sql/sql_union.cc  | 1 +
>  7 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> index 95ff41cb6f4..81ccfe5e514 100644
> --- a/sql/sql_base.cc
> +++ b/sql/sql_base.cc
> @@ -6919,7 +6919,7 @@ static bool setup_natural_join_row_types(THD *thd,
>
>  int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
>                List<Item> *sum_func_list,
> -              uint wild_num)
> +              uint wild_num, uint *hidden_bit_flds)
>  {
>    if (!wild_num)
>      return(0);
> @@ -6960,7 +6960,7 @@ int setup_wild(THD *thd, TABLE_LIST *tables,
> List<Item> &fields,
>        else if (insert_fields(thd, ((Item_field*) item)->context,
>                               ((Item_field*) item)->db_name,
>                               ((Item_field*) item)->table_name, &it,
> -                             any_privileges))
> +                             any_privileges, hidden_bit_flds))
>        {
>         if (arena)
>           thd->restore_active_arena(arena, &backup);
> @@ -7425,7 +7425,7 @@ bool get_key_map_from_key_list(key_map *map, TABLE
> *table,
>  bool
>  insert_fields(THD *thd, Name_resolution_context *context, const char
> *db_name,
>               const char *table_name, List_iterator<Item> *it,
> -              bool any_privileges)
> +              bool any_privileges, uint *hidden_bit_flds)
>  {
>    Field_iterator_table_ref field_iterator;
>    bool found;
> @@ -7545,6 +7545,9 @@ insert_fields(THD *thd, Name_resolution_context
> *context, const char *db_name,
>        else
>          it->after(item);   /* Add 'item' to the SELECT list. */
>
> +      if (item->type() == Item::FIELD_ITEM && item->field_type() ==
> MYSQL_TYPE_BIT)
> +        (*hidden_bit_flds)++;
> +
>  #ifndef NO_EMBEDDED_ACCESS_CHECKS
>        /*
>          Set privilege information for the fields of newly created views.
> diff --git a/sql/sql_base.h b/sql/sql_base.h
> index d6063f1b771..8d0dcb2f197 100644
> --- a/sql/sql_base.h
> +++ b/sql/sql_base.h
> @@ -154,11 +154,11 @@ bool fill_record_n_invoke_before_triggers(THD *thd,
> TABLE *table,
>                                            enum trg_event_type event);
>  bool insert_fields(THD *thd, Name_resolution_context *context,
>                    const char *db_name, const char *table_name,
> -                   List_iterator<Item> *it, bool any_privileges);
> +                   List_iterator<Item> *it, bool any_privileges, uint
> *hidden_bit_flds);
>  void make_leaves_list(THD *thd, List<TABLE_LIST> &list, TABLE_LIST
> *tables,
>                        bool full_table_list, TABLE_LIST *boundary);
>  int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
> -              List<Item> *sum_func_list, uint wild_num);
> +              List<Item> *sum_func_list, uint wild_num, uint *
> hidden_bit_flds);
>  bool setup_fields(THD *thd, Ref_ptr_array ref_pointer_array,
>                    List<Item> &item, enum_mark_columns mark_used_columns,
>                    List<Item> *sum_func_list, List<Item> *pre_fix,
> diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
> index 7b7a7e3f804..7bae7736cf3 100644
> --- a/sql/sql_delete.cc
> +++ b/sql/sql_delete.cc
> @@ -760,7 +760,8 @@ l
>                                      select_lex->leaf_tables, FALSE,
>                                      DELETE_ACL, SELECT_ACL, TRUE))
>      DBUG_RETURN(TRUE);
> -  if ((wild_num && setup_wild(thd, table_list, field_list, NULL,
> wild_num)) ||
> +  if ((wild_num && setup_wild(thd, table_list, field_list, NULL, wild_num,
> +                              &select_lex->hidden_bit_flds)) ||
>        setup_fields(thd, Ref_ptr_array(),
>                     field_list, MARK_COLUMNS_READ, NULL, NULL, 0) ||
>        setup_conds(thd, table_list, select_lex->leaf_tables, conds) ||
> diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
> index 3e36cac96b9..49150c863fe 100644
> --- a/sql/sql_lex.cc
> +++ b/sql/sql_lex.cc
> @@ -2143,6 +2143,7 @@ void st_select_lex::init_query()
>    m_agg_func_used= false;
>    window_specs.empty();
>    window_funcs.empty();
> +  hidden_bit_flds= 0;
>  }
>
>  void st_select_lex::init_select()
> @@ -2673,6 +2674,10 @@ ulong st_select_lex::get_table_join_options()
>
>  bool st_select_lex::setup_ref_array(THD *thd, uint order_group_num)
>  {
> +
> +  if (!((options & SELECT_DISTINCT) && !group_list))
> +    hidden_bit_flds= 0;
> +
>    // find_order_in_list() may need some extra space, so multiply by two.
>    order_group_num*= 2;
>
> @@ -2687,7 +2692,8 @@ bool st_select_lex::setup_ref_array(THD *thd, uint
> order_group_num)
>                         select_n_reserved +
>                         select_n_having_items +
>                         select_n_where_fields +
> -                       order_group_num) * 5;
> +                       order_group_num +
> +                       hidden_bit_flds) * 5;
>    if (!ref_pointer_array.is_null())
>    {
>      /*
> diff --git a/sql/sql_lex.h b/sql/sql_lex.h
> index 6d281930d1f..8283496604e 100644
> --- a/sql/sql_lex.h
> +++ b/sql/sql_lex.h
> @@ -956,6 +956,11 @@ class st_select_lex: public st_select_lex_node
>
>    /* it is for correct printing SELECT options */
>    thr_lock_type lock_type;
> +  /*
> +   it counts the number of bit fields in the SELECT list. These are used
> when DISTINCT is
> +   converted to a GROUP BY involving BIT fields.
> +  */
> +  uint hidden_bit_flds;
>
>    void init_query();
>    void init_select();
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index 4cd4754596b..83953661ce3 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -799,7 +799,9 @@ JOIN::prepare(TABLE_LIST *tables_init,
>        select_lex != select_lex->master_unit()->global_parameters())
>      real_og_num+= select_lex->order_list.elements;
>
> -  if (setup_wild(thd, tables_list, fields_list, &all_fields, wild_num))
> +  DBUG_ASSERT(select_lex->hidden_bit_flds == 0);
> +  if (setup_wild(thd, tables_list, fields_list, &all_fields, wild_num,
> +                 &select_lex->hidden_bit_flds))
>      DBUG_RETURN(-1);
>    if (select_lex->setup_ref_array(thd, real_og_num))
>      DBUG_RETURN(-1);
> diff --git a/sql/sql_union.cc b/sql/sql_union.cc
> index 3fe8a24f8bd..42904e341b6 100644
> --- a/sql/sql_union.cc
> +++ b/sql/sql_union.cc
> @@ -1474,6 +1474,7 @@ bool st_select_lex::cleanup()
>    }
>    inner_refs_list.empty();
>    exclude_from_table_unique_test= FALSE;
> +  hidden_bit_flds= 0;
>    DBUG_RETURN(error);
>  }
>
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits