nielspardon opened a new pull request, #12727:
URL: https://github.com/apache/gluten/pull/12727
## What changes were proposed in this pull request?
Part of #12597 (rebase the vendored Substrait proto from v0.23.0 to 0.98.0).
This PR migrates the **windowing** message family to official Substrait 0.98.0,
moving the JVM producer and both native consumers in lockstep. Because Gluten's
plans are transient and producer + consumers regenerate from one proto source
and ship together, there is no wire-compatibility constraint — only round-trip
correctness matters.
Three coupled proto changes:
- **`WindowRel` → `ConsistentPartitionWindowRel`.** The `Measure {
Expression.WindowFunction measure = 1 }` wrapper is repacked into a bare
`WindowRelFunction`. Gluten already hoists `partition_expressions`/`sorts` to
the rel level and never populated per-function partitions/sorts — exactly the
`ConsistentPartitionWindowRel` invariant — so this is a field-subset repack,
not a semantic change.
- **`WindowType` → `BoundsType` (silent value remap).** Fork: top-level
`enum WindowType { ROWS = 0; RANGE = 1; }` at
`Expression.WindowFunction.window_type = 13`. 0.98: nested `enum BoundsType {
BOUNDS_TYPE_UNSPECIFIED = 0; BOUNDS_TYPE_ROWS = 1; BOUNDS_TYPE_RANGE = 2; }` at
`bounds_type = 12`. Every value shifts by one; the producer maps via the named
constants (not raw ordinals).
- **`Bound` unbounded collapse.** `unbounded_preceding` /
`unbounded_following` collapse into a single `unbounded`; direction is inferred
from position (a `lower_bound` = start of partition, an `upper_bound` = end).
Two Gluten grafts are kept: the RANGE `ref` bound (`oneof { int64 offset =
1; Expression ref = 2; }`) consumed by Velox `PreComputeRangeFrameBound`, and
per-function `column_name` grafted onto `WindowRelFunction` (0.98 has none).
The Rel oneof keeps `window = 16` (only the field's type changes); whole-oneof
renumbering is deferred to the reconciliation increment.
Files: the vendored `algebra.proto`; `WindowFunctionNode` / `WindowRelNode`
(producer); Velox `SubstraitToVeloxPlan` + `SubstraitToVeloxPlanValidator`;
ClickHouse `WindowRelParser` + `AggregateFunctionParser`; the
`tpcds-q47-wholestage-9.json` static test plan; and a new `WindowRelProtoSuite`.
## How was this patch tested?
- **New unit test** `WindowRelProtoSuite` (gluten-substrait) — a producer
round-trip that pins the two silent risks a plain compile can't catch: it
asserts the frame-type enum value remap (`BOUNDS_TYPE_ROWS.getNumber == 1`,
`BOUNDS_TYPE_RANGE.getNumber == 2`) and the `unbounded` collapse at both the
lower and upper bound positions, plus the `column_name` graft.
- **Existing coverage exercised by the migrated paths** — the ClickHouse
`GlutenClickHouseTPCDSMetricsSuite` (via the updated
`tpcds-q47-wholestage-9.json` static plan, whose window operator metrics would
break on a wrong frame/direction) and the Velox / gluten-ut window suites
(`VeloxWindowExpressionSuite`, `GlutenDataFrameWindowFramesSuite`) which run
unbounded/ROWS/RANGE frames end-to-end.
- **Local build verification** — the vendored proto compiles with no
duplicate field numbers and matches upstream 0.98 field-for-field;
`gluten-substrait` builds and `WindowRelProtoSuite` passes; the Velox native
backend compiles against `bounds_type`/`Unbounded`/`WindowRelFunction`. The
ClickHouse native backend is covered by CI.
Generated-by: Claude Code (Claude Opus 4.8)
🤖 Generated with AI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
