[VL] Support partial fallback for unsupported expressions#12486
Open
taiyang-li wants to merge 7 commits into
Open
[VL] Support partial fallback for unsupported expressions#12486taiyang-li wants to merge 7 commits into
taiyang-li wants to merge 7 commits into
Conversation
taiyang-li
changed the title
…l project Support custom catalyst expressions from user jars in ColumnarPartialProject when Velox cannot map them natively, while keeping ScalaUDF and HiveUDF on their existing fallback paths.
Keep the new generic unmapped catalyst expression note concise while preserving the intent of the fallback behavior.
taiyang-li
force-pushed
the
liyang/fallback-generic-unmapped-catalyst-expr
branch
from
July 12, 2026 06:30
e567ad0 to
6b98127
Compare
Contributor
|
Will this pr affect the CH backend? Can some UT be added to the CH backend? |
Contributor
Author
No. Only codes in velox backend modified. For Velox backend, I add 2 extra testGluten() uts and disable 2 original test() uts in spark4.0 and spark4.1. And it shouldn't have affect on CH backend. |
Contributor
Author
jackylee-ch
reviewed
Jul 15, 2026
| expr.deterministic && | ||
| !expr.isInstanceOf[Unevaluable] && | ||
| !expr.isInstanceOf[LambdaFunction] && | ||
| !ExpressionMappings.expressionsMap.contains(expr.getClass) |
Contributor
There was a problem hiding this comment.
do we need to consider the ExpressionConverter.tryTransformWithoutExpressionMapping whole check?
Contributor
Author
There was a problem hiding this comment.
@jackylee-ch yes, it should be considered. I had a minor refactor later. Let's wait for the latest CI.
…n be evaluated rowly
Contributor
There was a problem hiding this comment.
Pull request overview
Extends Velox’s ColumnarPartialProjectExec to detect deterministic Catalyst expressions that are not convertible to native (including user-jar “generic unmapped” expressions) and route them through partial-project fallback earlier, avoiding native conversion/runtime failures.
Changes:
- Add a “row-evaluable but unsupported” detection path in
ColumnarPartialProjectExecand replace such expressions with aliases for partial fallback. - Add Velox unit tests in
UDFPartialProjectSuiteto validate fallback behavior for an unmapped custom Catalyst expression and for unsupported built-in functions. - Add Spark 4.0/4.1 gluten-ut coverage for
variant_get(non-literal path) and HLL negative tests, and update Velox test settings with new exclusions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backends-velox/src/main/scala/org/apache/gluten/execution/ColumnarPartialProjectExec.scala | Detects deterministic, row-evaluable expressions that can’t be natively converted and routes them through partial-project fallback via alias replacement. |
| backends-velox/src/test/scala/org/apache/gluten/expression/UDFPartialProjectSuite.scala | Adds a custom unmapped Catalyst expression + tests to assert partial-project fallback behavior. |
| gluten-ut/spark41/src/test/scala/org/apache/spark/sql/GlutenVariantSuite.scala | Adds a variant_get(parse_json(...), path, ...) non-literal-path test. |
| gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenVariantSuite.scala | Same variant_get non-literal-path test for Spark 4.0. |
| gluten-ut/spark41/src/test/scala/org/apache/spark/sql/GlutenDataFrameAggregateSuite.scala | Adds HLL negative tests checking wrapped Spark exceptions / error classes. |
| gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenDataFrameAggregateSuite.scala | Same HLL negative tests for Spark 4.0. |
| gluten-ut/spark41/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Excludes the newly added Variant/HLL tests for Velox test runs (and documents the rationale inline). |
| gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala | Same exclusions for Spark 4.0 Velox test settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+82
to
+86
| spark.sessionState.functionRegistry.registerFunction( | ||
| FunctionIdentifier("dummy_unmapped"), | ||
| new ExpressionInfo(classOf[DummyUnmappedExpr].getName, "dummy_unmapped"), | ||
| (children: Seq[Expression]) => DummyUnmappedExpr(children.head) | ||
| ) |
Comment on lines
863
to
867
| enableSuite[GlutenVariantSuite] | ||
| // TODO: Velox parquet writer marks all struct fields as OPTIONAL (nullable), | ||
| // but Spark's variant type requires REQUIRED fields. Needs Velox-side fix. | ||
| .exclude("non-literal variant_get") | ||
| .exclude("SPARK-47546: invalid variant binary") |
taiyang-li
changed the title
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 were proposed in this pull request?
ColumnarPartialProject previously did not recognize generic unmapped catalyst expressions shipped in user jars. When such a custom catalyst expression was not registered in
ExpressionMappings, it was not identified for partial-project fallback and could still reach native expression conversion and fail at runtime.This patch makes Velox detect those generic unmapped catalyst expressions earlier and route them through
ColumnarPartialProjectfallback.ScalaUDFand Hive UDF keep using their existing dedicated paths instead of being folded into the generic fallback path.How was this patch tested?
DummyUnmappedExprinUDFPartialProjectSuiteDoes this PR introduce any user-facing change?
No.
Was this patch authored or co-authored using generative AI tooling?
Yes.
Generated-by: OpenAI GPT-5