stevomitric commented on code in PR #56968:
URL: https://github.com/apache/spark/pull/56968#discussion_r3979890441
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -4520,8 +4432,8 @@ case class TimeBucket(
override def inputTypes: Seq[AbstractDataType] = Seq(
Review Comment:
we should probably update `@ExpressionDescription` (public facing docs).
They only specify TIMESTAMP and TIMESTAMP_NTZ atm.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala:
##########
@@ -2955,6 +2946,157 @@ class DateExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
}
}
+ test("time_bucket: nanosecond-precision day-time interval") {
+ import org.apache.spark.sql.catalyst.util.TimestampNanosTestUtils._
+ withSQLConf(
+ SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC",
+ SQLConf.TIMESTAMP_NANOS_TYPES_ENABLED.key -> "true") {
+ foreachNanosPrecision { p =>
+ Seq(TimestampLTZNanosType(p), TimestampNTZNanosType(p)).foreach { dt =>
+ val bucketSize = Literal(Duration.ofSeconds(1))
+
+ // Normal case: ts strictly inside a bucket, unrelated to any
sub-micro boundary.
+ // origin = (0us, 500ns); ts = (3_500_000us, 999ns) -> bucket =
(3_000_000us, 500ns).
+ checkEvaluation(
+ TimeBucket(
+ bucketSize,
+ Literal.create(nanosVal(3500000L, 999), dt),
+ Literal.create(nanosVal(0L, 500), dt)),
+ nanosVal(3000000L, 500))
+
+ // Boundary case, ts's sub-micro remainder < origin's: ts's true
instant falls just
+ // short of the nominal grid point at the same microsecond, so it
belongs to the
+ // previous bucket. origin = (0us, 500ns); ts = (3_000_000us, 200ns)
->
+ // bucket = (2_000_000us, 500ns), not (3_000_000us, 500ns).
+ checkEvaluation(
+ TimeBucket(
+ bucketSize,
+ Literal.create(nanosVal(3000000L, 200), dt),
+ Literal.create(nanosVal(0L, 500), dt)),
+ nanosVal(2000000L, 500))
+
+ // Boundary case, ts's sub-micro remainder >= origin's: ts's true
instant is at or
+ // after the nominal grid point, so it starts a new bucket there.
+ // origin = (0us, 500ns); ts = (3_000_000us, 700ns) -> bucket =
(3_000_000us, 500ns).
+ checkEvaluation(
+ TimeBucket(
+ bucketSize,
+ Literal.create(nanosVal(3000000L, 700), dt),
+ Literal.create(nanosVal(0L, 500), dt)),
+ nanosVal(3000000L, 500))
+
+ // Multi-day bucket (calendar-aware path) with sub-micro origin
remainder preserved.
+ checkEvaluation(
+ TimeBucket(
+ Literal(Duration.ofDays(1)),
+ Literal.create(nanosVal(DateTimeUtils.daysToMicros(3, UTC) +
500000L, 999), dt),
+ Literal.create(nanosVal(500, 250), dt)),
+ nanosVal(DateTimeUtils.daysToMicros(3, UTC) + 500, 250))
+
+ // NULL ts -> NULL
+ checkEvaluation(
+ TimeBucket(
+ bucketSize,
+ Literal.create(null, dt),
+ Literal.create(nanosVal(0L, 0), dt)),
+ null)
+
+ // NULL origin -> NULL
+ checkEvaluation(
+ TimeBucket(
+ bucketSize,
+ Literal.create(nanosVal(0L, 0), dt),
+ Literal.create(null, dt)),
+ null)
+ }
+ }
+ }
+ }
+
+ test("time_bucket: nanosecond-precision year-month interval") {
+ import org.apache.spark.sql.catalyst.util.TimestampNanosTestUtils._
+ withSQLConf(
+ SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC",
Review Comment:
Can we add some non-utc tests for LTZ? just to be coverage-complete.
--
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]
