Skip to content

Commit cae12ad

Browse files
rustyconoverclaude
andcommitted
docs: a window aggregate's rid is chunk-relative, not a partition row
The per-row window callback's `rid` was documented as the output row's index within the partition (AggregateWindowRequest, AggregateFunction.window(), AggregateClient.window()). It is not: DuckDB's WindowCustomAggregator evaluates one output chunk at a time and calls the callback with `i`, the row's index within that chunk (0 <= i < STANDARD_VECTOR_SIZE); the partition row is `row_idx + i`, which the per-row callback never receives. The extension forwards DuckDB's value untouched (it is also the index it copies the result into), so a worker that used `rid` to index the partition read the wrong rows past the first chunk. The frames were always partition-relative, and they are what a worker must use. The batched path is unchanged and correctly documented: its `row_idx` is the partition row of the first output row, and the default window_batch() passes partition rows to window() — so `rid` now says it identifies the row and must not be used to index the partition. Docstrings are outside the protocol hash, so no port's reflection changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 04a6147 commit cae12ad

3 files changed

Lines changed: 16 additions & 3 deletions

File tree

‎vgi/aggregate_function.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,14 @@ def window(
518518
"""Compute the aggregate value for one output row.
519519
520520
Args:
521-
rid: Partition-local row index being filled.
521+
rid: Identifies the output row being filled; do not use it to
522+
index the partition. Over the per-row ``aggregate_window``
523+
RPC it is DuckDB's own ``rid``: the row's index within the
524+
current output chunk (``0 <= rid < 2048``), not its row in
525+
the partition. When the default :meth:`window_batch` calls
526+
this, it passes the partition row index instead. Either way
527+
``subframes`` are partition-relative: use them to find the
528+
rows to aggregate.
522529
subframes: Frame ranges ``[(begin, end), ...]`` — 1 for the default
523530
frame, 3 when ``EXCLUDE`` produces multiple subframes.
524531
partition: The cached partition data.

‎vgi/client/aggregate.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,10 @@ def window(self, *, partition_id: int, rid: int, frames: Frames) -> pa.RecordBat
371371
372372
Args:
373373
partition_id: The partition previously shipped by :meth:`window_init`.
374-
rid: Row index within the partition of the output row being computed.
374+
rid: Identifies the output row. The DuckDB extension sends DuckDB's
375+
own per-row ``rid``, the row's index within the current output
376+
chunk rather than its row in the partition; ``frames`` carry the
377+
partition-relative bounds, and a worker must use those.
375378
frames: The row's subframes as ``(begin, end)`` half-open offsets
376379
into the partition. One entry normally; two or three for
377380
``EXCLUDE TIES`` / ``EXCLUDE GROUP``.

‎vgi/protocol.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2887,7 +2887,10 @@ class AggregateWindowRequest(ArrowSerializableDataclass):
28872887
execution_id: Identifier for one query execution, stable across the
28882888
coordinator and any secondary workers; keys worker-owned state.
28892889
partition_id: Identifier of the window partition this call addresses.
2890-
rid: Row index within the partition for the output row being computed.
2890+
rid: Which output row this call fills, as DuckDB's per-row window callback
2891+
reports it: the row's index within the current output chunk
2892+
(``0 <= rid < STANDARD_VECTOR_SIZE``), NOT its row in the partition.
2893+
The frames below are partition-relative; use them to find the rows.
28912894
frame_starts: Subframe start offsets (inclusive) within the partition, one
28922895
per subframe.
28932896
frame_ends: Subframe end offsets (exclusive) within the partition, parallel

0 commit comments

Comments
 (0)