uros-b commented on code in PR #56969:
URL: https://github.com/apache/spark/pull/56969#discussion_r3983010805
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -3032,24 +3032,46 @@ case class MonthsBetween(
override def second: Expression = date2
override def third: Expression = roundOff
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType,
TimestampType, BooleanType)
+ // Nanosecond-precision timestamps are accepted alongside the microsecond
types. The result is a
+ // fraction of a month derived from the whole-day and whole-second parts of
each operand, so the
+ // sub-microsecond remainder cannot move it and each operand contributes
only its epochMicros.
+ override def inputTypes: Seq[AbstractDataType] = Seq(
+ TypeCollection(AnyTimestampType, AnyTimestampNanoType),
+ TypeCollection(AnyTimestampType, AnyTimestampNanoType),
+ BooleanType)
override def dataType: DataType = DoubleType
override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
copy(timeZoneId = Option(timeZoneId))
+ @transient private lazy val zoneIdInEval: ZoneId =
zoneIdForType(date1.dataType)
+
+ // For the nanosecond carrier the child value is a boxed TimestampNanosVal,
so read its
+ // epochMicros; for the microsecond timestamp types it is already a boxed
Long.
+ private def toMicros(value: Any): Long = value match {
+ case v: TimestampNanosVal => v.epochMicros
+ case n => n.asInstanceOf[Long]
+ }
+
override def nullSafeEval(t1: Any, t2: Any, roundOff: Any): Any = {
DateTimeUtils.monthsBetween(
- t1.asInstanceOf[Long], t2.asInstanceOf[Long],
roundOff.asInstanceOf[Boolean], zoneId)
+ toMicros(t1), toMicros(t2), roundOff.asInstanceOf[Boolean], zoneIdInEval)
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
+ // The nanosecond carrier exposes epochMicros as a public field; the
microsecond types are
+ // already primitive longs. Reduce each operand to microseconds before
taking the difference.
+ def toMicrosCode(e: Expression): String => String = e.dataType match {
+ case _: AnyTimestampNanoType => c => s"$c.epochMicros"
+ case _ => c => c
+ }
+ val micros1 = toMicrosCode(date1)
+ val micros2 = toMicrosCode(date2)
+ val zid = ctx.addReferenceObj("zoneId", zoneIdInEval,
classOf[ZoneId].getName)
val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, (d1, d2, roundOff) => {
- s"""$dtu.monthsBetween($d1, $d2, $roundOff, $zid)"""
- })
+ defineCodeGen(ctx, ev, (d1, d2, roundOff) =>
+ s"""$dtu.monthsBetween(${micros1(d1)}, ${micros2(d2)}, $roundOff,
$zid)""")
}
Review Comment:
`@ExpressionDescription` still has no nanosecond-precision example
(unchanged from iteration 1); sibling extended nanos exprs added one.
##########
sql/core/src/test/resources/sql-tests/inputs/timestamp-ntz.sql:
##########
@@ -36,6 +36,14 @@ select timestampdiff(HOUR, timestamp_ntz'2022-02-14
01:02:03', timestamp_ltz'202
select timestampdiff(YEAR, date'2022-02-15', timestamp_ntz'2023-02-15
10:11:12');
select timestampdiff(MILLISECOND, timestamp_ntz'2022-02-14 23:59:59.123',
date'2022-02-15');
+-- SPARK-57819: months_between derives its time zone from the first operand's
family, mirroring
Review Comment:
The comment "mirroring the existing SubtractTimestamps / `timestamp -
timestamp` convention" is false at current master.
`DateTimeOperationsTypeCoercion` and `AnsiDateTimeOperationsTypeCoercion`
(TypeCoercionHelper.scala:864-905) cast a cross-family `SubtractTimestamps(l,
r)` pair to a common family (`subtractTimestampsCommonType`, unifying to the
NTZ family) BEFORE eval, so its `zoneIdForType(left)` never spans mixed
families and stays symmetric. `MonthsBetween` has no coercion rule (only
`FunctionRegistry.scala:648`) and is `ImplicitCastInputTypes` over
`TypeCollection` (each operand cast independently), so it is the ONE
timestamp-diff expression that evaluates a genuinely mixed pair with `date1`'s
zone applied to both. Correct the comment, or add the analogous coercion for
true symmetry.
--
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]
