[MDEV-30944] Range_rowid_filter::fill() leaves file->keyread at MAX_KEY Created: 2023-03-28  Updated: 2023-06-07  Resolved: 2023-06-07

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 11.0
Fix Version/s: 11.0.3, 11.1.2

Type: Bug Priority: Critical
Reporter: Alice Sherepa Assignee: Michael Widenius
Resolution: Fixed Votes: 0
Labels: None


 Description   

Repeatable on 11.0, not on 10.4-10.11, with Myisam, not Innodb

CREATE TABLE t1 ( a varchar(30) , i int , id int, UNIQUE KEY id (id), KEY (i ,id ,a), KEY (a(1),i)) engine=myisam;
 
 
 
INSERT INTO t1 VALUES('fej',NULL,1),('jeyw',8,2),(NULL,181,3),('wrkovd',9,4),('',NULL,5),('ko',NULL,6),('vdgzyxkop',217,7),('',7,8),('zy',0,9),('yxkopv',8,10),('kopv',1,11),('opv',4,12),('vc',9,13),('ri',1,14),('tkcn',1,15),('cnm',6,16),('m',0,17),('d',9,18),('e',28,19),(NULL,0,20);
 
 
 
SELECT i, MAX( id ) FROM t1 
WHERE ( a IS NULL OR a IN ('o','h') ) AND ( id BETWEEN 6 AND 7  OR id IN ( 8, 1) ) 
GROUP BY i;


Version: '11.0.2-MariaDB-debug-log'  
mariadbd: /11.0/src/sql/sql_select.cc:24092: int join_read_first(JOIN_TAB*): Assertion `table->no_keyread || !table->covering_keys.is_set(tab->index) || table->file->keyread == tab->index' failed.
 
230328 14:15:23 [ERROR] mysqld got signal 6 ;
 
 
 
Server version: 11.0.2-MariaDB-debug-log source revision: 6c3b1dced41c238f814aeabbc725e31c3106c0f4
 
 
 
??:0(gsignal)[0x7fc82b8b100b]
 
??:0(abort)[0x7fc82b890859]
 
/lib/x86_64-linux-gnu/libc.so.6(+0x22729)[0x7fc82b890729]
 
??:0(__assert_fail)[0x7fc82b8a1fd6]
 
sql/sql_select.cc:24095(join_read_first(st_join_table*))[0x561e247410f9]
 
sql/sql_select.cc:23033(sub_select(JOIN*, st_join_table*, bool))[0x561e247393f9]
 
sql/sql_select.cc:22568(do_select(JOIN*, Procedure*))[0x561e24737336]
 
sql/sql_select.cc:4895(JOIN::exec_inner())[0x561e246b8157]
 
sql/sql_select.cc:4672(JOIN::exec())[0x561e246b5508]
 
sql/sql_select.cc:5153(mysql_select(THD*, TABLE_LIST*, List<Item>&, Item*, unsigned int, st_order*, st_order*, Item*, st_order*, unsigned long long, select_result*, st_select_lex_unit*, st_select_lex*))[0x561e246b9c16]
 
sql/sql_select.cc:611(handle_select(THD*, LEX*, select_result*, unsigned long long))[0x561e24689819]
 
sql/sql_parse.cc:6267(execute_sqlcom_select(THD*, TABLE_LIST*))[0x561e245abac5]
 
sql/sql_parse.cc:3949(mysql_execute_command(THD*, bool))[0x561e2459a374]
 
sql/sql_parse.cc:7999(mysql_parse(THD*, char*, unsigned int, Parser_state*))[0x561e245b6848]
 
sql/sql_parse.cc:1896(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool))[0x561e2458ca9e]
 
sql/sql_parse.cc:1407(do_command(THD*, bool))[0x561e245897da]
 
sql/sql_connect.cc:1416(do_handle_one_connection(CONNECT*, bool))[0x561e24a56c33]
 
sql/sql_connect.cc:1320(handle_one_connection)[0x561e24a56590]
 
perfschema/pfs.cc:2203(pfs_spawn_thread)[0x561e2566fcd8]
 
nptl/pthread_create.c:478(start_thread)[0x7fc82bdbc609]
 
 
 
Query (0x6290001092a8): SELECT i, MAX( id ) FROM t1 
WHERE ( a IS NULL OR a IN ('o','h') ) AND ( id BETWEEN 6 AND 7  OR id IN ( 8, 1) ) 
GROUP BY i

also 11.0/src/sql/sql_select.cc:24131: int join_read_last(JOIN_TAB*): Assertion `table->no_keyread || !table->covering_keys.is_set(tab->index) || table->file->keyread == tab->index'



 Comments   
Comment by Rex Johnston [ 2023-06-01 ]

assert failure in sub_select()
error= (*join_tab->read_first_record)(join_tab);
table->file->keyread == tab->index

something is altering keyread when the table is MyISAM, not InnoDB

