[iceboundrock]

Problem

DataFusion 55 assumes a window aggregate's output is already sorted, based only on the aggregate: max, count and unsigned sum only rise as values join the set, and min only falls. window_equivalence_properties turns that into an ordering of the window column without looking at the frame, and EnforceSorting then drops a later sort that the ordering seems to satisfy. For the windows LTSeq builds, that ordering is often false, so sort("m") came back in the window's order. slice, head, tail and the writers followed it, with no error.

Reproduced on main (bc2cbb2) before this change:

Window	Table order	sort("m") lazily	after collect()
rolling(2).max(), int64	x desc	[3, 3, 2, 1]	[1, 2, 3, 3]
rolling(2).max(), string	x desc	['c', 'c', 'b']	['b', 'c', 'c']
rolling(2).sum(), uint64	x desc	[3, 5, 3, 1]	[1, 3, 3, 5]
rolling(2).count(), [1, 2, None, None]	x asc	[1, 2, 1, 0]	[0, 1, 1, 2]
cum_max(), [None, 1, 2]	x desc	[None, 2, 2]	[2, 2, None]
cum_sum(), uint64 [None, 1, 2]	x desc	[None, 2, 3]	[2, 3, None]
cum_sum(), uint64 [2**64 - 1, 1]	row order	[2**64 - 1, 0]	[0, 2**64 - 1]

The first four rows are the issue's cases. The last three are not in the issue:

• A running frame is claimed ASC NULLS LAST, but a running max or sum is NULL until the first non-NULL value, so those NULL rows led the column. This is the problem tracked upstream in Wrong results: ORDER BY ... NULLS LAST on a running MAX window function puts the NULL row first apache/datafusion#27267.
• DataFusion's sum wraps unsigned integers on overflow, so an unsigned running or rolling sum is not monotonic either.

A rolling min (claimed DESC NULLS LAST) never matched sort(desc=True), which asks for NULLs first, so no dropped sort was seen for it, as the issue predicted.

Change

src/transpiler/no_order_claim.rs adds NoOrderClaim, an AggregateUDFImpl wrapper that delegates everything to the wrapped aggregate (name, signature, return field and nullability, accumulators including the sliding one, statistics, simplification, reversal, beneficial ordering) except set_monotonicity, which returns NotMonotonic. aggregate_to_window, which builds every window aggregate LTSeq plans (rolling, cum_*, LTSeq.cum_sum, the group dialect), wraps the aggregate with it. Wrapping is idempotent, and a reversed aggregate is wrapped too. Name-based rewrites (the m0 checked and exact sums, narrow_decimals) still see sum; their replacements claim no order either.

In DataFusion 55, set monotonicity is read only for window orderings (window_equivalence_properties and PlainAggregateWindowExpr::add_equal_orderings), so grouped and ungrouped aggregates are unaffected. The #253 float min/max override stays as it is.

Why not frame-aware

set_monotonicity sees only the argument type. A narrower fix would keep the claim for running frames over non-nullable arguments. In practice almost no LTSeq column is non-nullable (from_dict, Arrow, CSV and most Parquet columns are nullable), and unsigned sum would still have to drop it because it wraps. So that variant would keep almost nothing while adding nullability plumbing. If the lost sort elision turns out to matter, recovering it is the issue's phase 2.

Cost

A sort by a window column now always runs. The only sort previously skipped correctly was the one after a cum_max/cum_min of a column with no NULLs. Physical plan for sort("m") after rolling(2).max() over x sorted descending:

Before (sort dropped):

ProjectionExec: expr=[x@0 as x, max(...) ROWS BETWEEN 1 PRECEDING AND CURRENT ROW@1 as m]
  BoundedWindowAggExec: ... mode=[Sorted]
    SortExec: expr=[x@0 DESC], preserve_partitioning=[false]
      DataSourceExec: partitions=1, partition_sizes=[1]

After:

ProjectionExec: expr=[x@0 as x, max(...) ROWS BETWEEN 1 PRECEDING AND CURRENT ROW@1 as m]
  SortExec: expr=[max(...) ROWS BETWEEN 1 PRECEDING AND CURRENT ROW@1 ASC NULLS LAST], preserve_partitioning=[false]
    BoundedWindowAggExec: ... mode=[Sorted]
      SortExec: expr=[x@0 DESC], preserve_partitioning=[false]
        DataSourceExec: partitions=1, partition_sizes=[1]

I did not run a benchmark. The added work is one SortExec (a TopK under slice/head) over the window output, only where the old plan skipped it.

Tests

py-ltseq/tests/test_window_order_claims.py:

