refactor: move string expression support checks to getSupportLevel#4676
Merged
Merged
Conversation
Static support decisions for the string expressions are moved out of the serde convert methods and into getSupportLevel: - Substring: pos and len must be literals - Left, Right: length argument must be a literal - ConcatWs: all-foldable args fall back so Spark can constant-fold (the NULL-separator case still converts directly to a NULL result) - Like: custom escape character is unsupported Behavior is unchanged: each case that previously fell back to Spark now reports Unsupported with the same reason. The remaining withFallbackReason calls are child-reason rollups.
comphead
reviewed
Jun 18, 2026
|
|
||
| object CometSubstring extends CometExpressionSerde[Substring] { | ||
|
|
||
| override def getSupportLevel(expr: Substring): SupportLevel = (expr.pos, expr.len) match { |
comphead
reviewed
Jun 18, 2026
| } | ||
| } | ||
|
|
||
| override def convert(expr: Like, inputs: Seq[Attribute], binding: Boolean): Option[Expr] = { |
Contributor
There was a problem hiding this comment.
maybe one day we should rename this method,it confuses me sometimes what is this conversion, is it expression rewrite, simplification, etc
Member
Author
There was a problem hiding this comment.
Would be good to consider doing all the renames we want to do before we release 1.0.0
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.
Which issue does this PR close?
Part of #4673.
Rationale for this change
The serde framework intends that an expression declares whether it can be accelerated through
getSupportLevel, with the central dispatcher inQueryPlanSerdetranslatingIncompatible/Unsupportedinto the appropriate fallback. Several string serdes instead made these decisions insideconvertby callingwithFallbackReasondirectly, even though the decisions are statically determinable from the expression.What changes are included in this PR?
Move static support decisions out of
convertand intogetSupportLevelfor the string expressions:Substring:posandlenmust be literals.Left,Right: the length argument must be a literal (folded into the existinggetSupportLevel).ConcatWs: all-foldable arguments fall back so Spark'sConstantFoldingcan handle them. The NULL-separator case still converts directly to a NULL result, so the precedence is preserved ingetSupportLevel.Like: a custom escape character is unsupported.This is behavior-preserving: every case that previously fell back to Spark continues to fall back with the same reason. The remaining
withFallbackReasoncalls in this file are child-reason rollups, which are the intended use insideconvert.How are these changes tested?
Covered by existing tests:
CometStringExpressionSuite(33 tests, includingconcat_ws,left,right) andCometExpressionSuite(like with custom escape,string type and substring,substring with start < 1), which exercise both the supported and fallback paths.