From b47bd3f8bf5dba600c5b7dc46fb77c0a8a73508c Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Fri, 14 Jun 2024 12:46:56 +0300 Subject: [PATCH] MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown Rowid Filter cannot be used with reverse-ordered scans, for the same reason as IndexConditionPushdown cannot be. test_if_skip_sort_order() already has logic to disable ICP when setting up a reverse-ordered scan. Added logic to also disable Rowid Filter in this case, factored out the code into prepare_for_reverse_ordered_access(), and added a comment describing the cause of this limitation. --- mysql-test/main/rowid_filter_innodb.result | 65 ++++++++++++ mysql-test/main/rowid_filter_innodb.test | 44 ++++++++ sql/sql_select.cc | 115 ++++++++++++++++----- 3 files changed, 196 insertions(+), 28 deletions(-) diff --git a/mysql-test/main/rowid_filter_innodb.result b/mysql-test/main/rowid_filter_innodb.result index 42c5f163e2c..c442ad43056 100644 --- a/mysql-test/main/rowid_filter_innodb.result +++ b/mysql-test/main/rowid_filter_innodb.result @@ -3947,3 +3947,68 @@ count(*) SET optimizer_switch=@save_optimizer_switch; DROP TABLE t0, t1; # End of 10.4 tests +# +# MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown +# +create table t1 ( +pk int primary key auto_increment, +a int, +b int, +f1 varchar(200), +f2 varchar(200), +f3 varchar(200), +f4 varchar(200), +f5 varchar(200), +key(a, pk), +key(b) +) engine=innodb; +insert into t1 (a,b,f1, f2, f3, f4) select +seq, seq, +repeat('1-', 100), +repeat('2-', 100), +repeat('3-', 100), +repeat('4-', 100) +from +seq_1_to_5000; +insert into t1 (a,b,f1, f2, f3, f4)select +30100, 30100, +'abcd','abcd','abcd','abcd' +from +seq_1_to_250; +insert into t1 (a,b,f1) values ( 110, 100, 12345); +analyze table t1; +Table Op Msg_type Msg_text +test.t1 analyze status Engine-independent statistics collected +test.t1 analyze status OK +# The following must NOT use Rowid Filter: +analyze format=json select * from t1 +where +a =30100 and b in (30100,30101,30102) +order by +pk desc; +ANALYZE +{ + "query_block": { + "select_id": 1, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "t1", + "access_type": "ref", + "possible_keys": ["a", "b"], + "key": "a", + "key_length": "5", + "used_key_parts": ["a"], + "ref": ["const"], + "r_loops": 1, + "rows": 250, + "r_rows": 250, + "r_table_time_ms": "REPLACED", + "r_other_time_ms": "REPLACED", + "filtered": 4.799086094, + "r_filtered": 100, + "attached_condition": "t1.a <=> 30100 and t1.b in (30100,30101,30102)" + } + } +} +drop table t1; diff --git a/mysql-test/main/rowid_filter_innodb.test b/mysql-test/main/rowid_filter_innodb.test index 65788ba19fc..e27c6366cf4 100644 --- a/mysql-test/main/rowid_filter_innodb.test +++ b/mysql-test/main/rowid_filter_innodb.test @@ -751,3 +751,47 @@ SET optimizer_switch=@save_optimizer_switch; DROP TABLE t0, t1; --echo # End of 10.4 tests + +--echo # +--echo # MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown +--echo # +create table t1 ( + pk int primary key auto_increment, + a int, + b int, + f1 varchar(200), + f2 varchar(200), + f3 varchar(200), + f4 varchar(200), + f5 varchar(200), + key(a, pk), + key(b) +) engine=innodb; + +insert into t1 (a,b,f1, f2, f3, f4) select + seq, seq, + repeat('1-', 100), + repeat('2-', 100), + repeat('3-', 100), + repeat('4-', 100) +from + seq_1_to_5000; + +insert into t1 (a,b,f1, f2, f3, f4)select + 30100, 30100, + 'abcd','abcd','abcd','abcd' +from + seq_1_to_250; +insert into t1 (a,b,f1) values ( 110, 100, 12345); +analyze table t1; + +--echo # The following must NOT use Rowid Filter: +--source include/analyze-format.inc +analyze format=json select * from t1 +where + a =30100 and b in (30100,30101,30102) +order by + pk desc; + +drop table t1; + diff --git a/sql/sql_select.cc b/sql/sql_select.cc index b99ea744ae8..06de7afa4f3 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -23957,6 +23957,90 @@ void compute_part_of_sort_key_for_equals(JOIN *join, TABLE *table, } +/* + @brief + This is called when switching table access to produce records + in reverse order. + + @detail + - Disable "Range checked for each record" (Is this strictly necessary + here?) + - Disable Index Condition Pushdown and Rowid Filtering. + + IndexConditionPushdownAndReverseScans, RowidFilteringAndReverseScans: + Suppose we're computing + + select * from t1 + where + key1 between 10 and 20 and extra_condition + order by key1 desc + + here the range access uses a reverse-ordered scan on (1 <= key1 <= 10) and + extra_condition is checked by either ICP or Rowid Filtering. + + Also suppose that extra_condition happens to be false for rows of t1 that + do not satisfy the "10 <= key1 <= 20" condition. + + For forward ordered range scan, the SQL layer will make these calls: + + h->read_range_first(RANGE(10 <= key1 <= 20)); + while (h->read_range_next()) { ... } + + The storage engine sees the end endpoint of "key1<=20" and can stop scanning + as soon as it encounters a row with key1>20. + + For backward-ordered range scan, the SQL layer will make these calls: + + h->index_read_map(key1=20, HA_READ_PREFIX_LAST_OR_PREV); + while (h->index_prev()) { + if (cmp_key(h->record, "key1=10" )<0) + break; // end of range + ... + } + + Note that the check whether we've walked beyond the key=10 endpoint is + made at the SQL layer. The storage engine has no information about the left + endpoint of the interval we're scanning. If all rows before that endpoint + do not satisfy ICP condition or do not pass the Rowid Filter, the storage + engine will enumerate the records until the table start. + + In MySQL, the API is extended with set_end_range() call so that the storage + engine "knows" when to stop scanning. +*/ + +static void prepare_for_reverse_ordered_access(JOIN_TAB *tab) +{ + /* Cancel "Range checked for each record" */ + if (tab->use_quick == 2) + { + tab->use_quick= 1; + tab->read_first_record= join_init_read_record; + } + /* + Cancel Pushed Index Condition, as it doesn't work for reverse scans. + */ + if (tab->select && tab->select->pre_idx_push_select_cond) + { + tab->set_cond(tab->select->pre_idx_push_select_cond); + tab->table->file->cancel_pushed_idx_cond(); + } + /* + The same with Rowid Filter: it doesn't work with reverse scans so cancel + it, too. + */ + { + /* + Rowid Filter is initialized at a later stage. It is not pushed to + the storage engine yet: + */ + DBUG_ASSERT(!tab->table->file->pushed_rowid_filter); + tab->range_rowid_filter_info= NULL; + delete tab->rowid_filter; + tab->rowid_filter= NULL; + } +} + + /** Test if we can skip the ORDER BY by using an index. @@ -24409,23 +24493,11 @@ check_reverse_order: tab->limit= 0; goto use_filesort; // Reverse sort failed -> filesort } - /* - Cancel Pushed Index Condition, as it doesn't work for reverse scans. - */ - if (tab->select && tab->select->pre_idx_push_select_cond) - { - tab->set_cond(tab->select->pre_idx_push_select_cond); - tab->table->file->cancel_pushed_idx_cond(); - } + prepare_for_reverse_ordered_access(tab); + if (select->quick == save_quick) save_quick= 0; // make_reverse() consumed it select->set_quick(tmp); - /* Cancel "Range checked for each record" */ - if (tab->use_quick == 2) - { - tab->use_quick= 1; - tab->read_first_record= join_init_read_record; - } } else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL && tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts) @@ -24438,20 +24510,7 @@ check_reverse_order: */ tab->read_first_record= join_read_last_key; tab->read_record.read_record_func= join_read_prev_same; - /* Cancel "Range checked for each record" */ - if (tab->use_quick == 2) - { - tab->use_quick= 1; - tab->read_first_record= join_init_read_record; - } - /* - Cancel Pushed Index Condition, as it doesn't work for reverse scans. - */ - if (tab->select && tab->select->pre_idx_push_select_cond) - { - tab->set_cond(tab->select->pre_idx_push_select_cond); - tab->table->file->cancel_pushed_idx_cond(); - } + prepare_for_reverse_ordered_access(tab); } } else if (select && select->quick)