You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "nielspardon (via GitHub)" <gi...@apache.org> on 2026/08/21 13:53:12 UTC
[PR] [GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions) [gluten]
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions) [gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan merged PR #12849:
URL: https://github.com/apache/gluten/pull/12849
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12849:
URL: https://github.com/apache/gluten/pull/12849#issuecomment-5370697344
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org