MDEV-40479 SELECT, order by desc, on partitioned table leads to incorrect results

[DerZc]

A descending scan of a partitioned table through a secondary index can stop early and omit qualifying rows.

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

Changes

ha_partition::handle_unordered_prev() compared the unordered prefix using key number 0 rather than the active index. Use active_index so the comparison matches the index that produced the row.

Regression coverage

The regression is integrated into the existing main.partition_order test. It scans a RANGE-partitioned table using a secondary (b,c) index and checks that ORDER BY c DESC returns all three values: 15, 11, and 6.

• mysql-test/main/partition_order.test
• mysql-test/main/partition_order.result

Validation

On 12.3 at 881b64e8add374802e1871814dd0a3a673a9b3e6:

• The server build passed with GCC 13 in Release mode.
• main.partition_order and main.select passed, with no test-state cleanup failures.
• main.partition_order also passed with --ps-protocol.
• The build and MTR results above apply to the exact source and regression files in this PR, verified by comparing their SHA-256 hashes against the tested files.
• git diff --check passed.
• The full regression suite and Windows builds were not run locally.

[CLAassistant]

All committers have signed the CLA.

[gkodinov]

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

Please squash your two commits. And avoid excess lines in .test files.

[gkodinov]

mostly looks good. but there's an extra file changed that shouldn't be a part of this.

[gkodinov]

LGTM. An optional suggestion below. Please stand by for the final review.

[gkodinov]

just a suggestion: I'd split the line at around column 80. Ideally, to improve visibility, I would try to make a "nice display" and split by logical parts, e.g.:

CREATE TABLE t1 (a INT, b INT, c INT,
                                 PRIMARY KEY(a, c),
                                 KEY(b, c))
  PARTITION BY RANGE(c)
    (PARTITION p1 VALUES LESS THAN(10),
     PARTITION p2 VALUES LESS THAN(20));

[holyfoot]

looks ok.

[mariadb-YuchenPei]

claude:

---

Review of 0a4bf82 (MDEV-40479)

The fix is correct. m_unordered_prefix_len is computed from m_curr_key_info[0], which is table->key_info + active_index (set in index_init, sql/ha_partition.cc:5710). So the prefix check in handle_unordered_prev() has to walk the key parts of active_index, not key 0.

In the test, key 0 is the PK (a,c). The buggy code compares the 4 bytes of a with the start key [null-byte][b…]. That comparison fails on the first index_prev in p2, which drops 11 and returns 15, 6. The fix makes it return all three rows.

Suggestions:

1. Make the test more robust. It relies on the optimizer picking a ref scan on (b,c) in reverse. If it ever switches to filesort, the test passes even without the fix. Add an EXPLAIN, or use FORCE INDEX(b), so the path being tested is pinned.
2. Test coverage (optional): add a KEY(b, c DESC) variant to cover m_unordered_reverse_scan, and ideally a case where the secondary-index prefix and PK key part 0 have different types or lengths.
3. Style:
   • sql/ha_partition.cc:7970 is over 80 columns, so wrap it after active_index,.
   • The CREATE TABLE line in the test is very long and would read better split.
4. Commit message:
   • There's a double space in the title ("desc, on").
   • "The regression scans…" should say "The regression test scans…".
5. Target branch: check whether this unordered-prefix path exists in older maintained branches. If it does, the fix should go to the oldest affected one rather than 12.3.

Existing issue, not caused by this commit: key_cmp_if_same() reads table->record[0], not buf. That's only correct if buf == record[0], which holds for the usual callers. A DBUG_ASSERT(buf == table->record[0]) would document that.