fix: correct TIME_WINDOW slot layout and prune unused aggregates

[ck89119]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #25699

Part of #25612. Follows #25613, which deferred TIME_WINDOW because its
row-count and special-slot contracts needed an execution-layer design.

What this PR does / why we need it:

The issue describes a missed optimization; it is also a wrong-answer bug

TIME_WINDOW's output batch layout was derived independently in two places:

• remapAllColRefs addressed slots with a counter that only advanced for
  projected entries;
• constructTimeWindow / calRes built the batch from the unpruned
  AggList.

As soon as an outer query discarded any window output, the two drifted and the
projection read a neighbouring column. Reproduced on a live instance
(interval(ts, 5, second) over two windows; baseline _wstart, max(v), min(v)
= (00:00:00, 20, 10), (00:00:05, 40, 30)):

query	before	after
select c from (select _wstart a, max(v) b, min(v) c from t interval(ts,5,second)) x	20, 40 — silently returns max(v)	10, 30
select a from (…same…) x	ERROR 1105: unsupported type INT for MYSQL_TYPE_TIMESTAMP	two timestamps
select d from (select _wstart a, _wend d, max(v) b …) x	ERROR 1064: Column remapping failed	two timestamps
select c from (… fill(prev)) x	panic: index out of range [1] with length 1	10, 30

Sliding windows are affected identically (select c returned 40, 40 for
min(v)). Causes, respectively: the compacted counter; the same counter
applied to a boundary slot; a break inside the switch that skipped the
_wend branch along with _wstart; and constructFill sizing ColLen from a
FILL.AggList that was never pruned alongside the window's.

Fix

BuildTimeWindowLayout becomes the single source of truth for the layout
([aggregates…, _wstart?, _wend?, partition keys…]), consumed by both the
planner and the compiler, so the two cannot drift again. Pruning then follows
from it:

• only retained aggregates take a reference on their inputs, so the child AGG
  drops those inputs too — the "prune aggregates and their inputs" the issue
  asks for;
• FILL prunes its columns and FillVal in step with the window. This must be
  decided before FillVal's refcounts, because fill(linear) builds
  FillVal out of references to those very aggregates and would otherwise keep
  every one alive;
• a row carrier is retained when every value is discarded — calRes sizes its
  batch off Vecs[0], and the window count is still the row count. A boundary
  is preferred, since it runs no aggregate;
• every _wstart reference maps to the one boundary slot, so repeats cannot
  drop a projection.

select c from (… max(v), min(v) …) x now compacts AggList from 3 entries to
1, and the child AGG drops max(v) and its input column.

GROUP BY + time window: partitioned execution

select _wstart, other, max(v) from t group by other interval(ts,5,second)
previously panicked. The window was never partitioned at all — the child AGG
grouped by (other, trunc_ts) while the window sorted by trunc_ts alone,
interleaving groups into one global stream, and the operator never emitted the
group column. This PR carries those keys as TimeWindowPartitionBy, orders by
them ahead of the timestamp, and restarts the window state at each boundary so
each group gets its own windows. Partition keys are never pruned (dropping one
would merge groups) and land in the tail slots, keeping the aggregates a prefix
for FILL.

Two further latent bugs found and fixed here

• node.WEnd's column reference was never remapped. The operator evaluates
  it against a batch holding only the timestamp, so it worked only while the
  window key happened to be group column 0; group by id moves it to 1 and it
  reads out of range. resetTimeWindowTsColRef makes the contract explicit.
• Every flush after the first orphaned its aggregate vectors. Only the
  Flush() results are owned by that batch (boundaries belong to their
  expression executors, partition keys to ctr.partOut). Previously this needed
  more than 8192 windows; partitioning makes flushing routine.

Also removes fill.AggIds, which was assigned by constructFill and never read.

Review round: partition-aware FILL and further fixes

All seven review findings were verified and fixed:

