revision-id: 6784d1ce238094f3cb16ad18a455810277320536 (mariadb-10.3.6-99-g6784d1c) parent(s): 90708fc15cf835d03b511a953ae6939081a0f9e1 author: Igor Babaev committer: Igor Babaev timestamp: 2018-12-07 12:13:26 -0800 message: MDEV-16188: Fixed problems of the implementation for rowid filter pushdown in InnoDB engine. Fixed some other problems at the SQL layer related to rowid filters. After this patch all tests in the main test suite passed including execution with --ps-protocol using both Debug and Release builds. --- mysql-test/main/join_cache.result | 2 +- sql/rowid_filter.cc | 7 +++++-- sql/rowid_filter.h | 10 +++++----- sql/sql_select.cc | 14 ++++++++++---- storage/innobase/handler/ha_innodb.cc | 14 ++++++++------ storage/innobase/row/row0sel.cc | 12 ++++++------ 6 files changed, 35 insertions(+), 24 deletions(-) diff --git a/mysql-test/main/join_cache.result b/mysql-test/main/join_cache.result index 46891e9..fde6e0f 100644 --- a/mysql-test/main/join_cache.result +++ b/mysql-test/main/join_cache.result @@ -1213,7 +1213,7 @@ ON City.Country=Country.Code AND City.Population > 5000000 WHERE Country.Name LIKE 'C%' AND Country.Population > 10000000; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE Country range Name Name 52 NULL # Using index condition; Using where; Rowid-ordered scan -1 SIMPLE City hash_range|filter Population,Country #hash#Country:Population|Population 3:4|4 world.Country.Code # Using where; Rowid-ordered scan; Using join buffer (flat, BNLH join); Using filter +1 SIMPLE City hash_range Population,Country #hash#Country:Population 3:4 world.Country.Code # Using where; Rowid-ordered scan; Using join buffer (flat, BNLH join) SELECT Country.Name, Country.Population, City.Name, City.Population FROM Country LEFT JOIN City ON City.Country=Country.Code AND City.Population > 5000000 diff --git a/sql/rowid_filter.cc b/sql/rowid_filter.cc index 5144319..f307fe5 100644 --- a/sql/rowid_filter.cc +++ b/sql/rowid_filter.cc @@ -213,7 +213,7 @@ Range_filter_cost_info for (uint i= best_filter_count; i < range_filter_cost_info_elements; i++) { Range_filter_cost_info *filter= &range_filter_cost_info[i]; - if (intersected_with->is_set(filter->key_no)) + if ((filter->key_no == ref_key_no) || intersected_with->is_set(filter->key_no)) continue; if (card < filter->intersect_x_axis_abcissa) break; @@ -233,10 +233,12 @@ bool Range_filter_ordered_array::fill() handler *file= table->file; THD *thd= table->in_use; QUICK_RANGE_SELECT* quick= (QUICK_RANGE_SELECT*) select->quick; - Item *pushed_idx_cond_save = file->pushed_idx_cond; + uint table_status_save= table->status; + Item *pushed_idx_cond_save= file->pushed_idx_cond; uint pushed_idx_cond_keyno_save= file->pushed_idx_cond_keyno; bool in_range_check_pushed_down_save= file->in_range_check_pushed_down; + table->status= 0; file->pushed_idx_cond= 0; file->pushed_idx_cond_keyno= MAX_KEY; file->in_range_check_pushed_down= false; @@ -264,6 +266,7 @@ bool Range_filter_ordered_array::fill() quick->range_end(); table->file->ha_end_keyread(); + table->status= table_status_save; file->pushed_idx_cond= pushed_idx_cond_save; file->pushed_idx_cond_keyno= pushed_idx_cond_keyno_save; file->in_range_check_pushed_down= in_range_check_pushed_down_save; diff --git a/sql/rowid_filter.h b/sql/rowid_filter.h index 4e3d7de..99fb75f 100644 --- a/sql/rowid_filter.h +++ b/sql/rowid_filter.h @@ -190,7 +190,7 @@ class Refpos_container_ordered_array : public Sql_alloc public: Refpos_container_ordered_array(uint elem_sz, uint max_elems) - : elem_size(elem_sz), max_elements(max_elems) {} + : elem_size(elem_sz), max_elements(max_elems), array(0) {} ~Refpos_container_ordered_array() { @@ -204,7 +204,7 @@ class Refpos_container_ordered_array : public Sql_alloc elem_size * max_elements/8 + 1); return array == NULL; } - + bool add(char *elem) { for (uint i= 0; i < elem_size; i++) @@ -227,7 +227,7 @@ class Refpos_container_ordered_array : public Sql_alloc { my_qsort2(array->front(), array->elements()/elem_size, elem_size, (qsort2_cmp) cmp, cmp_arg); - } + } }; class Range_filter_ordered_array : public Sql_alloc @@ -252,7 +252,7 @@ class Range_filter_ordered_array : public Sql_alloc bool is_filled() { return container_is_filled; } bool fill(); - + bool sort(); bool check(char *elem); @@ -276,7 +276,7 @@ class Rowid_filter : public Sql_alloc } bool is_active() - { + { return get_container()->is_filled(); } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c194fdb..bca13e6 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -10873,6 +10873,11 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond) sel->quick=tab->quick; // Use value from get_quick_... sel->quick_keys.clear_all(); sel->needed_reg.clear_all(); + if (is_hj && tab->rowid_filter) + { + delete tab->rowid_filter; + tab->rowid_filter= 0; + } } else { @@ -20421,6 +20426,8 @@ int join_init_read_record(JOIN_TAB *tab) if (tab->filesort && tab->sort_table()) // Sort table. return 1; + tab->fill_range_filter_if_needed(); + DBUG_EXECUTE_IF("kill_join_init_read_record", tab->join->thd->set_killed(KILL_QUERY);); if (tab->select && tab->select->quick && tab->select->quick->reset()) @@ -20436,8 +20443,7 @@ int join_init_read_record(JOIN_TAB *tab) if (!tab->preread_init_done && tab->preread_init()) return 1; - tab->fill_range_filter_if_needed(); - + if (init_read_record(&tab->read_record, tab->join->thd, tab->table, tab->select, tab->filesort_result, 1,1, FALSE)) return 1; @@ -25174,7 +25180,7 @@ int append_possible_keys(MEM_ROOT *alloc, String_list &list, TABLE *table, bool JOIN_TAB::save_filter_explain_data(Explain_table_access *eta) { - if (!filter) + if (!rowid_filter) return 0; (filter->selectivity*100 >= 1) ? eta->filter_perc= round(filter->selectivity*100) : eta->filter_perc= 1; @@ -25319,7 +25325,7 @@ bool JOIN_TAB::save_explain_data(Explain_table_access *eta, /* Build "key", "key_len", and "ref" */ - if (filter) + if (rowid_filter) { eta->key.set_filter(thd->mem_root, &filter->table->key_info[filter->key_no], diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index c521f9c..e43019b 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -7557,8 +7557,8 @@ ha_innobase::build_template( /* Below we check column by column if we need to access the clustered index. */ - if (pushed_rowid_filter) { - fetch_primary_key_cols = TRUE; + if (pushed_rowid_filter && rowid_filter_is_active) { + fetch_primary_key_cols = TRUE; m_prebuilt->pk_filter = this; } else { m_prebuilt->pk_filter = NULL; @@ -7584,8 +7584,9 @@ ha_innobase::build_template( /* Note that in InnoDB, i is the column number in the table. MySQL calls columns 'fields'. */ - if (active_index != MAX_KEY - && active_index == pushed_idx_cond_keyno) { + if ((active_index != MAX_KEY + && active_index == pushed_idx_cond_keyno) || + (pushed_rowid_filter && rowid_filter_is_active)) { ulint num_v = 0; /* Push down an index condition or an end_range check. */ @@ -7779,8 +7780,9 @@ ha_innobase::build_template( } } } - - m_prebuilt->idx_cond = this; + if (active_index == pushed_idx_cond_keyno) { + m_prebuilt->idx_cond = this; + } } else { no_icp: mysql_row_templ_t* templ; diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 2f2e74c..73c21d9 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -3816,7 +3816,7 @@ row_sel_enqueue_cache_row_for_mysql( /* For non ICP code path the row should already exist in the next fetch cache slot. */ - if (prebuilt->idx_cond != NULL) { + if (prebuilt->idx_cond != NULL || prebuilt->pk_filter != NULL ) { byte* dest = row_sel_fetch_last_buf(prebuilt); ut_memcpy(dest, mysql_rec, prebuilt->mysql_row_len); @@ -4427,7 +4427,7 @@ row_search_mvcc( mtr.commit(). */ ut_ad(!rec_get_deleted_flag(rec, comp)); - if (prebuilt->idx_cond) { + if (prebuilt->idx_cond || prebuilt->pk_filter) { switch (row_search_idx_cond_check( buf, prebuilt, rec, offsets)) { @@ -5359,7 +5359,7 @@ row_search_mvcc( result_rec = clust_rec; ut_ad(rec_offs_validate(result_rec, clust_index, offsets)); - if (prebuilt->idx_cond) { + if (prebuilt->idx_cond || prebuilt->pk_filter) { /* Convert the record to MySQL format. We were unable to do this in row_search_idx_cond_check(), because the condition is on the secondary index @@ -5420,7 +5420,7 @@ row_search_mvcc( /* We only convert from InnoDB row format to MySQL row format when ICP is disabled. */ - if (!prebuilt->idx_cond) { + if (!(prebuilt->idx_cond || prebuilt->pk_filter)) { /* We use next_buf to track the allocation of buffers where we store and enqueue the buffers for our @@ -5493,7 +5493,7 @@ row_search_mvcc( rec_offs_size(offsets)); mach_write_to_4(buf, rec_offs_extra_size(offsets) + 4); - } else if (!prebuilt->idx_cond) { + } else if (!(prebuilt->idx_cond || prebuilt->pk_filter)) { /* The record was not yet converted to MySQL format. */ if (!row_sel_store_mysql_rec( buf, prebuilt, result_rec, vrow, @@ -5727,7 +5727,7 @@ row_search_mvcc( DEBUG_SYNC_C("row_search_for_mysql_before_return"); - if (prebuilt->idx_cond != 0) { + if (prebuilt->idx_cond != 0 || prebuilt->pk_filter != 0) { /* When ICP is active we don't write to the MySQL buffer directly, only to buffers that are enqueued in the pre-fetch