diff --git a/mysql-test/main/rowid_filter_innodb.result b/mysql-test/main/rowid_filter_innodb.result index 1bf63d9a378..6bc00da6f4c 100644 --- a/mysql-test/main/rowid_filter_innodb.result +++ b/mysql-test/main/rowid_filter_innodb.result @@ -2717,6 +2717,35 @@ SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a; a 1 5 +# +# MDEV-25163 Rowid filter does not process storage engine error correctly. +# +CREATE TABLE two(tw int) Engine=MyISAM; +INSERT INTO two VALUES (1),(2); +SET GLOBAL innodb_stats_persistent= OFF; +EXPLAIN EXTENDED +SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 range|filter b,c b|c 13|1027 NULL 5 (42%) 41.67 Using index condition; Using where; Using filesort; Using rowid filter +Warnings: +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`c` < 'k' and `test`.`t1`.`b` > 't' order by `test`.`t1`.`a` for update +EXPLAIN EXTENDED +SELECT a FROM t1,two WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE two ALL NULL NULL NULL NULL 2 100.00 Using temporary; Using filesort +1 SIMPLE t1 range|filter b,c b|c 13|1027 NULL 5 (42%) 41.67 Using index condition; Using where; Using join buffer (flat, BNL join); Using rowid filter +Warnings: +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` join `test`.`two` where `test`.`t1`.`c` < 'k' and `test`.`t1`.`b` > 't' order by `test`.`t1`.`a` for update +SET @saved_dbug = @@SESSION.debug_dbug; +SET debug_dbug = '+d,innodb_report_deadlock'; +SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +ERROR 40001: Deadlock found when trying to get lock; try restarting transaction +SET debug_dbug = @saved_dbug; +SET debug_dbug = '+d,innodb_report_deadlock'; +SELECT a FROM t1,two WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +ERROR 40001: Deadlock found when trying to get lock; try restarting transaction +SET debug_dbug = @saved_dbug; +DROP TABLE two; DROP TABLE t1; SET GLOBAL innodb_stats_persistent= @stats.save; # diff --git a/mysql-test/main/rowid_filter_innodb.test b/mysql-test/main/rowid_filter_innodb.test index f4d0b241d11..d5f3e4d4a6b 100644 --- a/mysql-test/main/rowid_filter_innodb.test +++ b/mysql-test/main/rowid_filter_innodb.test @@ -1,5 +1,6 @@ --source include/no_valgrind_without_big.inc --source include/have_innodb.inc +--source include/have_debug.inc SET SESSION STORAGE_ENGINE='InnoDB'; @@ -132,6 +133,40 @@ SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a; SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a; +--echo # +--echo # MDEV-25163 Rowid filter does not process storage engine error correctly. +--echo # + +CREATE TABLE two(tw int) Engine=MyISAM; +INSERT INTO two VALUES (1),(2); + +# Switch off statistics update to catch correct create lock function call +SET GLOBAL innodb_stats_persistent= OFF; + +# Make sure it's still rowid filter + filesort +EXPLAIN EXTENDED +SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; + +EXPLAIN EXTENDED +SELECT a FROM t1,two WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; + +SET @saved_dbug = @@SESSION.debug_dbug; +# Deadlock error should be returned from InnoDB during rowid filter container +# filling. If MDEV-25163 is not fixed, there will be assertion failure in +# InnoDB, as allready rolled back trx_t object will be used in filesort +# operation. +SET debug_dbug = '+d,innodb_report_deadlock'; +--error ER_LOCK_DEADLOCK +SELECT a FROM t1 WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +SET debug_dbug = @saved_dbug; + +# Same as above for the join query: +SET debug_dbug = '+d,innodb_report_deadlock'; +--error ER_LOCK_DEADLOCK +SELECT a FROM t1,two WHERE c < 'k' AND b > 't' ORDER BY a FOR UPDATE; +SET debug_dbug = @saved_dbug; + +DROP TABLE two; DROP TABLE t1; SET GLOBAL innodb_stats_persistent= @stats.save; diff --git a/sql/rowid_filter.cc b/sql/rowid_filter.cc index 75f2ab71ecd..9ca2d3849f3 100644 --- a/sql/rowid_filter.cc +++ b/sql/rowid_filter.cc @@ -536,8 +536,11 @@ TABLE::best_range_rowid_filter_for_partial_join(uint access_key_no, range filter and place into the filter the rowids / primary keys read from key tuples when doing this scan. @retval - false on success - true otherwise + Rowid_filter::SUCCESS on success + Rowid_filter::NON_FATAL_ERROR the error which does not require transaction + rollback + Rowid_filter::FATAL_ERROR the error which does require transaction + rollback @note The function assumes that the quick select object to perform @@ -550,9 +553,9 @@ TABLE::best_range_rowid_filter_for_partial_join(uint access_key_no, purposes to facilitate a lazy building of the filter. */ -bool Range_rowid_filter::fill() +Rowid_filter::build_return_code Range_rowid_filter::build() { - int rc= 0; + build_return_code rc= SUCCESS; handler *file= table->file; THD *thd= table->in_use; QUICK_RANGE_SELECT* quick= (QUICK_RANGE_SELECT*) select->quick; @@ -573,18 +576,34 @@ bool Range_rowid_filter::fill() table->file->ha_start_keyread(quick->index); if (quick->init() || quick->reset()) - rc= 1; - - while (!rc) + rc= FATAL_ERROR; + else { - rc= quick->get_next(); - if (thd->killed) - rc= 1; - if (!rc) + for (;;) { + int quick_get_next_result= quick->get_next(); + if (thd->killed) + { + rc= FATAL_ERROR; + break; + } + if (quick_get_next_result != 0) + { + rc= (quick_get_next_result == HA_ERR_END_OF_FILE ? SUCCESS + : FATAL_ERROR); + /* + The error state has been set by file->print_error(res, MYF(0)) call + inside quick->get_next() call, in Mrr_simple_index_reader::get_next() + */ + DBUG_ASSERT(rc == SUCCESS || thd->is_error()); + break; + } file->position(quick->record); - if (container->add(NULL, (char*) file->ref)) - rc= 1; + if (container->add(NULL, (char *) file->ref)) + { + rc= NON_FATAL_ERROR; + break; + } else tracker->increment_container_elements_count(); } @@ -599,10 +618,9 @@ bool Range_rowid_filter::fill() file->in_range_check_pushed_down= in_range_check_pushed_down_save; tracker->report_container_buff_size(table->file->ref_length); - if (rc != HA_ERR_END_OF_FILE) - return 1; - table->file->rowid_filter_is_active= true; - return 0; + if (rc == SUCCESS) + table->file->rowid_filter_is_active= true; + return rc; } diff --git a/sql/rowid_filter.h b/sql/rowid_filter.h index eabf1f05fec..1e9b2eb5971 100644 --- a/sql/rowid_filter.h +++ b/sql/rowid_filter.h @@ -217,6 +217,11 @@ protected: Rowid_filter_tracker *tracker; public: + enum build_return_code { + SUCCESS, + NON_FATAL_ERROR, + FATAL_ERROR, + }; Rowid_filter(Rowid_filter_container *container_arg) : container(container_arg) {} @@ -224,7 +229,7 @@ public: Build the filter : fill it with info on the set of elements placed there */ - virtual bool build() = 0; + virtual build_return_code build() = 0; /* Check whether an element is in the filter. @@ -269,7 +274,7 @@ public: ~Range_rowid_filter(); - bool build() { return fill(); } + build_return_code build(); bool check(char *elem) { @@ -280,8 +285,6 @@ public: return was_checked; } - bool fill(); - SQL_SELECT *get_select() { return select; } }; diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index f1dd23d9618..e7b096b420b 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -2338,7 +2338,11 @@ enum_nested_loop_state JOIN_CACHE::join_matching_records(bool skip_last) if ((rc= join_tab_execution_startup(join_tab)) < 0) goto finish2; - join_tab->build_range_rowid_filter_if_needed(); + if (join_tab->build_range_rowid_filter_if_needed()) + { + rc= NESTED_LOOP_ERROR; + goto finish2; + } /* Prepare to retrieve all records of the joined table */ if (unlikely((error= join_tab_scan->open()))) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 805ba19f0bc..d7badbc59a5 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -13626,8 +13626,18 @@ bool error_if_full_join(JOIN *join) } -void JOIN_TAB::build_range_rowid_filter_if_needed() +/** + Build rowid filter. + + @retval + 0 ok + @retval + 1 Error, transaction should be rolled back +*/ + +bool JOIN_TAB::build_range_rowid_filter_if_needed() { + bool result= false; if (rowid_filter && !is_rowid_filter_built) { /** @@ -13642,10 +13652,9 @@ void JOIN_TAB::build_range_rowid_filter_if_needed() Rowid_filter_tracker *rowid_tracker= rowid_filter->get_tracker(); table->file->set_time_tracker(rowid_tracker->get_time_tracker()); rowid_tracker->start_tracking(); - if (!rowid_filter->build()) - { + Rowid_filter::build_return_code build_rc= rowid_filter->build(); + if (build_rc == Rowid_filter::SUCCESS) is_rowid_filter_built= true; - } else { delete rowid_filter; @@ -13653,7 +13662,9 @@ void JOIN_TAB::build_range_rowid_filter_if_needed() } rowid_tracker->stop_tracking(); table->file->set_time_tracker(table_tracker); + result= (build_rc == Rowid_filter::FATAL_ERROR); } + return result; } @@ -20853,7 +20864,9 @@ sub_select(JOIN *join,JOIN_TAB *join_tab,bool end_of_records) if (!join_tab->preread_init_done && join_tab->preread_init()) DBUG_RETURN(NESTED_LOOP_ERROR); - join_tab->build_range_rowid_filter_if_needed(); + if (join_tab->build_range_rowid_filter_if_needed()) + DBUG_RETURN(NESTED_LOOP_ERROR); + if (join_tab->rowid_filter && join_tab->rowid_filter->is_empty()) rc= NESTED_LOOP_NO_MORE_ROWS; @@ -21810,7 +21823,8 @@ int join_init_read_record(JOIN_TAB *tab) if (tab->distinct && tab->remove_duplicates()) // Remove duplicates. return 1; - tab->build_range_rowid_filter_if_needed(); + if (tab->build_range_rowid_filter_if_needed()) + return 1; if (tab->filesort && tab->sort_table()) // Sort table. return 1; diff --git a/sql/sql_select.h b/sql/sql_select.h index 5c00a77638a..b0fa0b66b62 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -541,7 +541,7 @@ typedef struct st_join_table { /* Becomes true just after the used range filter has been built / filled */ bool is_rowid_filter_built; - void build_range_rowid_filter_if_needed(); + bool build_range_rowid_filter_if_needed(); void cleanup(); inline bool is_using_loose_index_scan()