cloud-fan commented on code in PR #57013:
URL: https://github.com/apache/spark/pull/57013#discussion_r3552072909
##########
sql/core/src/test/scala/org/apache/spark/sql/ApproxTopKSuite.scala:
##########
@@ -513,6 +517,15 @@ class ApproxTopKSuite extends SharedSparkSession {
)
)
+ // TIME has no common type with DATE/TIMESTAMP/TIMESTAMP_NTZ (unlike those
three, which all
+ // mutually widen and so only fail at approx_top_k_combine runtime, not at
UNION analysis), so
+ // it cannot join mixedDateTimeTypes: "among different ... types - fail at
combine" below
+ // asserts every pair in that Seq unions successfully and only fails at the
runtime combine.
+ val mixedTimeTypes: Seq[(DataType, String, Seq[String])] = Seq(
Review Comment:
`mixedTimeTypes` has only a single `TIME(6)` entry, so it can't exercise the
TIME analogue of the "among different ... types - fail at combine" test. Two
different-precision TIMEs (e.g. `TIME(3)` vs `TIME(6)`) widen at UNION via
`findWiderDateTimeType` (`TimeType(max(p1,p2))`), so the UNION succeeds — but
the two sketch states carry `time(3)` vs `time(6)`, which `updateItemDataType`
rejects with `!=` at combine runtime (`APPROX_TOP_K_SKETCH_TYPE_NOT_MATCH`).
That "widens at UNION, fails at combine on precision" path is the exact TIME
parallel of the tested datetime behavior and is currently uncovered. Adding a
second, different-precision TIME entry here would close it. Non-blocking — the
behavior is correct; this is coverage 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]
