MaxGekk commented on code in PR #48610:
URL: https://github.com/apache/spark/pull/48610#discussion_r1853861146


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/xml/XmlExpressionEvalUtils.scala:
##########
@@ -40,3 +41,98 @@ object XmlExpressionEvalUtils {
     UTF8String.fromString(dataType.sql)
   }
 }
+
+object XPathEvaluatorFactory {
+  def create(dataType: DataType, path: UTF8String): XPathEvaluator = {
+    dataType match {

Review 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:
   ```scala
     @transient override lazy val evaluator = 
XPathBooleanEvaluator(pathUTF8String)
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/xml/XmlExpressionEvalUtils.scala:
##########
@@ -40,3 +41,98 @@ object XmlExpressionEvalUtils {
     UTF8String.fromString(dataType.sql)
   }
 }
+
+object XPathEvaluatorFactory {
+  def create(dataType: DataType, path: UTF8String): XPathEvaluator = {
+    dataType match {
+      case BooleanType => XPathBooleanEvaluator(path)
+      case ShortType => XPathShortEvaluator(path)
+      case IntegerType => XPathIntEvaluator(path)
+      case LongType => XPathLongEvaluator(path)
+      case FloatType => XPathFloatEvaluator(path)
+      case DoubleType => XPathDoubleEvaluator(path)
+      case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path)
+      case ArrayType(elementType, _) if elementType.isInstanceOf[StringType] =>

Review Comment:
   ```suggestion
         case ArrayType(_: StringType, _) =>
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/xml/XmlExpressionEvalUtils.scala:
##########
@@ -40,3 +41,98 @@ object XmlExpressionEvalUtils {
     UTF8String.fromString(dataType.sql)
   }
 }
+
+object XPathEvaluatorFactory {
+  def create(dataType: DataType, path: UTF8String): XPathEvaluator = {
+    dataType match {
+      case BooleanType => XPathBooleanEvaluator(path)
+      case ShortType => XPathShortEvaluator(path)
+      case IntegerType => XPathIntEvaluator(path)
+      case LongType => XPathLongEvaluator(path)
+      case FloatType => XPathFloatEvaluator(path)
+      case DoubleType => XPathDoubleEvaluator(path)
+      case dt if dt.isInstanceOf[StringType] => XPathStringEvaluator(path)

Review Comment:
   nit: just for consistency w/ other places:
   ```suggestion
         case _: StringType => XPathStringEvaluator(path)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to