[GLUTEN-12597][CORE] Migrate FetchRel offset/count to expressions (Substrait 0.98) - #12704
Open
nielspardon wants to merge 1 commit into
Open
[GLUTEN-12597][CORE] Migrate FetchRel offset/count to expressions (Substrait 0.98)#12704nielspardon wants to merge 1 commit into
nielspardon wants to merge 1 commit into
Conversation
…bstrait 0.98) Substrait 0.98 replaced FetchRel's scalar `int64 offset = 3` / `int64 count = 4` with `Expression offset_expr = 5` / `Expression count_expr = 6`. This migrates the vendored proto, the JVM producer, and both native consumers in lockstep. Gluten's only FetchRel producer (LimitExecTransformer) always supplies literal Long offset/count from Spark's Limit/Offset plans, so the producer wraps each into an i64-literal Expression and the consumers extract the literal back out, guarded by has_offset_expr()/has_count_expr() and falling back to 0 when unset (per the 0.98 "offset unset is treated as 0" semantics). - Proto: reserve fields 3/4, add offset_expr=5/count_expr=6 (verbatim 0.98). - Producer: FetchRelNode wraps offset/count via ExpressionBuilder.makeLongLiteral. - Velox: SubstraitToVeloxPlan + SubstraitToVeloxPlanValidator read the literals via SubstraitParser::getLiteralValue<int64_t>; VeloxToSubstraitPlan (the reverse converter) writes i64-literal expressions. - ClickHouse: FetchRelParser and SortRelParser::parseLimit read the count_expr / offset_expr literals. - Add FetchRelProtoSuite pinning the producer contract. Part of apache#12597 Generated-by: Claude Code (Claude Opus 4.8)
|
Run Gluten Clickhouse CI on x86 |
nielspardon
marked this pull request as ready for review
August 7, 2026 05:43
19 tasks
JkSelf
approved these changes
Aug 12, 2026
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
2 participants
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 replaced
FetchRel's scalarint64 offset = 3/int64 count = 4withExpression offset_expr = 5/Expression count_expr = 6. This migrates Gluten's vendored proto, the JVM producer, and both native consumers in lockstep, as one step of the Substraitv0.23.0→0.98.0proto rebase (#12597).Gluten's only
FetchRelproducer (LimitExecTransformer) always supplies literalLongoffset/count from Spark's Limit/Offset plans, so the change is mechanical: the producer wraps each value into an i64-literalExpression, and the consumers extract the literal back out — guarded byhas_offset_expr()/has_count_expr(), falling back to0when unset (matching the 0.98 "offset unset is treated as 0" semantics).FetchRelstays at Rel-oneof field 3, so there is no oneof renumbering.offset_expr = 5/count_expr = 6(vendored verbatim from 0.98).FetchRelNodewraps offset/count viaExpressionBuilder.makeLongLiteral(...).SubstraitToVeloxPlanandSubstraitToVeloxPlanValidatorread the literals viaSubstraitParser::getLiteralValue<int64_t>;VeloxToSubstraitPlan(the Velox→Substrait reverse converter) writes i64-literal expressions.FetchRelParserandSortRelParser::parseLimitread thecount_expr/offset_exprliterals.How was this patch tested?
FetchRelProtoSuitepins the producer contract (offset/count land as i64 literal expressions).protocdup-field check;gluten-substraitbuilds and the new suite passes (mvn -Pspark-3.5); the Velox native backend compiles (consumer, validator, and reverse converter);clang-format(v15) clean on all changed C++ files.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
🤖 Generated with AI