• fill(prev/next/linear) leaked values across partitions — prev filled a
  group's leading NULL with the previous group's value; next dropped whole
  batches. The partition key positions now travel to the FILL operator
  (time_window_partition_col_pos), and all three modes treat a key change as
  a hard boundary. NEXT/LINEAR are rewritten to materialize their input first:
  the old per-column streaming state machine returned early whenever a batch
  contained no NULL and silently discarded every batch after it (a
  pre-existing bug that partitioning made routine).
• A window spanning two batches was dropped — fillRows reset
  withoutFill per batch instead of per window, so a window whose rows came
  in one batch and whose closing row came in the next was emitted as empty.
  Found by the new batch-split invariance test.
• remakeAggs leaked one set of aggregate executors per partition — the old
  executor still owns its state after Flush; it is now freed first.
• Side-effecting fill values were pruned — fill(value, sleep(1)) on a
  discarded column now pins the matching window slot (verified end-to-end:
  1.02s with sleep retained vs 0.017s with a plain value pruned).
• TimeWindowPartitionBy joins the planner infrastructure —
  DeepCopyNode (whose TIME_WINDOW/FILL fields were never copied at all),
  the plan visitor, and replaceColumnsForNode, each with tests.
• make static-check passes — the two prealloc findings are fixed.
• The golden file's pre-existing expectations are restored — genrs had
  rewritten them into the new recorder format; only this PR's cases are
  appended now (verified: zero deleted lines, suite 61/61).

Partitioned fill(prev/next/linear) was re-verified against per-partition
references over 9200 rows / 4 partitions: identical (576/576/576/863 rows
across the four specs).

Tests

• Differential (the strongest signal): 9200 rows / 4 partitions / irregular
  gaps, comparing group by p <spec> against per-partition where p=N <spec>.
  Identical across interval(1 minute) (1723 rows), interval(3 minute) fill(prev) (576), sliding(2 minute) (863), sliding(20 minute) (88), and
  interval(30s) sliding(10s) (8794 rows, spanning batch boundaries). This
  caught two bugs in the implementation: partitions losing their trailing
  windows, and an out-of-range read that broke all non-sliding time-window
  queries.
• UT: BuildTimeWindowLayout (slot/boundary/partition/repeat/empty cases and
  a planner-vs-compiler consistency check); plan-shape tests for each pruning
  shape above; operator tests for partition reset, trailing windows,
  single-partition equivalence with the unpartitioned operator, NULL keys
  grouping together, and the pass-through path — all asserting CurrNB() == 0,
  which is what surfaced the flush leak.
• BVT: new cases in optimizer/column_pruning covering every query in the
  table above plus sliding, all three fill modes, empty input, row carriers,
  and partitioned windows.

Coverage of changed code: time_window_layout.go 100%; the TIME_WINDOW/FILL
remap branches 92.3%; colexec/timewin 35.4% → 72.9%.

Suites: time_window 31/31, optimizer 1078/1078, window 1543/1559 — the 4
failures are external01 external-table reads returning 0 rows, verified to
fail identically on an unmodified baseline build (select count(*) from external01 has no TIME_WINDOW node at all).

Review round 2: TimeWin reusable after Reset (195c1a0)

Reproduced the reported crash: a partitioned sliding TimeWin run to ExecStop,
then Reset + Prepare with a fresh child, panicked in
nextWindow -> AggFuncExec.GroupGrow on the second run.

• resetParam now rewinds every per-generation field: the buffer cursor
  i (so tsVec/aggVec/partVec reuse restarts at index 0 instead of
  appending after stale entries), the window cursors and bounds
  (curVecIdx/curRowIdx, preVecIdx/preRowIdx, left/right,
  nextLeft/nextRight), last/lastVal, withoutFill, and all partition
  break state (partEnd, breakVecIdx/breakRowIdx, partLast*). With the
  cursors at zero, receive routes into firstWindow again.
