MySQL Bugs: #117791: FORCE INDEX does not skip records_in

Bug #117791	FORCE INDEX does not skip records_in_range() for 'x IS NULL'
Submitted:	25 Mar 2025 13:30	Modified:	23 Mar 1:58
Reporter:	Kaiwang CHen (OCA)	Email Updates:
Status:	Open	Impact on me:	None
Category:	MySQL Server: Optimizer	Severity:	S3 (Non-critical)
Version:	8.0, 8.4, 9.2	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Contribution

Description:
Both WL#5957 and WL#6526 tried to save index dive overhead. They
differ in selected queries that, for WL#5957 reasonable estimation
is still needed, while for WL#6526 estimation is not needed at all 
because the best execution plan is already given by the user.

WL#6526 tried to unify both effort with one single control flag,
SKIP_RECORDS_IN_RANGE. When it is on, index dive of the range is
supposed to be skipped.

However, the flag was not set precisely, because index statistics
are not suitable for equality ranges with null parts, and they are 
even not present. Although originally WL#5957 checked both cases,
WL#6526 broke both features to some extent.

The skip flag with non-null test unexpectedly reduced the scope of
WL#6526, leading to index dive for 'x IS NULL'. Even having passed
the test, index statistics might not be ready, leading to 1 row for 
any equality range which is not reasonable.

How to repeat:
CREATE TABLE t1 (c1 INT, KEY (c1));
INSERT INTO t1 VALUES (1), (2), (3), (4), (5), (6), (7), (8), (NULL), (NULL);
ANALYZE TABLE t1;

set debug='+d,crash_records_in_range';

EXPLAIN SELECT * FROM t1 FORCE INDEX(c1) WHERE c1 IS NULL;

set debug='-d,crash_records_in_range';

Suggested fix:
Add more checks in the range iterator so that SKIP_RECORDS_IN_RANGE
itself precisely decides that alternative estimation should be used
instead of index dive. Then make the skip flag the only test to
use alternative estimation.

See enclosed in the next comment.

Add more checks in the range iterator so that SKIP_RECORDS_IN_RANGE itself precisely decides the alternative estimation. (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug_117791.patch (application/octet-stream, text), 5.62 KiB.

The second path adds null test for index statistics inside the alternative estimation branch, which makes force index nicer. (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug_117791_2.patch (application/octet-stream, text), 616 bytes.

Hello Kaiwang,

Thank you for the report and contribution.

regards,
Umesh

Hi Kaiwang,

Thank you for the contribution. We would like to apply your contribution.  However, it seems there is an issue with other storage engines than InnoDB.
Tests like perfschema.table_component and main.myisam hits an assert in handler::multi_range_read_info_const().  This needs to be resolved before we can take the contribution.

Here is a quick fix to illustrate the problem. It is the precheck in the range iterator does not match the test in the estimator here, so unavailable statistics (HA_POS_ERROR) in reported storage engines leads to the assert.

diff --git a/sql/handler.cc b/sql/handler.cc
index f6972876732..a6459404a1a 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -6416,7 +6416,8 @@ ha_rows handler::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
     else if (range.range_flag & SKIP_RECORDS_IN_RANGE) { // 2)
       if ((range.range_flag & EQ_RANGE) &&
           !(range.range_flag & NULL_RANGE) &&
-          (keyparts_used = std::popcount(range.start_key.keypart_map))) {
+          (keyparts_used = std::popcount(range.start_key.keypart_map)) &&
+          table->key_info[keyno].has_records_per_key(keyparts_used - 1)) {
         rows = static_cast<ha_rows>(
             table->key_info[keyno].records_per_key(keyparts_used - 1));
       } else {

However, I prefer to using a common function to avoid the problem. See the next comment.

Here is a quick fix to illustrate the problem. It is the precheck in the range iterator does not match the test in the estimator here, so unavailable statistics (HA_POS_ERROR) in reported storage engines leads to the assert.

diff --git a/sql/handler.cc b/sql/handler.cc
index f6972876732..a6459404a1a 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -6416,7 +6416,8 @@ ha_rows handler::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
     else if (range.range_flag & SKIP_RECORDS_IN_RANGE) { // 2)
       if ((range.range_flag & EQ_RANGE) &&
           !(range.range_flag & NULL_RANGE) &&
-          (keyparts_used = std::popcount(range.start_key.keypart_map))) {
+          (keyparts_used = std::popcount(range.start_key.keypart_map)) &&
+          table->key_info[keyno].has_records_per_key(keyparts_used - 1)) {
         rows = static_cast<ha_rows>(
             table->key_info[keyno].records_per_key(keyparts_used - 1));
       } else {

However, I prefer to using a common function to avoid the problem. See the next comment.

Enclosed is the full patch with a common function and the assert solved.

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug_117791_cc.patch (application/octet-stream, text), 8.14 KiB.

Enclosed is a fix for the assert only, using a common function can_use_index_statistics.

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug_117791_assert.patch (application/octet-stream, text), 5.89 KiB.