maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Thu, Apr 16, 2020 at 1:17 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

<cut>
> where's a test case for MDEV-22077?

One can't use the test case in MDEV-22077 as it involves a table
created by a 32 bit MariaDB
version. Doing another test case it's very hard so I ignored it and
just did run the test to verify
the patch.

> > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
> > index 19eabbb053c..2fc0de4345f 100644
> > --- a/sql/sql_delete.cc
> > +++ b/sql/sql_delete.cc
> > @@ -547,10 +547,12 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
> >      else
> >      {
> >        ha_rows scanned_limit= query_plan.scanned_rows;
> > +      table->no_keyread= 1;
> >        query_plan.index= get_index_for_order(order, table, select, limit,
> >                                              &scanned_limit,
> >                                              &query_plan.using_filesort,
> >                                              &reverse);
> > +      table->no_keyread= 0;
>
> why?

get_index_for_order is not allowed to change to use key_reads as there
is no guarantee
at this point that the suggested index will be used. That was the
reason we got crashes.

> > @@ -28568,8 +28568,6 @@ test_if_cheaper_ordering(const JOIN_TAB *tab, ORDER *order, TABLE *table,
> >    if (new_used_key_parts != NULL)
> >      *new_used_key_parts= best_key_parts;
> >    table->file->ha_end_keyread();
> > -  if (is_best_covering && !table->no_keyread)
> > -    table->file->ha_start_keyread(best_key);
>
> I find it very confusing that test_if_cheaper_ordering has a side effect
> of ending keyread. It was ok, when it ended previous keyread and started
> a new one. But just disabling - it's looks very strange.

It has to be disabled as we may in the future use another index than
the one we originally planned to use (with keyread).
This is something that should be re-factored in the future so that we
only enable key-read's when we have done the final
decision of which index we will really use.

Regards,
Monty