• Reset now establishes the generation boundary for owned memory: it frees
  the aggregate prefix of the last flushed batch (the only part that batch
  owns — boundaries belong to their expression executors, partition keys to
  partOut), drops the batch reference, and discards the flushed aggregate
  executors (they cannot be rewound after Flush, the same property
  remakeAggs documents). Prepare rebuilds the aggregates behind a gate
  separate from the expression executors.
• freeVector additionally releases the _wstart/_wend staging vectors,
  which the Reset path would otherwise retain.
• Regression tests (TestTimeWinReuseAfterReset,
  TestTimeWinIntervalPathReuseAfterReset): exhaust the operator,
  Reset + Prepare, exhaust it again on fresh input, compare the two
  complete outputs, and assert CurrNB() == 0 after Free — for the
  partitioned sliding, plain sliding, and interval pass-through paths. The
  first generation reads two child batches and the second reads one, so the
  index-0 buffer-reuse claim is exercised, not just asserted. Reverting the
  fix makes the new test fail with the exact reported stack (panic in
  nextWindow).

Verification: colexec/timewin UT green incl. -race, coverage 77.5%
(Reset/resetParam 100%); BVT time_window 31/31 and
optimizer/column_pruning 61/61; prepared-statement re-execution of a
partitioned sliding window returns identical results across executions.

Review round 3: fill(next/linear) incremental again, not fully materialized (957441a)

The round-2 rewrite gathered the entire child stream before emitting the first
NEXT/LINEAR result, which turned even the no-NULL path into O(total rows)
retained memory and full-input first-row latency (an outer LIMIT 1 could not
stop the child; a wide time range could OOM although no look-ahead was needed).

Replaced with a batch-level incremental engine:

• bats is now a pending FIFO keyed by an absolute sequence number (baseSeq),
  so a captured coordinate stays valid after the FIFO pops its head. A batch is
  emitted as soon as every fill column has resolved its rows; the child is
  pulled again only when nothing is emittable. toFree releases the batch handed
  to the parent on the previous Call.
• NEXT keeps each column's unfilled NULL run and back-fills it on the next
  non-NULL value; LINEAR keeps the last non-NULL as a left endpoint and pins its
  batch until superseded, interpolating a pending run against it. Partition
  boundaries drop the pending candidates (they stay NULL), preserving the
  existing cross-partition semantics. A no-NULL NEXT stream buffers nothing; a
  no-NULL LINEAR stream buffers at most one batch. A genuinely long unresolved
  suffix still buffers until the next value, which is inherent to the operator's
  semantics (the original streamed the same way); no disk spill.
• Deterministic regressions with a call-counting child: NEXT emits its first
  batch after exactly one child pull and LINEAR after one look-ahead, both well
  before child EOF, with len(ctr.bats) <= 1 throughout; plus long cross-batch
  gaps, EOF tails, independent multi-column gaps, and cross-batch LINEAR
  interpolation.

Verification: colexec/fill UT green incl. -race, coverage 85.4%,
golangci-lint 0 issues; BVT time_window 31/31 and optimizer/column_pruning
61/61; partitioned prev/next/linear/value verified end to end with no
cross-partition bleed and LIMIT 1 returning immediately.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

Blocking: the new partitioned sliding TIME_WINDOW path is not reusable after Reset.

I reproduced this at the current head (0949294) by running a partitioned sliding TimeWin to ExecStop, calling Reset + Prepare, attaching a fresh child with the same rows, and running it again. The second execution reliably panics in aggState.grow from container.nextWindow -> AggFuncExec.GroupGrow.

resetParam only resets status/end/group/window slices and three partition fields. It leaves generation-local state such as last, i, the current/previous/break cursors, partEnd, and the flushed aggregate executors intact. After the first execution, receive therefore resumes through nextWindow with an aggregate whose flushed result vector is no longer usable, causing the nil dereference. Merely clearing one cursor would still leave stale rows/state or unbounded retained buffers across reuse.

