revision-id: 92fa873fabfe28dc845b8384fe332a8e784d7f6f (mariadb-10.4.11-88-g92fa873fabf) parent(s): a17a327f116302612a889af7c448ef1cd8243f28 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-02-28 01:44:14 +0300 message: MDEV-21794: Optimizer flag rowid_filter leads to long query The issue is the same as Index Condition Pushdown had: before checking pushed index condition (or rowid filter), we must check that we have not walked out of the range we are scanning. If we fail to check this, we may end up scanning till the end of the table which is a huge performance killer for queries that scan small ranges. The solution is the same as in ICP: if we haven't make the end-of-range check for ICP, make it in Rowid Filter callback. --- include/my_compare.h | 3 +- mysql-test/main/rowid_filter_innodb.result | 71 ++++++++++++++++++++++++++++++ mysql-test/main/rowid_filter_innodb.test | 43 ++++++++++++++++++ sql/handler.cc | 12 ++++- sql/handler.h | 4 +- storage/innobase/row/row0sel.cc | 17 +++++-- storage/myisam/mi_extra.c | 2 +- storage/myisam/mi_key.c | 5 ++- storage/myisam/mi_rkey.c | 2 +- storage/myisam/mi_rnext.c | 2 +- storage/myisam/mi_rnext_same.c | 2 +- storage/myisam/mi_rprev.c | 2 +- storage/myisam/myisamdef.h | 6 +-- 13 files changed, 154 insertions(+), 17 deletions(-) diff --git a/include/my_compare.h b/include/my_compare.h index 6568e5f2f27..8d6e9ea1de5 100644 --- a/include/my_compare.h +++ b/include/my_compare.h @@ -152,6 +152,7 @@ typedef enum icp_result { } ICP_RESULT; typedef ICP_RESULT (*index_cond_func_t)(void *param); -typedef int (*rowid_filter_func_t)(void *param); +typedef ICP_RESULT (*rowid_filter_func_t)(void *param, my_bool icp_check_done); +typedef int (*rowid_filter_is_active_func_t)(void *param); #endif /* _my_compare_h */ diff --git a/mysql-test/main/rowid_filter_innodb.result b/mysql-test/main/rowid_filter_innodb.result index 9423fb18fd9..d885f6c4ea8 100644 --- a/mysql-test/main/rowid_filter_innodb.result +++ b/mysql-test/main/rowid_filter_innodb.result @@ -2521,6 +2521,77 @@ ORDER BY pk LIMIT 1; id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra 1 SIMPLE t1 index a,b PRIMARY 4 NULL 3008 3008.00 1.36 0.00 Using where DROP TABLE t1; +# +# MDEV-21794: Optimizer flag rowid_filter leads to long query +# +create table t10(a int); +insert into t10 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t11(a int); +insert into t11 select A.a + B.a* 10 + C.a * 100 from t10 A, t10 B, t10 C; +CREATE TABLE t1 ( +el_id int(10) unsigned NOT NULL , +el_index blob NOT NULL, +el_index_60 varbinary(60) NOT NULL, +filler blob, +PRIMARY KEY (el_id), +KEY el_index (el_index(60)), +KEY el_index_60 (el_index_60,el_id) +) engine=innodb; +insert into t1 +select +A.a+1000*B.a, +A.a+1000*B.a + 10000, +A.a+1000*B.a + 10000, +'filler-data-filler-data' +from +t11 A, t10 B; +# This must have key=el_index|el_index_60 +explain +select * from t1 +where el_index like '10%' and (el_index_60 like '10%' or el_index_60 like '20%'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range|filter el_index,el_index_60 el_index|el_index_60 62|62 NULL 1000 (10%) Using where; Using rowid filter +# This must show r_filtered=100%. r_filtered=10% means the bug is there. +analyze format=json +select * from t1 +where el_index like '10%' and (el_index_60 like '10%' or el_index_60 like '20%'); +ANALYZE +{ + "query_block": { + "select_id": 1, + "r_loops": 1, + "r_total_time_ms": "REPLACED", + "table": { + "table_name": "t1", + "access_type": "range", + "possible_keys": ["el_index", "el_index_60"], + "key": "el_index", + "key_length": "62", + "used_key_parts": ["el_index"], + "rowid_filter": { + "range": { + "key": "el_index_60", + "used_key_parts": ["el_index_60"] + }, + "rows": "REPLACED", + "selectivity_pct": "REPLACED", + "r_rows": 1000, + "r_selectivity_pct": 100, + "r_buffer_size": "REPLACED", + "r_filling_time_ms": "REPLACED" + }, + "r_loops": 1, + "rows": "REPLACED", + "r_rows": 1000, + "r_table_time_ms": "REPLACED", + "r_other_time_ms": "REPLACED", + "filtered": "REPLACED", + "r_filtered": 100, + "attached_condition": "t1.el_index like '10%' and (t1.el_index_60 like '10%' or t1.el_index_60 like '20%')" + } + } +} +drop table t10, t11, t1; SET global innodb_stats_persistent= @stats.save; # # MDEV-21610: Using rowid filter with BKA+MRR diff --git a/mysql-test/main/rowid_filter_innodb.test b/mysql-test/main/rowid_filter_innodb.test index 74349b8c6bb..36af2a30e56 100644 --- a/mysql-test/main/rowid_filter_innodb.test +++ b/mysql-test/main/rowid_filter_innodb.test @@ -380,6 +380,49 @@ SELECT * FROM t1 ORDER BY pk LIMIT 1; DROP TABLE t1; + +--echo # +--echo # MDEV-21794: Optimizer flag rowid_filter leads to long query +--echo # +create table t10(a int); +insert into t10 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); + +create table t11(a int); +insert into t11 select A.a + B.a* 10 + C.a * 100 from t10 A, t10 B, t10 C; + + +CREATE TABLE t1 ( + el_id int(10) unsigned NOT NULL , + el_index blob NOT NULL, + el_index_60 varbinary(60) NOT NULL, + filler blob, + + PRIMARY KEY (el_id), + KEY el_index (el_index(60)), + KEY el_index_60 (el_index_60,el_id) +) engine=innodb; + +insert into t1 +select + A.a+1000*B.a, + A.a+1000*B.a + 10000, + A.a+1000*B.a + 10000, + 'filler-data-filler-data' +from + t11 A, t10 B; + +--echo # This must have key=el_index|el_index_60 +explain +select * from t1 +where el_index like '10%' and (el_index_60 like '10%' or el_index_60 like '20%'); + +--echo # This must show r_filtered=100%. r_filtered=10% means the bug is there. +--source include/analyze-format-and-estimates.inc +analyze format=json +select * from t1 +where el_index like '10%' and (el_index_60 like '10%' or el_index_60 like '20%'); + +drop table t10, t11, t1; SET global innodb_stats_persistent= @stats.save; --echo # diff --git a/sql/handler.cc b/sql/handler.cc index 77b7f490590..4307a819eae 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -5983,12 +5983,20 @@ extern "C" enum icp_result handler_index_cond_check(void* h_arg) keys of the rows whose data is to be fetched against the used rowid filter */ -extern "C" int handler_rowid_filter_check(void *h_arg) +extern "C" +enum icp_result handler_rowid_filter_check(void *h_arg, my_bool icp_check_done) { handler *h= (handler*) h_arg; TABLE *tab= h->get_table(); + + if (!icp_check_done) + { + if (h->end_range && h->compare_key2(h->end_range) > 0) + return ICP_OUT_OF_RANGE; + } + h->position(tab->record[0]); - return h->pushed_rowid_filter->check((char *) h->ref); + return h->pushed_rowid_filter->check((char*)h->ref)? ICP_MATCH: ICP_NO_MATCH; } diff --git a/sql/handler.h b/sql/handler.h index 421941c0b35..95aef8c4cf8 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -2912,7 +2912,9 @@ class ha_statistics extern "C" enum icp_result handler_index_cond_check(void* h_arg); -extern "C" int handler_rowid_filter_check(void* h_arg); +extern "C" +enum icp_result handler_rowid_filter_check(void* h_arg, + my_bool icp_check_done); extern "C" int handler_rowid_filter_is_active(void* h_arg); uint calculate_key_len(TABLE *, uint, const uchar *, key_part_map); diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 7b6df752043..2e35311224c 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -3952,9 +3952,20 @@ row_search_idx_cond_check( ut_ad(len == DATA_ROW_ID_LEN); memcpy(prebuilt->row_id, data, DATA_ROW_ID_LEN); } - if (!handler_rowid_filter_check(prebuilt->pk_filter)) { - MONITOR_INC(MONITOR_ICP_MATCH); - return(ICP_NO_MATCH); + bool icp_check_done = prebuilt->idx_cond?true:false; + result = handler_rowid_filter_check(prebuilt->pk_filter, + icp_check_done); + switch (result) { + case ICP_NO_MATCH: + MONITOR_INC(MONITOR_ICP_NO_MATCH); + return(result); + case ICP_OUT_OF_RANGE: + MONITOR_INC(MONITOR_ICP_OUT_OF_RANGE); + return(result); + case ICP_MATCH: + break; + default: + ut_error; } } /* Convert the remaining fields to MySQL format. diff --git a/storage/myisam/mi_extra.c b/storage/myisam/mi_extra.c index d1bf903bd46..67cb714e7bf 100644 --- a/storage/myisam/mi_extra.c +++ b/storage/myisam/mi_extra.c @@ -424,7 +424,7 @@ void mi_set_index_cond_func(MI_INFO *info, index_cond_func_t func, void mi_set_rowid_filter_func(MI_INFO *info, rowid_filter_func_t check_func, - rowid_filter_func_t is_active_func, + rowid_filter_is_active_func_t is_active_func, void *func_arg) { info->rowid_filter_func= check_func; diff --git a/storage/myisam/mi_key.c b/storage/myisam/mi_key.c index dd838a05ada..22242d96810 100644 --- a/storage/myisam/mi_key.c +++ b/storage/myisam/mi_key.c @@ -530,9 +530,10 @@ ICP_RESULT mi_check_index_cond(register MI_INFO *info, uint keynr, } -int mi_check_rowid_filter(MI_INFO *info) +ICP_RESULT mi_check_rowid_filter(MI_INFO *info) { - return info->rowid_filter_func(info->rowid_filter_func_arg); + return info->rowid_filter_func(info->rowid_filter_func_arg, + (info->index_cond_func)?1:0); } int mi_check_rowid_filter_is_active(MI_INFO *info) diff --git a/storage/myisam/mi_rkey.c b/storage/myisam/mi_rkey.c index 8ef1d917f38..41edc35cc0f 100644 --- a/storage/myisam/mi_rkey.c +++ b/storage/myisam/mi_rkey.c @@ -122,7 +122,7 @@ int mi_rkey(MI_INFO *info, uchar *buf, int inx, const uchar *key, (info->index_cond_func && (res= mi_check_index_cond(info, inx, buf)) == ICP_NO_MATCH) || (mi_check_rowid_filter_is_active(info) && - !mi_check_rowid_filter(info))) + (res= mi_check_rowid_filter(info)) == ICP_NO_MATCH)) { uint not_used[2]; /* diff --git a/storage/myisam/mi_rnext.c b/storage/myisam/mi_rnext.c index 9f5f4702ed2..69ecc0f021f 100644 --- a/storage/myisam/mi_rnext.c +++ b/storage/myisam/mi_rnext.c @@ -104,7 +104,7 @@ int mi_rnext(MI_INFO *info, uchar *buf, int inx) (info->index_cond_func && (icp_res= mi_check_index_cond(info, inx, buf)) == ICP_NO_MATCH) || (mi_check_rowid_filter_is_active(info) && - !mi_check_rowid_filter(info))) + (icp_res= mi_check_rowid_filter(info)) == ICP_NO_MATCH)) { /* If we are at the last key on the key page, allow writers to diff --git a/storage/myisam/mi_rnext_same.c b/storage/myisam/mi_rnext_same.c index ee6b962c8c3..ededafaecac 100644 --- a/storage/myisam/mi_rnext_same.c +++ b/storage/myisam/mi_rnext_same.c @@ -97,7 +97,7 @@ int mi_rnext_same(MI_INFO *info, uchar *buf) (!info->index_cond_func || (icp_res= mi_check_index_cond(info, inx, buf)) != ICP_NO_MATCH) && (!mi_check_rowid_filter_is_active(info) || - mi_check_rowid_filter(info))) + (icp_res= mi_check_rowid_filter(info)) != ICP_NO_MATCH)) break; } } diff --git a/storage/myisam/mi_rprev.c b/storage/myisam/mi_rprev.c index cac0d672765..edac9e3f74d 100644 --- a/storage/myisam/mi_rprev.c +++ b/storage/myisam/mi_rprev.c @@ -61,7 +61,7 @@ int mi_rprev(MI_INFO *info, uchar *buf, int inx) (info->index_cond_func && (icp_res= mi_check_index_cond(info, inx, buf)) == ICP_NO_MATCH) || (mi_check_rowid_filter_is_active(info) && - !mi_check_rowid_filter(info))) + (icp_res= mi_check_rowid_filter(info)) == ICP_NO_MATCH)) { /* If we are at the last (i.e. first?) key on the key page, diff --git a/storage/myisam/myisamdef.h b/storage/myisam/myisamdef.h index f7b61ae638c..ca00df0a274 100644 --- a/storage/myisam/myisamdef.h +++ b/storage/myisam/myisamdef.h @@ -307,7 +307,7 @@ struct st_myisam_info index_cond_func_t index_cond_func; /* Index condition function */ void *index_cond_func_arg; /* parameter for the func */ rowid_filter_func_t rowid_filter_func; /* rowid filter check function */ - rowid_filter_func_t rowid_filter_is_active_func; /* is activefunction */ + rowid_filter_is_active_func_t rowid_filter_is_active_func; /* is activefunction */ void *rowid_filter_func_arg; /* parameter for the func */ THR_LOCK_DATA lock; uchar *rtree_recursion_state; /* For RTREE */ @@ -726,7 +726,7 @@ int mi_munmap_file(MI_INFO *info); void mi_remap_file(MI_INFO *info, my_off_t size); ICP_RESULT mi_check_index_cond(MI_INFO *info, uint keynr, uchar *record); -int mi_check_rowid_filter(MI_INFO *info); +ICP_RESULT mi_check_rowid_filter(MI_INFO *info); int mi_check_rowid_filter_is_active(MI_INFO *info); /* Functions needed by mi_check */ int killed_ptr(HA_CHECK *param); @@ -738,7 +738,7 @@ extern void mi_set_index_cond_func(MI_INFO *info, index_cond_func_t check_func, void *func_arg); extern void mi_set_rowid_filter_func(MI_INFO *info, rowid_filter_func_t check_func, - rowid_filter_func_t is_active_func, + rowid_filter_is_active_func_t is_active_func, void *func_arg); int flush_blocks(HA_CHECK *param, KEY_CACHE *key_cache, File file, ulonglong *dirty_part_map);