[GLUTEN-12597][CORE] Migrate ReadRel read_type to Substrait 0.98 (add iceberg_table, relocate stream_kafka) - #12832
Open
nielspardon wants to merge 1 commit into
Conversation
|
Run Gluten Clickhouse CI on x86 |
This was referenced Aug 20, 2026
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 20, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. Tag 1000 is marked `reserved` so it is not reused. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 20, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. Tag 1000 is marked `reserved` so it is not reused. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 20, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. Tag 1000 is marked `reserved` so it is not reused. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 20, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. Tag 1000 is marked `reserved` so it is not reused. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
nielspardon
marked this pull request as ready for review
August 20, 2026 16:58
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 20, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. Tag 1000 is marked `reserved` so it is not reused. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Gluten’s vendored Substrait ReadRel.read_type layout to align with Substrait 0.98 by introducing the official iceberg_table variant at tag 9 and relocating Gluten’s stream_kafka graft to a non-colliding tag (1000). This is part of the broader Substrait proto rebase work tracked under GLUTEN-12597.
Changes:
- Add
IcebergTable iceberg_table = 9toReadRel.read_typeand vendor the nestedReadRel.IcebergTablemessage structure. - Relocate
bool stream_kafkafrom field 9 to field 1000 to avoid collision with upstream’s newiceberg_table.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 21, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. read_type now matches upstream exactly, which never allocated field 1000, so the retired tag is not reserved. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
… iceberg_table, relocate stream_kafka) Substrait 0.98 added `IcebergTable iceberg_table = 9` to the `ReadRel.read_type` oneof, exactly where Gluten's local `stream_kafka` graft sat. This vendors the 0.98 `iceberg_table` field and `IcebergTable` message verbatim and relocates the `stream_kafka` graft off the collision, as one step of the Substrait v0.23.0 -> 0.98.0 proto rebase (apache#12597). The graft moves to field 1000, following the "Gluten-local fields start at 1000" convention established for WriteRel's bucket_spec (apache#12746); it stays inside the read_type oneof. The enclosing Rel.read oneof tag is unchanged. All accessors are name-based (setStreamKafka/hasStreamKafka), so the field renumber needs no source change, and the new iceberg_table field is unreferenced by any producer or consumer, so no source is touched. A descriptor-level `ReadRelProtoSuite` pins the `read_type` field numbers (and the vendored `IcebergTable` layout), since a renumber round-trips cleanly through the shared schema and would otherwise be invisible to tests. This is the first of three ReadRel slices (read_type / text options / VirtualTable); the other two are separate follow-ups. Part of apache#12597 Generated-by: Claude Code (Claude Opus 4.8)
nielspardon
force-pushed
the
feat/substrait-0.98-readrel-read-type
branch
from
August 21, 2026 05:55
890970f to
17936ca
Compare
|
Run Gluten Clickhouse CI on x86 |
nielspardon
added a commit
to nielspardon/gluten
that referenced
this pull request
Aug 21, 2026
…Substrait 0.98) Part of apache#12597. Stacked on apache#12832 (the iceberg_table half of the ReadRel read_type migration). Makes ReadRel.read_type verbatim upstream Substrait 0.98 by removing Gluten's last graft on it -- the `bool stream_kafka = 1000` discriminator and the nested `ReadRel.StreamKafka` message. read_type now matches upstream exactly, which never allocated field 1000, so the retired tag is not reserved. Gluten's Kafka streaming read is remodeled onto the official `extension_table = 8` path, the mechanism MergeTree and Range already use. The StreamKafka payload moves to a new Gluten-owned `kafka.proto` (`package gluten`, `org.apache.gluten.proto`), packed into a `google.protobuf.Any` and carried in `ReadRel.ExtensionTable.detail`. The native consumer discriminates a Kafka read by the detail's type_url (`detail().Is<gluten::StreamKafka>()`) -- mirroring the already-merged Velox Iceberg idiom (`enhancement().Is<gluten::IcebergReadExtension>()`). This Any-in-an-official-extension-field pattern is preferred over grafting new fields; `WriteRel.bucket_spec` (field 1000) remains the one legacy graft. Gluten plans are transient and the JAR + native library are generated from one proto source and ship together, so there is no wire-compatibility constraint. Two side effects: the Kafka split-info payload changes from a bare StreamKafka to a ReadRel.ExtensionTable wrapping it, and read_type -- previously set on every ReadRel by the unconditional stream_kafka flag -- is now left unset for non-Kafka scans (no native code reads read_type_case). A JAR and native library must therefore be rebuilt together. ClickHouse-only; Kafka has no Velox path. Generated-by: Claude Code (Claude Opus 4.8)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
Substrait 0.98 added
IcebergTable iceberg_table = 9to theReadRel.read_typeoneof — exactly the field number Gluten's localstream_kafkagraft occupies. This migrates Gluten's vendoredReadRel.read_typeto the 0.98 layout by vendoring the officialiceberg_tablefield andIcebergTablemessage verbatim and relocating thestream_kafkagraft off the collision, as one step of the Substraitv0.23.0→0.98.0proto 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 theread_typeoneof, and the enclosingRel.readoneof tag is unchanged. All accessors are name-based (setStreamKafka/hasStreamKafka), so the renumber needs no source change; the newiceberg_tablefield is unreferenced by any producer or consumer, so no producer or consumer source is touched.IcebergTable iceberg_table = 9to theread_typeoneof and vendor the 0.98IcebergTablemessage verbatim (nestedMetadataFileReadwithmetadata_uriand asnapshot_id/snapshot_timestamponeof); relocatebool stream_kafkafrom field 9 to 1000.ReadRelis 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), andVirtualTable.values→expressions— each landing as its own PR. They touch disjoint proto regions and disjoint consumers.How was this patch tested?
protocdup-field check (all imports resolve, no duplicate field numbers, the newIcebergTable/MetadataFileReadresolve);gluten-substraitbuilds with the proto codegen regenerated (mvn -Pspark-3.5 -pl gluten-substrait -am clean install), scalastyle clean.algebra.pb.ccand all five ReadRel-relevant Velox translation units (SubstraitToVeloxPlan,SubstraitToVeloxPlanValidator,VeloxToSubstraitPlan,VeloxToSubstraitExpr,SubstraitToVeloxExpr) compile against the new proto. A full locallibveloxlink is currently blocked by an unrelated Velox-EP skew (WholeStageResultIterator.ccreferencesQueryConfig::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.protocalready enforces. The text-options andVirtualTableslices — 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