[GitHub] [spark] RyanBerti commented on a diff in pull request #41203: [SPARK-16484][SQL] Update hll function type checks to also check for non-foldable inputs

["RyanBerti (via GitHub)" <gi...@apache.org>]

RyanBerti commented on code in PR #41203:
URL: https://github.com/apache/spark/pull/41203#discussion_r1196857575


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/datasketchesAggregates.scala:
##########
@@ -265,6 +288,26 @@ case class HllUnionAgg(
   override protected def withNewChildrenInternal(newLeft: Expression, newRight: Expression):
   HllUnionAgg = copy(left = newLeft, right = newRight)
 
+  // Overrides for ExpectsInputTypes
+
+  override def checkInputDataTypes(): TypeCheckResult = {

Review Comment:
   I think it's possible, but might be more valuable to create a new trait that extends ExpectsInputTypes that by default allows all of the types defined by inputTypes to be non-foldable, but provides a mechanism to override each type and define whether its foldable/non-foldable. Feels like this could be utilized by other functions too - thoughts?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org