nielspardon opened a new pull request, #12746: URL: https://github.com/apache/gluten/pull/12746
## What changes are proposed in this pull request? Substrait 0.98 extended `WriteRel`: field 7 became `RelCommon common`, and it gained `CreateMode create_mode = 8` and `substrait.extensions.AdvancedExtension advanced_extension = 9`, while the `OutputMode` value `OUTPUT_MODE_MODIFIED_TUPLES` was renamed to `OUTPUT_MODE_MODIFIED_RECORDS`. Gluten's local `bucket_spec` graft (#8386) sat on field 7 — exactly where 0.98 puts `common`. This migrates Gluten's vendored `WriteRel` to the 0.98 body and relocates the 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**, adopting a "Gluten-local fields start at 1000" convention that keeps grafts clear of the range upstream allocates from, so a future upstream field cannot collide with them again. The enclosing `Rel.write` oneof tag stays at **18**; reconciling the whole `Rel` oneof to upstream's numbers is a separate follow-up. All producer/consumer accessors are name-based (`setBucketSpec`, `has_common`), so the field renumber needs no source change — the one source edit is a guard on the Velox consumer. - **Proto:** vendor the 0.98 `WriteRel` body verbatim (`common = 7`, `create_mode = 8`, `advanced_extension = 9`, the `CreateMode` enum, `OUTPUT_MODE_MODIFIED_RECORDS`) and relocate `BucketSpec bucket_spec` from field 7 to 1000. The nested `BucketSpec` message is unchanged. - **Velox:** reject a `common.emit` mapping in the `WriteRel` handler, which the writer does not apply, mirroring the guard on other rels — `WriteRel` now carries a `common`, so an emit could otherwise be silently ignored. - **Docs:** record the rebase and the 1000+ numbering convention in `SubstraitModifications.md`, noting that `WriteRel.common` still uses Gluten's pre-0.98 `RelCommon` copy and so cannot carry a full 0.98 `common` payload. Gluten's `WriteRel` producer sets only `table_schema`, `named_table` (with `advanced_extension`), `input`, and `bucket_spec`; it never sets `op`, `output`, or `common`. The Velox and ClickHouse consumers read `input`, `table_schema`, `bucket_spec`, and `named_table.advanced_extension` — all by name — and ignore the renamed enum value (it appears only in generated code). So no producer or consumer source needs to change for the proto edits. ## How was this patch tested? - New `WriteRelProtoSuite` pins the producer contract (named table, explicit schema, relocated bucket spec, and the never-set 0.98 fields at their defaults) and asserts the 0.98 field numbers on the descriptor — including `bucket_spec = 1000` — plus the `OUTPUT_MODE_MODIFIED_RECORDS` rename. A renumber or enum-value rename is invisible to a round trip through the shared schema, so the descriptor assertions are the real guard. - Locally verified: `protoc` dup-field check (all imports resolve, no duplicate numbers); `gluten-substrait` builds with the proto codegen regenerated and the new suite passes (`mvn -Pspark-3.5`); scalastyle + spotless clean. - The Velox `WriteRel` consumer body is unchanged and the emit guard reads `common().has_emit()` by name; a full local `libvelox` link is currently blocked by an unrelated Velox-EP skew in `ConfigExtractor.cc`, so the end-to-end bucketed-write path (Velox) and the ClickHouse `WriteRel` parser are exercised by CI, not locally. A green local run does not imply full backend coverage here. ## 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]
