Conversation
## Summary Route decimal BROUND to decimal_bround and pass the Spark expression's captured ANSI flag for integral calls. Add scale/JVM planning guards and Spark 4.1 integration coverage that distinguishes complete native execution from partial or full Spark fallback. ## Prompting Intent Integrate the independently designed upstream BROUND implementation in Apache Gluten, preserve Spark 4.1 correctness across execution modes and supported JVMs, add and run regression tests, and prepare a draft companion PR using only public implementation material and qualification evidence. ## Linked Sources - Companion upstream Velox implementation: facebookincubator/velox#19072 - Spark 4.1.1 BRound and captured ANSI semantics: https://github.com/apache/spark/blob/v4.1.1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala - Apache Gluten architecture baseline: https://github.com/apache/gluten/tree/48b5fca64d72b863960bca8129d11cbfdfae03d8 - Public pinned Velox dependency used with the companion BROUND patch: https://github.com/IBM/velox/tree/dft-2026_09_14 ## Rationale Serialize expression-level ANSI state rather than reading a later session setting; even an already materialized native physical plan must retain its overflow policy. The backend hook keeps other backends' existing argument shape unchanged. Check supported scales before native initialization can defer errors, and retain Spark execution for pre-JDK21 nonzero floating scales whose decimal-conversion semantics differ. The tests cover both mode-change directions after analysis and physical planning, native floating null scales, overflow, output types, and exact floating bits. Existing ROUND execution remains covered. The companion draft depends on the Velox change reaching the pinned dependency before merge; the stock dependency is not claimed to contain the new function. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d412b830-d346-4c14-b070-3bc5a6cacba1
|
Run Gluten Clickhouse CI on x86 |
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?
Wire Spark BROUND to the corresponding Velox functions, including routing
decimal inputs to
decimal_bround.Integral calls carry the Spark expression's captured ANSI flag as a native
constant. This preserves overflow behavior even when a DataFrame is analyzed
or physically planned before the session mode changes.
Add planning-time compatibility checks:
before native execution is necessary because primitive initialization errors
can be deferred.
preserve their decimal-conversion behavior. Scale-zero floating rounding and
integral/decimal calls remain eligible for native execution.
retain safe fallback.
Existing ROUND behavior is unchanged.
Dependency: this draft requires the companion upstream Velox BROUND change
to be included in Gluten's pinned Velox dependency before merge.
facebookincubator/velox#19072
The integration was built and exercised against public IBM Velox
dft-2026_09_14with that same production patch applied:https://github.com/IBM/velox/tree/dft-2026_09_14
The stock tag does not yet contain BROUND. This PR does not claim otherwise or
replace the dependency pin with an unrelated upstream build.
How was this patch tested?
addition to the earlier parser regressions. The new tests first failed
against the baseline: missing BROUND registration and incorrect decimal
function mapping.
assert the actual native BROUND projection or the intentional fallback,
compare schemas and floating-point bits, and exercise integral overflow,
decimal precision/overflow, folded and NULL scales (including native FLOAT
and DOUBLE NULL-scale calls on both JVMs and execution modes), unsupported scales,
and mode changes after analysis and after cached native physical planning.
A remaining Spark BROUND in a partially native projection fails the check.
interpreted/generated expressions on 5,535,698 distinct typed inputs
(11,071,396 LEGACY/ANSI mode-cases per flat and dictionary/partial-selection
shape) with zero mismatches. Another 586,332 mode-cases per shape covered
every supported scale.
the native query mode deliberately set opposite to the captured mode.
The multi-million floating campaign uses JDK 21. A separate actual-JVM
comparison established the older-JDK differences that motivate the fallback;
461,532 native-eligible dense-scale cases matched between JDK 17 and JDK 21.
The evidence does not claim universal cross-JDK floating equivalence, exact
error-message equivalence, exhaustive correctness, or performance gains.
Six sequential OpenAI code-review rounds completed; findings were corrected
and rechecked before publication.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: GitHub Copilot CLI 1.0.86-2; OpenAI models.