fix: allow free RANGE window frames over Duration and Interval ORDER BY types

[edubraqd]

Which issue does this PR close?

• Part of Window functions with ORDER BY on a Duration / Interval / Struct / Map column fail with "Internal error: Cannot run range queries" #24915. This PR covers Duration and Interval ORDER BY types; Struct and Map stay rejected because their RANGE peer comparison (partial_cmp_struct / partial_cmp_map) does not match the sort order, and are left for a follow-up (fix the comparators, then extend supports_free_range_frame).

Rationale for this change

OVER (ORDER BY x) defaults to a free range frame with no offsets, which only needs the ORDER BY values to be comparable. coerce_window_frame nevertheless required an arithmetic target type for the ORDER BY column first, so row_number() OVER (ORDER BY x) on a Duration or Interval column failed with an internal error asking the user to file a bug.

What changes are included in this PR?

Only require the arithmetic target type for frames with a finite offset. A free range frame whose ORDER BY expressions are all either arithmetic types or Duration / Interval (also nested in Dictionary / RunEndEncoded) is returned unchanged; every ORDER BY expression is checked, since all of them take part in the peer comparison. Frames over supported types are coerced exactly as before, so plans do not change. The failure for unsupported types is now a planning error, RANGE window frames are not supported for ORDER BY type ..., which fits both free and offset frames.

Are these changes tested?

Yes. window.slt gains row_number / rank / count over Duration and Interval ORDER BY keys (default and explicit free range frame, single and two-column ORDER BY), and checks that Struct and Map keys, a Map after a Duration, and a finite offset over a Struct are still rejected at planning time. The existing Struct case that pinned the internal-error text now expects the planning error.

Are there any user-facing changes?

Window functions ordered by Duration or Interval columns work with the default frame. Unsupported ORDER BY types in a RANGE frame report a planning error instead of an internal error.

[kumarUjjawal]

Thank you @edubraqd for working on this. I left two comments, please take a look.

[kumarUjjawal]

This fallback now admits Map and Struct order keys, but RANGE peer detection does not use the same comparison as sorting. partial_cmp_map ignores map values it compares only entry column 0 and partial_cmp_struct skips child fields when either side is NULL. Therefore, distinct keys such as map(['a'], [1]) and map(['a'], [2]) can be treated as peers, making COUNT(*) OVER (ORDER BY x) return 2, 2 instead of 1, 2.

Could we either restrict this fallback to types whose peer comparison matches the sorter, or fix those comparators and add Map and NULL-containing Struct regression tests?

[kumarUjjawal]

These comment blocks repeat the same rationale, and the comment above plan_err! is inaccurate: this function is also reached for free RANGE frames; its error is created and then discarded by the caller. Could we keep one concise explanation beside the free-range handling and remove the duplicate block?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.64%. Comparing base (d02bd78) to head (ca7eba6).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/optimizer/src/analyzer/type_coercion.rs	80.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24916      +/-   ##
==========================================
+ Coverage   81.63%   81.64%   +0.01%     
==========================================
  Files        1123     1123              
  Lines      409546   410261     +715     
  Branches   409546   410261     +715     
==========================================
+ Hits       334319   334945     +626     
- Misses      55591    55624      +33     
- Partials    19636    19692      +56     

☔ 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.

[edubraqd]

Thanks for the review, both points addressed in the follow-up commit:

• The fallback is now limited to Duration and Interval ORDER BY types (plus those nested in Dictionary / RunEndEncoded), which compare the same way in the RANGE peer check as in the sort. Struct and Map keep being rejected at planning time; window.slt now has a Map case with distinct values and the Struct cases as planning errors.
• The explanation lives in one place, on the new supports_free_range_frame helper next to the fallback. The error in extract_window_frame_target_type no longer claims to be offset-specific: it now reads RANGE window frames are not supported for ORDER BY type ....

[kumarUjjawal]

Could you update the title and description to scope the change to Duration and Interval? Since issue #24915 also requests Struct and Map support, Closes #24915 should either be changed or the remaining work should be tracked separately.

[kumarUjjawal]

Free RANGE frames may contain multiple ORDER BY expressions, but this check validates only the first expression’s type. For example, ORDER BY duration_col, map_col is accepted because the first type is Duration, even though the second Map key still has the peer-comparison mismatch discussed earlier.

With equal duration values and maps that have the same key but different values, sorting distinguishes the rows while the RANGE peer check treats them as peers. COUNT(*) OVER (...) can therefore return 2, 2 instead of 1, 2.

Could we validate every ORDER BY expression used for peer comparison, or restrict this fallback to a single ORDER BY expression and add a regression test?

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

     Cloning apache/main
    Building datafusion-optimizer v55.0.0 (current)
       Built [  36.168s] (current)
     Parsing datafusion-optimizer v55.0.0 (current)
      Parsed [   0.025s] (current)
    Building datafusion-optimizer v55.0.0 (baseline)
       Built [  22.230s] (baseline)
     Parsing datafusion-optimizer v55.0.0 (baseline)
      Parsed [   0.026s] (baseline)
    Checking datafusion-optimizer v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.219s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/inherent_method_missing.ron

Failed in:
  TypeCoercionRewriter::coerce_asof_join, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/4c53331e39b3a9af4690d688c53480f38e370023/datafusion/optimizer/src/analyzer/type_coercion.rs:289

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  60.575s] datafusion-optimizer
    Building datafusion-sqllogictest v55.0.0 (current)
       Built [  79.404s] (current)
     Parsing datafusion-sqllogictest v55.0.0 (current)
      Parsed [   0.019s] (current)
    Building datafusion-sqllogictest v55.0.0 (baseline)
       Built [  80.032s] (baseline)
     Parsing datafusion-sqllogictest v55.0.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-sqllogictest v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.116s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [ 162.369s] datafusion-sqllogictest

[kumarUjjawal]

@edubraqd On the side note, I see you have around 14 prs https://github.com/apache/datafusion/issues?q=is%3Apr+is%3Aopen+author%3Aedubraqd opened. Reviews take a lot of bandwidths of members so please be mindful about this. You can pick 3 prs which you want to work next and make other as drafts so its easier both for you and the members to review and move forward.

[edubraqd]

Thanks, all three addressed:

• All ORDER BY expressions are now checked for the free-range fallback, not just the first one: each key has to be an arithmetic type or one accepted by supports_free_range_frame. window.slt gains ORDER BY duration, map (rejected at planning time) and ORDER BY duration, duration (runs).
• Title and description now say Duration / Interval, and the PR is "Part of Window functions with ORDER BY on a Duration / Interval / Struct / Map column fail with "Internal error: Cannot run range queries" #24915" rather than closing it; Struct / Map are left for a follow-up on the comparators.
• semver check: the missing coerce_asof_join was only the branch being behind main; rebased on current main.

On the number of open PRs: understood. I left #24916, #24903 and #24897 ready for review and converted the other eleven to drafts; I'll promote one draft at a time as these land.