[VL] Offload format_number to Velox native execution - #12775
Closed
minni31 wants to merge 1 commit into
Closed
Conversation
Wires Spark's format_number(numeric, int) Catalyst expression through
Gluten to the Velox native implementation for integer and floating-point
inputs. Two overloads are not supported natively and always fall back to
vanilla Spark:
- the format-pattern STRING second argument (e.g. '#,##0.00'), which
Velox does not implement, and
- DecimalType inputs, which would lose precision when cast to Double.
Fallback is achieved by throwing GlutenNotSupportException from the
transformer, which the validation path converts into a graceful fallback.
Details:
- ExpressionNames / ExpressionMappings: register Sig[FormatNumber].
- ExpressionConverter: dispatch FormatNumber to genFormatNumberTransformer.
- SparkPlanExecApi: default hook throws GlutenNotSupportException so
backends without native support fall back cleanly.
- VeloxSparkPlanExecApi: override with config gate plus StringType and
DecimalType fallbacks.
- GlutenConfig: add spark.gluten.sql.columnar.formatNumber (default true).
- Tests: integration tests in VeloxStringFunctionsSuite covering native
offload for int/bigint/float/double and fallback for string-pattern,
DecimalType, and the disabled-config kill switch.
- docs/Configuration.md: document the new config.
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Velox-native offload support for Spark SQL format_number(numeric, int) in Gluten (Velox backend), including a config gate and explicit fallbacks for unsupported overloads (string pattern overload and DecimalType input).
Changes:
- Register
FormatNumberin expression names/mappings and route it throughExpressionConverterinto a backend hook. - Introduce
spark.gluten.sql.columnar.formatNumber(defaulttrue) and implement Velox-side gating + type-based fallbacks viaGlutenNotSupportException. - Add Velox integration tests asserting both offload presence and fallback behavior, and document the new config.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shims/common/src/main/scala/org/apache/gluten/expression/ExpressionNames.scala | Adds the FORMAT_NUMBER expression name constant. |
| gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionMappings.scala | Registers Sig[FormatNumber] mapping to format_number. |
| gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala | Adds a FormatNumber case to delegate lowering to the backend hook. |
| gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala | Adds config definition + accessor for spark.gluten.sql.columnar.formatNumber. |
| gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala | Adds default genFormatNumberTransformer hook that forces fallback on non-supporting backends. |
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala | Implements Velox lowering with config gate and explicit fallbacks for String-pattern and DecimalType. |
| docs/Configuration.md | Documents the new spark.gluten.sql.columnar.formatNumber config. |
| backends-velox/src/test/scala/org/apache/gluten/execution/VeloxStringFunctionsSuite.scala | Adds integration tests for native offload and fallback scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+716
to
+727
| test("format_number executes natively for integer input") { | ||
| runQueryAndCompare( | ||
| s"select l_orderkey, format_number(l_orderkey, 2) " + | ||
| s"from $LINEITEM_TABLE limit $LENGTH")(assertFormatNumberOffloaded) | ||
| } | ||
|
|
||
| test("format_number executes natively for double input") { | ||
| runQueryAndCompare( | ||
| s"select l_orderkey, format_number(CAST(l_orderkey AS DOUBLE), 4) " + | ||
| s"from $LINEITEM_TABLE limit $LENGTH")(assertFormatNumberOffloaded) | ||
| } | ||
|
|
Member
|
Is this PR duplicate to #12754? |
Contributor
Author
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?
This PR offloads Spark's
format_number(numeric, int)function to the Velox native implementation via Gluten.Native offload is applied for integer and floating-point inputs (Byte/Short/Int/Long/Float/Double), which match Velox's registered
format_numbersignatures. Two overloads are intentionally not supported natively and always fall back to vanilla Spark:format_number(x, '#,##0.00')) — Velox only implements the integer decimal-places second argument, not the JavaDecimalFormatpattern overload.BigDecimalformatting; casting toDoublewould lose precision for values with more than 15 significant digits.Fallback is achieved by throwing
GlutenNotSupportExceptionfromgenFormatNumberTransformer; the validation path (ValidatablePlan.failValidationWithException) converts this into a graceful fallback to vanilla Spark, so no query can crash on an unsupported overload.The feature is controlled by a new config
spark.gluten.sql.columnar.formatNumber(defaulttrue).Wiring summary:
ExpressionNames/ExpressionMappings: registerSig[FormatNumber].ExpressionConverter: dispatchFormatNumbertogenFormatNumberTransformer.SparkPlanExecApi: default hook throwsGlutenNotSupportExceptionso backends without native support fall back cleanly.VeloxSparkPlanExecApi: override with the config gate plusStringType/DecimalTypefallbacks.How was this patch tested?
Added integration tests in
VeloxStringFunctionsSuite(compared against vanilla Spark viarunQueryAndCompare):ProjectExecTransformeractually carries theFormatNumberexpression (not merely that some transformer exists in the plan).FormatNumberis absent from anyProjectExecTransformerand present in a vanillaProjectExec.Was this patch authored or co-authored using generative AI tooling?
Generated-by: GitHub Copilot