• test_sort_by_window_sorts: rolling(2|3).max/min/count/sum and cum_max/cum_min/cum_sum over int64, uint64, float64, decimal128, string and date32 (sum numeric only). Each case runs with the table sorted by the argument ascending, descending, or by row id (NULLs interspersed); with no NULLs, one, three, or all NULL; and with and without partition_by. The window values come from an independent Python oracle; then sort("m") and sort("m", desc=True) are checked whole and through slice (TopK) against the oracle's sorted values and row multiset, and against collect().sort(...). Pinned at 2 partitions and batch size 3.
• test_issue_cases_in_every_layout: the cases above in every engine layout (engine_layout) crossed with every input layout (data_layout: one or many batches, Parquet file, row groups, files, CSV file, files). CSV reads unsigned columns back as int64; those cases skip.
• LTSeq.cum_sum (built apart from derive), the wrapping unsigned cum_sum, head/tail, write_parquet/write_csv round trips, and a physical-plan check that a SortExec on the window column sits above the window.
• Rust: no_order_claim::tests::claims_nothing_and_keeps_the_rest checks the wrapper claims nothing where max, min, count and unsigned sum do, and keeps name, signature, nullability, return type and reversal.

With the wrapper temporarily removed from aggregate_to_window, 72 of the 109 selected cases fail: every reported case and the running ones. The min and float cases pass either way.

Run locally:

• pytest py-ltseq/tests/test_window_order_claims.py: 469 passed, 36 skipped
• 18 related files (test_derive*, test_float_min_max, test_group_*, test_nested_derive_proxy, test_no_materialization_rule, test_offset_slice, test_ordered_invariants, test_ranking, test_sort_*, test_when_cumagg, test_window_*): 1489 passed, 36 skipped
• cargo clippy --all-targets -- -D warnings: clean
• cargo test: 190 passed

The full Python suite and the tutorial notebooks are left to CI. No Python API, docstring or stub changed. CHANGELOG.md has a Fixed entry.

Not in this PR

• Upstream: Wrong results: ORDER BY ... NULLS LAST on a running MAX window function puts the NULL row first apache/datafusion#27267 is linked only. Per the issue's plan, no upstream issue or comment was made.
• An unsigned cum_sum/rolling().sum() that overflows wraps silently ([2**64 - 1, 1] sums to 0). This PR only stops the wrong sort elision there. Whether it should raise is a separate decision.

Closes #354

[iceboundrock]

AI Code Review — Round 1

Reviewed HEAD: dfb4b9e1661e0b2e64c725b3f94384210d610a36 against base main (merge base bc2cbb27a2b3772a1e45a3fa5079a74b1d4e737c)

Findings

None.

Spec

The implementation addresses the original ordering criteria through a type-independent mitigation at the shared window-aggregate builder. Passing regressions cover rolling min/max/count/sum, cumulative aggregates, both sort directions, NULL patterns, partitioned windows, slices, execution/input layouts and writers. Independent Python window-value oracles and a physical-plan assertion supplement the collect-and-sort comparison.

Standards

Consistent with the Rust/DataFusion architecture and no-materialization rule. The wrapper implements all 29 current AggregateUDFImpl methods, changing monotonicity while preserving accumulator and typing behavior. The change includes an Unreleased changelog entry and introduces no Python API requiring stub changes. The new Rust module passes rustfmt; repository-wide formatting failures occur in existing code, with corresponding failures confirmed at the merge base. Reviewed sources remain unchanged and the worktree is clean.

Assessment

No actionable correctness, safety or maintainability defect found. Conservatively suppressing ordering claims is an appropriate correctness-first workaround for the pinned DataFusion version. Reversal and beneficial-ordering replacements retain the wrapper, and the affected rolling, cumulative and group-dialect paths use the shared builder. The principal tradeoff is additional sorting rather than changed aggregate semantics.

Observations

The captured CI checks were all pending; their eventual outcomes and the full Python/tutorial suites were not independently verified. Runtime and peak-memory benchmarking from the proposed expanded checklist are absent, as the PR explicitly acknowledges; recovering safe sort-elision optimizations remains phase-two work. Existing unsigned-sum overflow behavior is unchanged.

Verification

• Confirmed the recorded PR head and main base match this round; read the full bound diff, surrounding implementation, repository guidance and supplied GitHub context.
• Rebuilt the bound revision with maturin develop --offline.
• cargo test --offline: 190 passed, after configuring the local Python shared-library path.
• cargo clippy --offline --all-targets -- -D warnings: passed.
• test_window_order_claims.py: 469 passed, 36 skipped.
• Ten related Python files covering float extrema, windows, group windows, derive and materialization guards: 564 passed.
• Additional rebuilt-extension probes: 214 supported type/direction/window combinations, eight downstream explicit-order window comparisons and 12 empty/singleton/width-one cases passed. Unsupported nonnumeric sums were recorded separately.
• Physical EXPLAIN retains the required SortExec above the rolling window; git diff --check passed.
• An additional nine-file regression run exceeded 600 seconds without reporting an assertion failure and is incomplete, not counted as passed. A subsequently isolated offset-slice case passed.
• No pytest process remains; HEAD and tracked files are unchanged.

Summary

No findings requiring another fix round. The focused regression suite, 564 related Python tests, Rust tests and clippy passed. Full CI and broader-suite verification remain incomplete.

Needs another fix round: NO