Please make Reset establish a complete generation boundary: reset every control/cursor flag, reset or recreate aggregate state after Flush, and make buffered vector reuse start at index 0. Add a regression test that (1) exhausts the partitioned sliding operator, (2) calls Reset and Prepare, (3) exhausts it again with fresh input, (4) compares both complete outputs, and (5) verifies the mpool returns to zero after Free. The current TestTimeWin calls Exec only once on the interval path, so it cannot cover this failure.

I also checked planner/compiler layout alignment, partition-boundary NULL semantics, cross-batch PREV/NEXT/LINEAR behavior, ownership cleanup, and the targeted package tests. The existing tests pass, but the lifecycle reproducer fails with SIGSEGV, so this needs to be fixed before merge.

[gouhongshen]

Codex automated review

A valid TIME_WINDOW query using GROUP BY NULL can panic during operator preparation because the new partition-key setup does not support T_any.

P1 - Avoid panicking on a T_any partition key from GROUP BY NULL (pkg/sql/colexec/timewin/timewin.go:83)

The binder accepts GROUP BY NULL, whose literal type is T_any (pkg/sql/plan/make.go:134-145). appendTimeWindowNode now converts every non-time-window group into TimeWindowPartitionBy (pkg/sql/plan/query_builder.go:5453-5474), so a query such as SELECT _wstart, count(*) FROM t GROUP BY NULL INTERVAL(ts, 5, SECOND) reaches this call. vector.GetConstSetFunction has no T_any case and panics in its default branch (pkg/container/vector/vector.go:2548). Before this change, TIME_WINDOW preparation did not construct partition broadcast functions, so this valid grouping shape did not hit the panic. Constant/null partition keys should be omitted or handled before constructing the set function.

[gouhongshen]

Codex automated review

The new TIME_WINDOW pruning exposes a sliding execution path with unbounded memory retention.

P1 - Clear unused sliding boundary buffers after pruning (pkg/sql/plan/query_builder.go:1511)

For a query such as select c from (select _wstart as a, max(v) as b, min(v) as c from t interval(ts, 10, second) sliding(5, second)) x, this liveness check removes the boundary entries, leaving WStart and WEnd false. The sliding executor still appends every flushed window's bounds to ctr.wStart and ctr.wEnd, but calRes returns from its no-boundary branch without clearing them; only the boundary branch and query Reset clear these slices. Consequently, repeated flushes retain all prior window bounds and can OOM on large inputs. Clear the unused slices after each flush and add a many-window regression test.

[XuPeng-SH]

Blocking: the new bounded-memory spill path still loses the streaming and early-stop guarantee after the threshold is crossed.

At head 6123607, driveFill enters collectSpill whenever the spill is not ready, and collectSpill can only return after the child reaches EOF. It does not stop when a NEXT endpoint, LINEAR right endpoint, or partition boundary has already resolved the earliest spilled suffix.

I reproduced this deterministically with one-row NEXT batches:

10, NULL, NULL, 40, 50, 60

and SpillThreshold = 2 rows. The first Fill.Call returns 10 after one child pull. During the second Fill.Call, row 40 closes the pending NULL gap on the fourth child pull, so the next output is fully determined and an outer LIMIT could stop there. The implementation instead performs seven child pulls, consuming 50, 60, and EOF before returning. The unrelated tail can be arbitrarily large, so this creates unbounded extra scan work, spill I/O, and first-result latency after a large gap. LINEAR follows the same EOF-only collection path.

Please make spill replay segment-based: finalize and replay the earliest segment once every unresolved column affecting that segment has reached its endpoint, partition boundary, or EOF, then resume the child after replay. Add deterministic child-call-count regressions for NEXT and LINEAR, including multiple fill columns and a partition boundary.

The correctness, Reset/reuse, T_any partition handling, pruned-boundary cleanup, and resource ownership checks otherwise look closed on this head. Local modified-package tests passed; fill/timewin race tests passed with count 10; and a temporary merge with current main passed the modified-package tests.