MDEV-38649: Wrong result upon condition pushdown on table with DESC key

[DaveGosselin-MariaDB]

This PR covers two issues.

MDEV-38649: Wrong result upon condition pushdown on table with DESC key

QUICK_SELECT_DESC::get_next() sets a stale end range on the storage engine.
When checking the first range, it sets last_range to the first range and
then, if it cannot avoid descending scan, it sets the minimum endpoing on
the storage engine.  However, get_next() may decide that this range is not
a match after updating the minimum endpoint on the storage engine.  In this
case, it will continue to the next range.  When continuing, it needs to
reset not only the state of 'last_range' to point to the next range that
it's checking, but it also needs to clear the now-stale end range set on
the storage engine.  While this explanation covers the first and second
ranges, the issue at hand extends to the general case of ranges 1...N and
N+1.

Before this change and when get_next() decided that a range was not a match, it
would clear last_range at each loop continuation point.  Rather than clearing
the minimum endpoint on the storage engine at this point, we move any loop
cleanup to a lambda that's invoked by the for loop directly.  This consolidates
any logic that needs to be evaluated on loop continuation to one place and
reduces code duplication.

MySQL fixed this same problem at sha b65ca959efd6ec5369165b1849407318b4886634
with a different implementation.

MDEV-38921: Incorrect ORDER BY Result

In the case that we cannot avoid a descending scan and there's no
minimum endpoint (i.e., the minimum endpoint is zero), then clear
the end range on the storage engine.  This is necessary especially
when there are multiple ranges considered and a later range
indicates that there's no minimum endpoint (such as in the attached
test case), thus an earlier range endpoint must be removed/cleared.

[spetrunia]

Please fix the above and ok to push.

[spetrunia]

See the comment in https://jira.mariadb.org/browse/MDEV-38921 about https://jira.mariadb.org/browse/MDEV-38934 - needs to be fixed in the same PR

[spetrunia]

MDEV-38921: Incorrect ORDER BY Result

In the case that we cannot avoid a descending scan and there's no
minimum endpoint (i.e., the minimum endpoint is zero), then clear
the end range on the storage engine.  This is necessary especially
when there are multiple ranges considered and a later range
indicates that there's no minimum endpoint (such as in the attached
test case), thus an earlier range endpoint must be removed/cleared.

The MDEV title not very descriptive, follow the change in MDEV to:

MDEV-38921 Wrong result for range w/o min endpoint, ORDER BY DESC and ICP

Would wording like this be more clear:

In QUICK_SELECT_DESC::get_next(), when starting to scan a range with
no start endpoint, like "key < 10",  clear the end_range on the storage engine.
We could have scanned another range before, like "key BETWEEN 20 and 30"
and after this the engine's end_range points to its left endpoint, the "20".

[spetrunia]

The patch itself is ok.

[spetrunia]

See the above input.

[spetrunia]

The last fix is incorrect.
Let's adjust the testcase:

diff --git a/mysql-test/main/order_by.test b/mysql-test/main/order_by.test
index becc98d40b3..d7f9615dff0 100644
--- a/mysql-test/main/order_by.test
+++ b/mysql-test/main/order_by.test
@@ -2858,11 +2858,11 @@ CREATE TABLE t1 (
   INDEX(a)
 );
 INSERT INTO t1 SELECT seq, seq FROM seq_1_to_10000;
-SELECT COUNT(*) FROM t1 WHERE a > 9900 AND (a+1 > 9901) ORDER BY a DESC;
+SELECT COUNT(*) FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC;
 FLUSH STATUS;
 --disable_result_log
 --disable_ps_protocol
-SELECT * FROM t1 WHERE a > 9900 AND (a+1 > 9901) ORDER BY a DESC;
+SELECT * FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC;
 --enable_ps_protocol
 --enable_result_log
 SELECT * FROM information_schema.SESSION_STATUS WHERE VARIABLE_NAME LIKE '%icp%';

and the output will show familiar 10K ICP attempts:

 COUNT(*)
-100
+0
 FLUSH STATUS;
