Rex Johnston added a comment - 2023-06-01 03:55 - edited 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.

Rex Johnston added a comment - 2023-06-01 22:38 Following your commit from 2019-02-03. Is this OK?

Rex Johnston added a comment - 2023-06-02 02:03 - edited 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

Michael Widenius added a comment - 2023-06-07 11:12 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...

Michael Widenius added a comment - 2023-06-07 18:53 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.