zhztheplayer commented on code in PR #13105:
URL: https://github.com/apache/gluten/pull/13105#discussion_r4090992871


##########
backends-velox/src/main/scala/org/apache/spark/sql/expression/UDFResolver.scala:
##########
@@ -386,14 +422,62 @@ object UDFResolver extends Logging {
     }
   }
 
+  /**
+   * Resolves the types of a call by binding 'argTypes' against the signatures 
the library
+   * registered with Velox for 'name'. Returns the resolved types, or None if 
nothing binds.
+   *
+   * Binding is exact: no cast is injected to make a call fit. Velox does not 
support coercion for
+   * signatures carrying type variables, and where one is in play "cast the 
arguments until they
+   * bind" has too many answers to pick from.
+   */
+  private def resolveFromRegistry(
+      name: String,
+      argTypes: Seq[DataType],
+      isAggregate: Boolean): RegistryResolution = {
+    def resolve(): RegistryResolution = {
+      // Velox types carry no nullability, so it is not part of the question
+      // being asked here.
+      val argTypeNodes = argTypes.map(t => ConverterUtils.getTypeNode(t, 
nullable = true))
+      val serialized = TypeBuilder.makeStruct(false, 
argTypeNodes.asJava).toProtobuf.toByteArray
+
+      val resolved = if (isAggregate) {
+        UdfJniWrapper.resolveUdafTypes(name, serialized)
+      } else {
+        UdfJniWrapper.resolveUdfType(name, serialized)
+      }
+
+      if (resolved == null) {
+        logDebug(
+          s"No Velox signature of $name binds to " +
+            s"${argTypes.map(_.simpleString).mkString(", ")}.")
+        None
+      } else if (isAggregate) {
+        // {returnType, intermediateType}, as one struct.
+        val fields = ConverterUtils
+          .parseFromBytes(resolved)
+          .dataType
+          .asInstanceOf[StructType]
+          .fields
+        Some(fields.map(f => ExpressionType(f.dataType, f.nullable)).toSeq)
+      } else {
+        Some(Seq(ConverterUtils.parseFromBytes(resolved)))
+      }
+    }
+
+    registryResolutions.computeIfAbsent((name, argTypes), _ => resolve())

Review Comment:
   Can the code be strengthened, in case the same name is registered as both an 
UDF and an UDAF? E.g., should we have both `registryUdfResolutions` and 
`registryUdafResolutions`?



-- 
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