-SELECT * FROM t1 WHERE a > 9900 AND (a+1 > 9901) ORDER BY a DESC;
+SELECT * FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC;
 SELECT * FROM information_schema.SESSION_STATUS WHERE VARIABLE_NAME LIKE '%icp%';
 VARIABLE_NAME  VARIABLE_VALUE
-HANDLER_ICP_ATTEMPTS   100
-HANDLER_ICP_MATCH      100
+HANDLER_ICP_ATTEMPTS   10000
+HANDLER_ICP_MATCH      0
 DROP TABLE t1;

This is because set_end_range() call needs to be before the ha_index_last() call. The ha_index_last call already needs end_range to be set correctly.

What is the point of using so many statements:

SELECT COUNT(*) FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC;
FLUSH STATUS;
--disable_result_log
--disable_ps_protocol
SELECT * FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC;
--enable_ps_protocol
--enable_result_log

The SELECT COUNT(*) will use index-only scan and will not use ICP...
Why not just use ANALYZE SELECT ... ? This will show the query plan (to make sure we're using the intended query plan), and will update the counter values...

I think set_min_range is a poor name as one wonders what's the difference for set_end_range . How about reusing "set_end_range" name... set_end_range_for_endpoint ?

[spetrunia]

git log --oneline

80aeedacaa8 MDEV-38934: ICP, Reverse scan: range access will scan whole index for range             w/o max endpoint
8801d543fd1 MDEV-38934: ICP, Reverse scan: range access will scan whole index for range w/o max endpoint
1317b930da7 MDEV-38921: Wrong result for range w/o min endpoint, ORDER BY DESC and ICP
2f921d7eb66 MDEV-38921: Wrong result for range w/o min endpoint, ORDER BY DESC and ICP
520fc4bffbd MDEV-38649: Wrong result upon condition pushdown on table with DESC key
7b54798a6ff MDEV-38649: Wrong result upon condition pushdown to table with DESC key

It is confusing that there are commits with the same short name. They look like rebase duplicates.

If you are using two commits-per-fix (which most of others do not), please make it apparent, something like:

12345678  MDEV-38649: Testcase: Wrong result upon condition pushdown on table with DESC key
87654321 MDEV-38649: Wrong result upon condition pushdown to table with DESC key

[spetrunia]

520fc4bffbd MDEV-38649: Wrong result upon condition pushdown on table with DESC key
7b54798a6ff MDEV-38649: Wrong result upon condition pushdown to table with DESC key

Please remove mention of "condition pushdown" from here and MDEV title as we've agreed it's not relevant for the bug.

[spetrunia]

commit 520fc4bffbd40968e6dd3bf6dfd63440586ebb1c
Author: Dave Gosselin <dave.gosselin@mariadb.com>
Date:   Mon Feb 23 15:45:12 2026 -0500

    MDEV-38649: Wrong result upon condition pushdown on table with DESC key
    
...
    loop cleanup to a lambda that's invoked by the for loop directly.  This

Please remove mention of lambda as it is no longer used.

[spetrunia]

Please address the above

[DaveGosselin-MariaDB]

Thanks for the feedback, @spetrunia . I started using two commits per fix according to @sanja-byelkin 's latest "Recommended MariaDB Git workflow 2026" document came out wherein he suggests making a test that shows the bug, then solving the problem on top of that and updating the test case. Perhaps I incorrectly presumed this meant two commits (one with the test showing the bug, then another with the fix). I can squash the commits if you prefer. This accounts for the duplicate titles: it indicates that the two commits accompany one another for a given ticket.

Regarding "What is the point of using so many statements" I like using the counters available because they're less verbose than the analyze output, especially in cases like this one where we only care about one value from the analyze output and that value is captured by a counter. I disable the result log to avoid filling it with 100 rows of unimportant output. I disable the ps-protocol as that results in 2x ICP hits (1x for the query and another 1x under PS). Also, SELECT COUNT(*) FROM t1 WHERE a > 9900 AND (a+1 > 999000) ORDER BY a DESC; can show 100 results but generate no output when the COUNT is omitted (because the aggregate doesn't need to pull the resultset to get the count). So I have both that query and the query-under-test as a validation.

I'll remove the mentions of condition pushdown, but I'll also update the ticket titles to match. I didn't do that initially to preserve the original submission paper trail.