The optimizer, when testing for rowid_filters
best_access_path()/apply_filter()

        if (table->can_use_rowid_filter(key_no))
 
        {
 
          filter= table->best_range_rowid_filter(key_no,
 
                                                 rows2double(range->rows),
 
                                                 file->cost(&tmp),
 
                                                 file->cost(tmp.index_cost),
 
                                                 record_count,
 
                                                 &records_best_filter);
 
          set_if_smaller(best.records_out, records_best_filter);
 
          if (filter)
 
          {
 
            filter= filter->apply_filter(thd, table, &tmp,
 
                                         &records_after_filter,
 
                                         &startup_cost,
 
                                         range->ranges,
 
                                         record_count);
 
 ...

decides that when using MyISAM, a rowid filter is faster than not using one.
When using InnoDB, the costs are slightly different and it chooses NOT to include a rowid filter.

Filling the rowid filter object container

sub_select()/build_range_rowid_filter()

  if (unlikely(join_tab->rowid_filter))
 
  {
 
    if (unlikely(join_tab->need_to_build_rowid_filter))
 
    {
 
      join_tab->build_range_rowid_filter();
 
...

ends up calling Range_rowid_filter::fill()

  quick->range_end();
 
  file->ha_end_keyread();

ha_end_keyread() sets file->keyread to MAX_KEY

back up the stack, in sub_select(), a subsequent call

  if (rc != NESTED_LOOP_NO_MORE_ROWS)
 
  {
 
    error= (*join_tab->read_first_record)(join_tab);

executes the assert

  DBUG_ASSERT(table->no_keyread ||
 
              !table->covering_keys.is_set(tab->index) ||
 
              table->file->keyread == tab->index);

each element of which, in this case will be false.

The obvious solution is reset the keyread variable back to it's state as at the start of the Range_rowid_filter::fill() routine, like other similar table/file variables.

rowid filters are evaluated in Range_rowid_filter_cost_info::apply_filter()

  org_cost= (file->cost_for_reading_multiple_times(prev_records,
 
                                                   cost) +
 
             records * tmp);
 
 
 
  new_cost= (file->cost_for_reading_multiple_times(prev_records,
 
                                                   &adjusted_cost) +
 
             new_records * tmp + filter_startup_cost);

Any scenario where new_cost < org_cost, will lead to a rowid filter being constructed and used.

Comment by Rex Johnston [ 2023-06-01 ]

Following your commit from 2019-02-03. Is this OK?

Comment by Rex Johnston [ 2023-06-02 ]

output from

drop table if exists t1;
 
CREATE TABLE t1 ( a varchar(30) , i int , id int, UNIQUE KEY id (id), KEY (i ,id ,a), KEY (a(1),i)) engine=myisam;
 
 
INSERT INTO t1 VALUES('fej',NULL,1),('jeyw',8,2),(NULL,181,3),('wrkovd',9,4),('',NULL,5),('ko',NULL,6),('vdgzyxkop',217,7),('',7,8),('zy',0,9),('yxkopv',8,10),('kopv',1,11),('opv',4,12),('vc',9,13),('ri',1,14),('tkcn',1,15),('cnm',6,16),('m',0,17),('d',9,18),('e',28,19),(NULL,0,20);
 
 
explain format=json SELECT i, MAX( id ) FROM t1 
WHERE ( a IS NULL OR a IN ('ko','h') ) AND ( id BETWEEN 6 AND 7  OR id IN ( 8, 1) ) 
GROUP BY i;

for 11.0

  "query_block": {
 
    "select_id": 1,
 
    "cost": 0.006729708,
 
    "nested_loop": [
 
      {
 
        "table": {
 
          "table_name": "t1",
 
          "access_type": "index",
 
          "possible_keys": ["id", "a"],
 
          "key": "i",
 
          "key_length": "43",
 
          "used_key_parts": ["i", "id", "a"],
 
          "rowid_filter": {
 
            "range": {
 
              "key": "a",
 
              "used_key_parts": ["a"]
 
            },
 
            "rows": 5,
 
            "selectivity_pct": 25
 
          },
 
          "loops": 1,
 
          "rows": 20,
 
          "cost": 0.006099522,
 
          "filtered": 5,
 
          "attached_condition": "(t1.a is null or t1.a in ('ko','h')) and (t1.`id` between 6 and 7 or t1.`id` in (8,1))",
 
          "using_index": true
 
        }
 
      }
 
    ]
 
  }
 
}

for 10.11

  "query_block": {
 
    "select_id": 1,
 
    "nested_loop": [
 
      {
 
        "table": {
 
          "table_name": "t1",
 
          "access_type": "index",
 
          "possible_keys": ["id", "a"],
 
          "key": "i",
 
          "key_length": "43",
 
          "used_key_parts": ["i", "id", "a"],
 
          "rows": 20,
 
          "filtered": 5,
 
          "attached_condition": "(t1.a is null or t1.a in ('ko','h')) and (t1.`id` between 6 and 7 or t1.`id` in (8,1))",
 
          "using_index": true
 
        }
 
      }
 
    ]
 
  }

we can clearly see the new cost based optimization in action.

These news costs introduced in commit d9d0e78039fd3fbeac814edd27fabfe3e4450bc5

Comment by Michael Widenius [ 2023-06-07 ]

I don't think the current patch with saving and restoring keyread will work.
The reason is that the engine does other processing when keyread is changed.
I will come up with a better patch...

Comment by Michael Widenius [ 2023-06-07 ]

MDEV-30944 Range_rowid_filter::fill() leaves file->keyread at MAX_KEY

This test case exposed 2 different bugs:

  • When replacing a range with an index scan on a covering key
    in test_if_skip_sort_order() we didn't disable filtering.
    Filtering does not make much sense in this case.
  • Fixed by disabling filtering in this case.
  • Range_rowid_filter::fill() did not take into account that keyread
    could already active, which caused an assert when it tried to
    activate another keyread.
  • Fixed by remembering old keyread state at start and restoring it
    at end.
Generated at Thu Feb 08 10:20:05 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.