MDEV-35443: opt_search_plan_for_table() may degrade to full table scan

opt_calc_index_goodness(): Correct an inaccurate condition.
We can very well use a clustered index of a table that is subject
to online rebuild. But we must not choose an index that has not been
committed (it is a secondary index that was not fully created)
or that is corrupted or not a normal B-tree index.

opt_search_plan_for_table(): Remove some redundant code, now that
opt_calc_index_goodness() checks against corrupted indexes.

The test case allows this code to be exercised. The main observation
in the following:
	./mtr --rr innodb.stats_persistent
	rr replay var/log/mysqld.1.rr/latest-trace
should be that when opt_search_plan_for_table() is being invoked by
dict_stats_update_persistent() on the being-altered statistics table
in the 2nd call after ha_innobase::inplace_alter_table(),
and the fix in opt_calc_index_goodness() is absent,
it would choose the code path if (n_fields == 0), that is, a full
table scan, instead of searching for the record. The GDB commands to
execute in "rr replay" would be as follows:
	break ha_innobase::inplace_alter_table
	continue
	break opt_search_plan_for_table
	continue
	continue
	next
	next
	…

Reviewed by: Vladislav Lesin

Author: dr-m

mysql-test/suite/innodb/r/stats_persistent.result

@@ -6,14 +6,23 @@ SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go';
 ANALYZE TABLE t1;
 connect con1, localhost, root;
 SET DEBUG_SYNC='now WAIT_FOR stop';
+connect con2, localhost, root;
+SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL astop WAIT_FOR ago';
+ALTER TABLE mysql.innodb_index_stats FORCE;
+connection con1;
 SELECT SUM(DATA_LENGTH+INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB';
 SUM(DATA_LENGTH+INDEX_LENGTH)
 SUM
+SET DEBUG_SYNC='now WAIT_FOR astop';
 SET DEBUG_SYNC='now SIGNAL go';
 disconnect con1;
 connection default;
 Table	Op	Msg_type	Msg_text
 test.t1	analyze	status	Engine-independent statistics collected
 test.t1	analyze	status	OK
+SET DEBUG_SYNC='now SIGNAL ago';
+connection con2;
+disconnect con2;
+connection default;
 SET DEBUG_SYNC= 'RESET';
 DROP TABLE t1;

mysql-test/suite/innodb/t/stats_persistent.test

@@ -14,14 +14,25 @@ SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go';
 --connect(con1, localhost, root)
 SET DEBUG_SYNC='now WAIT_FOR stop';
 
+--connect(con2, localhost, root)
+SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL astop WAIT_FOR ago';
+send ALTER TABLE mysql.innodb_index_stats FORCE;
+
+--connection con1
 --replace_column 1 SUM
 SELECT SUM(DATA_LENGTH+INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB';
 
+SET DEBUG_SYNC='now WAIT_FOR astop';
 SET DEBUG_SYNC='now SIGNAL go';
 --disconnect con1
 
 --connection default
 --reap
+SET DEBUG_SYNC='now SIGNAL ago';
+--connection con2
+reap;
+--disconnect con2
+--connection default
 SET DEBUG_SYNC= 'RESET';
 DROP TABLE t1;
 

storage/innobase/include/dict0mem.h

@@ -1203,6 +1203,14 @@ struct dict_index_t {
 	/** @return whether this is the change buffer */
 	bool is_ibuf() const { return UNIV_UNLIKELY(type & DICT_IBUF); }
 
+	/** @return whether this is a normal, non-virtual B-tree index
+	(not the change buffer, not SPATIAL or FULLTEXT) */
+	bool is_normal_btree() const noexcept {
+		return UNIV_LIKELY(!(type & (DICT_IBUF | DICT_SPATIAL
+					     | DICT_FTS | DICT_CORRUPT
+					     | DICT_VIRTUAL)));
+	}
+
 	/** @return whether the index includes virtual columns */
 	bool has_virtual() const { return type & DICT_VIRTUAL; }
 

storage/innobase/pars/pars0opt.cc

@@ -340,9 +340,7 @@ opt_calc_index_goodness(
 	ulint		op;
 	ulint		j;
 
-	/* At least for now we don't support using FTS indexes for queries
-	done through InnoDB's own SQL parser. */
-	if (dict_index_is_online_ddl(index) || (index->type & DICT_FTS)) {
+	if (!index->is_normal_btree() || !index->is_committed()) {
 		return(0);
 	}
 
@@ -572,11 +570,10 @@ opt_search_plan_for_table(
 	/* Calculate goodness for each index of the table */
 
 	index = dict_table_get_first_index(table);
-	best_index = index; /* Eliminate compiler warning */
+	best_index = index;
 	best_goodness = 0;
 
-	/* should be do ... until ? comment by Jani */
-	while (index) {
+	do {
 		goodness = opt_calc_index_goodness(index, sel_node, i,
 						   index_plan, &last_op);
 		if (goodness > best_goodness) {
@@ -590,8 +587,8 @@ opt_search_plan_for_table(
 			best_last_op = last_op;
 		}
 
-		dict_table_next_uncorrupted_index(index);
-	}
+		index = dict_table_get_next_index(index);
+	} while (index);
 
 	plan->index = best_index;