[DerZc]

Ordered partition scans keep one row per partition in queue buffers, while pushed index conditions use Field objects bound to table->record[0]. Reading directly into a queue buffer can therefore evaluate a condition against a different row and skip matching rows.

Route ha_index_last() and ha_index_prev() through the existing read_with_icp() helper, as already done for forward ordered reads. With ICP, read into table->record[0] and copy the row to the queue buffer only on success. EOF and other read errors leave the queue buffer unchanged.

Full forward index scans do not use ICP. Document and assert this invariant before calling ha_index_first() directly.

The minimal regression uses four rows in two MyISAM partitions, including matching rows and a row that the pushed condition must reject. The expected c1 values are 1, 2 and 4 in descending c2 order.

Bug report: https://jira.mariadb.org/browse/MDEV-39644

Validation

Based on 12.3 at 30728cb6725e793076a91d1b72865acb07a15bdf:

• Earlier negative controls: the unmodified server returns no rows for the regression, while all preceding results in main.partition_order match. Fixing only the initial reverse read incorrectly returns (3,2,0), confirming coverage of subsequent reads.
• The final source builds and installs in Release and assertion-enabled Debug configurations.
• Native MTR passes for main.partition_order, main.partition_index_scan (asc/desc), and main.select, in both ordinary and prepared-statement protocols on both builds: 16 successful combinations, without skips, retries, mismatches or cleanup failures.
• Eight independent full-index/range query checks per build pass across MyISAM and InnoDB in both directions (16 total). GDB confirms four executions of the new assertion site with no pushed condition during ascending full index scans.
• Eight local checks of the exact extracted switch arm and the project's Debug assertion macro pass. Invalid ICP states abort before the stub engine callback; no-ICP states preserve buffer routing, scan direction and engine return codes. These are assertion contract checks, not SQL crash reproductions.
• Earlier local testing of the exact read_with_icp() helper covered 72 buffer/error scenarios under AddressSanitizer and UndefinedBehaviorSanitizer. The unguarded helper failed 18 preservation cases; the guarded helper passed all 72. No new sanitizer build was run for this assertion-only revision.
• The previously reviewed minimal SQL regression is unchanged. The full server test suite was not run.

[gkodinov]

Thank you for your contribution! This is a preliminary review.

[gkodinov]

Thanks! LGTM. Please stand by for the final review.

[DaveGosselin-MariaDB]

Recommend adding this to keep the queue buffer unchanged on a failed read:

diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
index bc5ccf1bfdc..342643a8618 100644
--- a/sql/ha_partition.cc
+++ b/sql/ha_partition.cc
@@ -104,7 +104,8 @@ static int read_with_icp(TABLE* table,
     return func(record_buf);

   int error= func(table->record[0]);
-  memcpy(record_buf, table->record[0], record_buf_len);
+  if (likely(!error))
+    memcpy(record_buf, table->record[0], record_buf_len);
   return error;
 }

[DaveGosselin-MariaDB]

LGTM and passing to @spetrunia for final review.

[spetrunia]

The patch looks generally good.
Two points:

• The commit comment needs some rewording.
• Let's add an assert like this:

diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
index 342643a8618..ca3d6211e1e 100644
--- a/sql/ha_partition.cc
+++ b/sql/ha_partition.cc
@@ -8209,6 +8209,11 @@ int ha_partition::handle_ordered_index_scan(uchar *buf, bool reverse_order)
       /* Caller has specified reverse_order */
       break;
     case partition_index_first:
+      /*
+        We don't call read_with_icp() here as full index scans are not done
+        with ICP.
+      */
+      DBUG_ASSERT(!pushed_idx_cond);
       error= file->ha_index_first(rec_buf_ptr);
       reverse_order= FALSE;
       break;

Running it in CI now...

[spetrunia]

... CI didn't show any issues.

[spetrunia]

... and I see that the patch is now updated.

Thanks for quick turnaround, @DerZc !

The patch is ok to push now.

[spetrunia]

The failures are unrelated spurious errors, trying the other way.

[spetrunia]

After some fight with unrelated CI failures, pushed this:

5a3a6b5

commit 5a3a6b55823bab7741b576f563d507b47c617971 (HEAD -> 12.3, origin/bb-12.3-MDEV-39644-push, origin/12.3, bb-12.3-MDEV-39644-push)
Author: DerZc <34330257+DerZc@users.noreply.github.com>
Date:   Mon Sep 28 14:18:20 2026 +0800

    MDEV-39644 Wrong result with ICP in reverse ordered partition scans
    
    Ordered partition scans keep one row per partition in queue buffers.
    However, pushed index conditions use Field objects bound to
    table->record[0]. Reading directly into a queue buffer can therefore
    evaluate the condition against a different row and skip matching rows.
    
    Route ha_index_last() and ha_index_prev() through read_with_icp(), as
    already done for forward ordered reads. With ICP, read into
    table->record[0] and copy the row to the queue buffer only on success.
    Preserve the queue buffer when a read returns EOF or another error.
    
    Full forward index scans do not use ICP. Assert this invariant before
    calling ha_index_first() directly. Add a minimal MyISAM regression with
    matching and rejected rows in two partitions.
    
    Bug report: https://jira.mariadb.org/browse/MDEV-39644

@DerZc , thanks for the contribution!