[jayzhan211]

Which issue does this PR close?

• Closes ORDER BY ... LIMIT returns wrong rows for List/Struct sort keys with NULL elements #25998.

Rationale for this change

ORDER BY <list or struct column> LIMIT n can return wrong rows when the sort key values contain NULL elements, because the TopK dynamic filter compares nested values with a NULL ordering that differs from the sort's. See #25998 for details and a reproducer.

What changes are included in this PR?

• TopK::build_filter_expression stops at the first sort key whose threshold value is nested (DataType::is_nested, which also covers dictionaries of nested values).
  • If that is the first key, no filter is built and the dynamic filter stays true.
  • Otherwise the filter keeps the terms for the earlier flat keys and adds "all earlier keys equal their thresholds" as the last disjunct, so rows tied on the flat prefix are kept.
• Filters for flat sort keys are unchanged. Nested sort keys only lose dynamic filter pruning; the TopK result is still computed with the row format, which follows the sort's NULL ordering.
• compare_op_for_nested and the semantics of < / > on nested types are not changed (the PiecewiseMergeJoinExec sibling is PiecewiseMergeJoin returns wrong results for List keys with NULL elements and fails on Struct keys #25957).

What is the testing strategy for this PR?

New sqllogictest cases in topk.slt cover a list key (ASC, DESC, NULLS FIRST), a struct key, a nested key after a flat key (ORDER BY a, l LIMIT 1 with ties on a), and a nested key followed by a flat key (ORDER BY l, b LIMIT 1). All of them except the NULLS FIRST case fail on main; NULLS FIRST happens to match the default nested ordering and is kept as a regression check. Existing EXPLAIN ANALYZE tests that show TopK dynamic filters on flat keys (e.g. dynamic_row_group_pruning.slt, push_down_filter_parquet.slt) pass unchanged.

Are there any user-facing changes?

Queries with ORDER BY on list/struct columns and LIMIT return correct results. No API changes.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.64%. Comparing base (7c67e4c) to head (512ed96).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25999      +/-   ##
==========================================
- Coverage   82.64%   82.64%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      445491   445508      +17     
  Branches   445491   445508      +17     
==========================================
+ Hits       368162   368175      +13     
- Misses      54972    54975       +3     
- Partials    22357    22358       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bvolpato]

LGTM, thanks for fixing this. Checked the nested-key fallback and flat-prefix ties; the TopK SQL regressions pass locally and fail without the change.

[kosiew]

@jayzhan211,

Thanks for the fix. Stopping TopK predicate construction at the first nested threshold addresses the NULL-ordering mismatch while preserving safe filtering on preceding scalar keys. The added SQL regressions cover the main affected cases, and the approach is appropriately scoped.

Predicate construction is the right boundary for this correction because pruning must agree with sorting semantics. The guard avoids incompatible nested comparisons without changing SQL comparison behavior or heap ordering. Disabling all filtering would lose safe prefix pruning, while introducing sort-aware comparisons would broaden the change unnecessarily.

I found no blocking issues. The fallback preserves rows tied on the safe prefix, including NULL-aware equality, and leaves filtering unrestricted when the first key is nested. I have two optional suggestions for additional regression coverage, but neither is required for approval.

[kosiew]

Optional: could we extend this test with explicit NULLS FIRST/LAST and a NULL flat-prefix threshold, followed by a better nested value tied on that NULL prefix? The current non-NULL a values do not exercise the retained null-aware equality branch, which must admit NULL-prefix rows under both orderings. Please compare LIMIT results against the full ordering.

[kosiew]

Lower-priority follow-up: could we add a LIMIT 2 test with ASC NULLS LAST, initially filling the heap with [1, NULL] and [1, 9], then introducing [1, 8] and [1, 7] in successive batches? The boundary should progress through [1, NULL], [1, 9], and [1, 8], producing [1, 7] and [1, 8]. Please ensure execution preserves the batch boundaries, since separate inserts alone do not guarantee repeated updates.

[jayzhan211]

Thanks @kosiew @bvolpato !