[GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions)

[nielspardon]

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

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

Pull request overview

This PR updates Gluten’s vendored Substrait ReadRel.VirtualTable to match Substrait 0.98’s official shape by migrating from values: repeated Expression.Literal.Struct (tag 1) to expressions: repeated Expression.Nested.Struct (tag 2), and updates the Velox-side producer/consumer plus fixtures to keep VirtualTable semantics literal-only and behavior-preserving.

Changes:

• Update the vendored proto to reserve tag/name values and introduce expressions on tag 2 as Expression.Nested.Struct.
• Update Velox producer/consumer paths to write/read VirtualTable row groups via Expression-wrapped literals.
• Add a JVM descriptor-level regression test to pin wire tags and message element types post-rebase, and update the JSON fixture accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
gluten-substrait/src/test/scala/org/apache/gluten/substrait/rel/VirtualTableProtoSuite.scala	Adds descriptor-based assertions to pin VirtualTable/read_type wire tags and nested element types.
gluten-substrait/src/main/resources/substrait/proto/substrait/algebra.proto	Migrates ReadRel.VirtualTable from values (tag 1) to expressions (tag 2) and reserves the old field.
cpp/velox/tests/data/substrait_virtualTable.json	Updates the test plan JSON to match the new expressions container and Expression.literal wrapping.
cpp/velox/substrait/VeloxToSubstraitPlan.cc	Producer now emits virtual_table.expressions and appends per-cell literals wrapped as Expression.
cpp/velox/substrait/VeloxToSubstraitExpr.h	Updates the VirtualTable literal-append API to accept Expression_Nested_Struct.
cpp/velox/substrait/VeloxToSubstraitExpr.cc	Implements the new literal-wrapping append behavior for VirtualTable emission.
cpp/velox/substrait/SubstraitToVeloxPlan.cc	Consumer now reads virtual_table.expressions, validates/unwraps Expression.literal, and derives batch size per row group.

Suppressed comments (1)

cpp/velox/substrait/VeloxToSubstraitExpr.cc:604

• toSubstraitLiteral(arena, vectorValue, litValue) allocates and returns substraitField but the scalar-vector path never populates it (conversion appends into litValue via add_fields()->mutable_literal()), so this returns an empty Expression_Literal. Also, the scalar path dereferences litValue inside convertVectorValue without guarding against nullptr, but toSubstraitExpr(constExpr, litValue = nullptr) can reach this overload when constExpr->hasValueVector() is true.

Consider removing the unused substraitField allocation and returning the last appended literal from litValue (or explicitly requiring litValue != nullptr in the scalar-vector path and handling the nullptr case safely).

      google::protobuf::Arena::CreateMessage<::substrait::Expression_Literal>(&arena);
  if (vectorValue->isScalar()) {
    VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH(
        convertVectorValue, vectorValue->type()->kind(), arena, vectorValue, litValue, substraitField);
    return *substraitField;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zhouyuan]

Thanks.