uros-b commented on code in PR #58744:
URL: https://github.com/apache/spark/pull/58744#discussion_r4005882797
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/datasketchesAggregates.scala:
##########
@@ -120,7 +120,8 @@ case class HllSketchAgg(
IntegerType,
LongType,
StringTypeWithCollation(supportsTrimCollation = true),
- BinaryType),
+ BinaryType,
+ AnyTimeType),
IntegerType)
Review Comment:
`datasketchesAggregates.scala` (`inputTypes` `TypeCollection`, ~L118-124) --
the reorder is a silent, unexplained ordering constraint. Nothing in the code
(no comment) or the tests records that `AnyTimeType` MUST stay after
`StringType`, and the restored TIMESTAMP/TIMESTAMP_NTZ/DATE -> STRING coercion
is still untested (the added goldens/unit test cover only TIME inputs). A
future reorder -- e.g. grouping TIME next to the numeric members -- would
silently reintroduce the timestamp under-count with green CI. Add a
datetime-input golden case (or at least a one-line comment on the ordering
requirement) to lock it in. Correctness is now fine; this is regression
hardening only.
--
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]
