nielspardon opened a new pull request, #12849:
URL: https://github.com/apache/gluten/pull/12849
## What changes are proposed in this pull request?
Part of #12597 (the Substrait v0.23.0 -> 0.98.0 vendored-proto rebase). This
slice migrates `ReadRel.VirtualTable` onto its official 0.98 shape.
Substrait 0.98 reshaped `VirtualTable` from a list of literal structs to a
list of expression structs. This is a container-type change, not a rename: the
element type goes from `Expression.Literal.Struct` (whose fields are
`Expression.Literal`) to `Expression.Nested.Struct` (whose fields are
`Expression`), so each cell now travels as an `Expression` wrapping a `Literal`
rather than as a bare `Literal`.
```
- // A table composed of literals.
message VirtualTable {
- repeated Expression.Literal.Struct values = 1;
+ reserved 1;
+ reserved "values";
+
+ repeated Expression.Nested.Struct expressions = 2;
}
```
The vendored body is now byte-identical to upstream 0.98, so this region of
`ReadRel` becomes verbatim-upstream (no graft). The migration is deliberately
container-only and semantics-preserving: Gluten only ever uses literal-valued
virtual tables (the consumer rejects non-constant expressions and the producer
only emits literals), so each `Nested.Struct` field is kept as an `Expression`
that wraps a `Literal`, reusing all existing per-type literal machinery
unchanged.
- Forward consumer (`SubstraitToVeloxPlan.cc`): reads `expressions()`
instead of `values()`, derives the column-major batch size per struct, and
unwraps `expr.literal()` behind an explicit `has_literal()` check before the
existing literal conversion path.
- Reverse producer (`VeloxToSubstraitPlan.cc` +
`VeloxToSubstraitExpr.{h,cc}`): builds each row into an
`Expression.Nested.Struct`, wrapping every emitted literal in
`add_fields()->mutable_literal()`.
`VirtualTable` is Velox-only: no JVM producer emits it and no ClickHouse
consumer reads it, so there are no JVM or ClickHouse source changes. Because
producer and consumer share one generated schema, a renumber or rename cannot
be caught by a round trip; the new `VirtualTableProtoSuite` pins the wire tags
at the descriptor level (`expressions` on tag 2 holding
`Expression.Nested.Struct`, tag 1 / `values` gone, `virtual_table` still on
`read_type` tag 5).
## How was this patch tested?
Native (Velox) build: full clean rebuild links `libvelox.dylib` cleanly with
the reworked producer and consumer.
`VirtualTableProtoSuite` (new, JVM): passes (3/3), pinning the post-rebase
descriptor tags.
The existing `VeloxSubstraitRoundTripTest` (`values` / `null`) and
`Substrait2VeloxValuesNodeConversionTest` exercise the renamed `expressions`
field end to end in Velox CI (round trip through reverse producer -> forward
consumer, plus the forward-only JSON fixture that this PR updates); the `null`
case covers the zero-column batch-size branch.
## Was this patch authored or co-authored using generative AI tooling?
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]
