[SPARK-50081][SQL] Codegen Support for XPath*(by Invoke & RuntimeReplaceable)
#48610
Conversation
panbingkun
changed the title
XPath...(by Invoke & RuntimeReplaceable)XPath*(by Invoke & RuntimeReplaceable)
| case LongType => XPathLongEvaluator(path) | ||
| case FloatType => XPathFloatEvaluator(path) | ||
| case DoubleType => XPathDoubleEvaluator(path) | ||
| case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path) |
There was a problem hiding this comment.
Because we need to support Collation (eg: StringType(...)), we have to write it this way.
There was a problem hiding this comment.
Will it work after your changes? Just in case, are there any tests? cc @stefankandic @uros-db
There was a problem hiding this comment.
@panbingkun please add expression-level tests (using checkEvaluation) for xpath_* functions
see for example: CollationStringExpressionsSuite
There was a problem hiding this comment.
Will it work after your changes? Just in case, are there any tests? cc @stefankandic @uros-db
What I described may have caused misunderstandings,
Let me rephrase: This is written so that the case CollationExpressionWalkerSuite#SPARK-48280: Expression Walker for SQL query examples can be passed:
There was a problem hiding this comment.
@panbingkun please add expression-level tests (using
checkEvaluation) forxpath_*functions see for example:CollationStringExpressionsSuite
Currently, it is not supported. My description caused a misunderstanding, and I have corrected it.
MaxGekk
left a comment
MaxGekk
left a comment
There was a problem hiding this comment.
@panbingkun Could you resolve conflicts, please.
Updated, thanks! |
| case LongType => XPathLongEvaluator(path) | ||
| case FloatType => XPathFloatEvaluator(path) | ||
| case DoubleType => XPathDoubleEvaluator(path) | ||
| case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path) |
There was a problem hiding this comment.
nit: just for consistency w/ other places:
| case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path) | |
| case _: StringType => XPathStringEvaluator(path) |
| case FloatType => XPathFloatEvaluator(path) | ||
| case DoubleType => XPathDoubleEvaluator(path) | ||
| case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path) | ||
| case ArrayType(elementType, _) if elementType.isInstanceOf[StringType] => |
There was a problem hiding this comment.
| case ArrayType(elementType, _) if elementType.isInstanceOf[StringType] => | |
| case ArrayType(_: StringType, _) => |
|
|
||
| object XPathEvaluatorFactory { | ||
| def create(dataType: DataType, path: UTF8String): XPathEvaluator = { | ||
| dataType match { |
There was a problem hiding this comment.
Chasing the dataType doesn't look nice. How about to distribute instantiations across expressions:
- Define in
XPathExtract:
protected def evaluator: XPathEvaluator
- and override it childs:
@transient override lazy val evaluator = XPathBooleanEvaluator(pathUTF8String)
|
+1, LGTM. Merging to master. |
3 participants
What changes were proposed in this pull request?
The pr aims to add
CodegenSupport forxpath*, include:xpath_booleanxpath_shortxpath_intxpath_longxpath_floatxpath_doublexpath_stringxpathWhy are the changes needed?
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GA & Existed UT (eg:
XPathFunctionsSuite,XPathExpressionSuite,CollationSQLExpressionsSuite#*XPath*,CollationExpressionWalkerSuite)Was this patch authored or co-authored using generative AI tooling?
No.