maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On Apr 13, Michael Widenius wrote:
> revision-id: faf8de3aa98 (mariadb-10.5.2-121-gfaf8de3aa98)
> parent(s): 3cbe15bd78c
> author: Michael Widenius <monty@xxxxxxxxxxx>
> committer: Michael Widenius <monty@xxxxxxxxxxx>
> timestamp: 2020-04-09 01:37:02 +0300
> message:
> 
> Fixed some assert crashes related to keyread.
> 
> - MDEV-22062 Assertion `!table->file->keyread_enabled()' failed in
>   close_thread_table()
> - MDEV-22077 table->no_keyread .. failed in join_read_first()
> 
> diff --git a/mysql-test/main/keyread.test b/mysql-test/main/keyread.test
> index d9d3002d392..76d0f5674dd 100644
> --- a/mysql-test/main/keyread.test
> +++ b/mysql-test/main/keyread.test
> @@ -8,3 +8,14 @@ create view v1 as select * from t1 where f2 = 1;
>  select distinct f1 from v1;
>  drop view v1;
>  drop table t1;
> +
> +#
> +# MDEV-22062 Assertion `!table->file->keyread_enabled()' failed in
> +# close_thread_table
> +#
> +
> +CREATE TABLE t1 (a INT NOT NULL, UNIQUE(a)) ENGINE=InnoDB;
> +INSERT INTO t1 VALUES (1),(2);
> +DELETE FROM t1 ORDER BY a LIMIT 1;
> +SELECT * FROM t1;
> +DROP TABLE t1;

where's a test case for MDEV-22077?

> 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?

>        if (!query_plan.using_filesort)
>          query_plan.scanned_rows= scanned_limit;
>      }
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index 129dae9eedb..1f05156b96f 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -23584,7 +23584,7 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
>           If ref_key used index tree reading only ('Using index' in EXPLAIN),
>           and best_key doesn't, then revert the decision.
>        */
> -      if (table->covering_keys.is_set(best_key))
> +      if (table->covering_keys.is_set(best_key) && !table->no_keyread)
>          table->file->ha_start_keyread(best_key);
>        else
>          table->file->ha_end_keyread();
> @@ -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.

>    DBUG_RETURN(TRUE);
>  }

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