yadavay-amzn commented on code in PR #56910:
URL: https://github.com/apache/spark/pull/56910#discussion_r3502321750
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -1073,6 +1074,7 @@ object CatalogColumnStat extends Logging {
case TimestampNTZType =>
getTimestampFormatter(isParsing = false, forTimestampNTZ = true)
.format(v.asInstanceOf[Long])
+ case _: TimeType => TimeFormatter(isParsing =
false).format(v.asInstanceOf[Long])
Review Comment:
Fixed - switched the TimeType arm to TimeFormatter.getFractionFormatter(),
which preserves up to nanosecond resolution and round-trips losslessly with the
stringToTimeAnsi parse on the read side. Verified 12:00:00.123456 (nanos
43200123456000) now survives toExternalString -> fromExternalString unchanged
(previously it serialized to "12:00:00" and read back as 43200000000000).
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala:
##########
@@ -1047,6 +1047,7 @@ object CatalogColumnStat extends Logging {
case TimestampType => getTimestampFormatter(isParsing = true).parse(s)
case TimestampNTZType =>
getTimestampFormatter(isParsing = true, forTimestampNTZ =
true).parse(s)
+ case _: TimeType => TimeFormatter(isParsing = true).parse(s)
Review Comment:
Added a sub-second TIME min/max round-trip test in StatisticsCollectionSuite
using 12:00:00.123456. Confirmed it FAILS with the old fractionless formatter
(deserializes to 43200000000000) and passes with the fraction formatter, so it
guards this truncation going forward.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/FilterEstimationSuite.scala:
##########
@@ -523,6 +533,44 @@ class FilterEstimationSuite extends
StatsEstimationTestBase {
expectedRowCount = 3)
}
+ test("ctime = cast('10:00:00' AS TIME)") {
+ val t10 = DateTimeUtils.localTimeToNanos(java.time.LocalTime.of(10, 0, 0))
+ validateEstimatedStats(
+ Filter(EqualTo(attrTime, Literal(t10, TimeType())),
+ childStatsTestPlan(Seq(attrTime), 10L)),
+ Seq(attrTime -> ColumnStat(distinctCount = Some(1),
+ min = Some(t10), max = Some(t10),
+ nullCount = Some(0), avgLen = Some(8), maxLen = Some(8))),
+ expectedRowCount = 1)
+ }
+
+ test("ctime < cast('12:00:00' AS TIME)") {
+ // 12:00 is 43200000000000L nanos. Range is [08:00, 17:00] = 10 distinct
values.
+ // Fraction: (12:00 - 08:00) / (17:00 - 08:00) = 4/9 hours => ~4.44 =>
rounded to 5
Review Comment:
Reworded to: (12:00 - 08:00) / (17:00 - 08:00) = 4/9 of 10 rows => ~4.44 =>
ceil => 5.
--
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]
