nielspardon opened a new pull request, #12832:
URL: https://github.com/apache/gluten/pull/12832
## What changes are proposed in this pull request?
Substrait 0.98 added `IcebergTable iceberg_table = 9` to the
`ReadRel.read_type` oneof — exactly the field number Gluten's local
`stream_kafka` graft occupies. This migrates Gluten's vendored
`ReadRel.read_type` to the 0.98 layout by vendoring the official
`iceberg_table` field and `IcebergTable` message verbatim and relocating the
`stream_kafka` graft off the collision, as one step of the Substrait `v0.23.0`
→ `0.98.0` proto rebase (#12597).
The graft moves to field **1000**, following the "Gluten-local fields start
at 1000" convention adopted for `WriteRel.bucket_spec` (#12746), so a future
upstream field cannot collide with it again. It stays inside the `read_type`
oneof, and the enclosing `Rel.read` oneof tag is unchanged. All accessors are
name-based (`setStreamKafka` / `hasStreamKafka`), so the renumber needs no
source change; the new `iceberg_table` field is unreferenced by any producer or
consumer, so no producer or consumer source is touched.
- **Proto:** add `IcebergTable iceberg_table = 9` to the `read_type` oneof
and vendor the 0.98 `IcebergTable` message verbatim (nested `MetadataFileRead`
with `metadata_uri` and a `snapshot_id` / `snapshot_timestamp` oneof); relocate
`bool stream_kafka` from field 9 to 1000.
`ReadRel` is the largest of the vendored messages, so this is the first of
three independent ReadRel slices — this one (`read_type`), the text-options
redesign (`TextReadOptions` → `DelimiterSeparatedTextReadOptions`), and
`VirtualTable.values` → `expressions` — each landing as its own PR. They touch
disjoint proto regions and disjoint consumers.
## How was this patch tested?
- Locally verified: `protoc` dup-field check (all imports resolve, no
duplicate field numbers, the new `IcebergTable` / `MetadataFileRead` resolve);
`gluten-substrait` builds with the proto codegen regenerated (`mvn -Pspark-3.5
-pl gluten-substrait -am clean install`), scalastyle clean.
- Native: the regenerated `algebra.pb.cc` and all five ReadRel-relevant
Velox translation units (`SubstraitToVeloxPlan`,
`SubstraitToVeloxPlanValidator`, `VeloxToSubstraitPlan`,
`VeloxToSubstraitExpr`, `SubstraitToVeloxExpr`) compile against the new proto.
A full local `libvelox` link is currently blocked by an unrelated Velox-EP skew
(`WholeStageResultIterator.cc` references
`QueryConfig::kBypassHashProbeBloomFilterMin{Rows,Pct}`, absent from the built
EP), so the end-to-end Kafka/Iceberg read paths and the ClickHouse read parser
are exercised by CI, not locally. A green local run does not imply full backend
coverage here.
- No new test: this slice is a purely additive field plus a name-based
renumber with no source-side behavior change, so there is no producer/consumer
contract to pin beyond the descriptor, which `protoc` already enforces. The
text-options and `VirtualTable` slices — which do change accessor semantics —
carry their own round-trip tests.
## 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]
