From de46799d3dd2ab229f2cd7756b42b81803ac47ed Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 28 Sep 2026 12:54:49 -0600 Subject: [PATCH 1/3] fix: count the days partition transform in UTC CometDays cast timestamps to dates in the session timezone, while CometHours and Iceberg's days transform count in UTC. Spark cannot evaluate the transform, so there is no Spark result to match. Use UTC so the transforms agree with each other and with Iceberg. Closes #6333. --- .../src/main/scala/org/apache/comet/serde/datetime.scala | 9 ++++----- .../org/apache/comet/CometTemporalExpressionSuite.scala | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/serde/datetime.scala b/spark/src/main/scala/org/apache/comet/serde/datetime.scala index 5106e0059ac..333b694e6ff 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -22,7 +22,6 @@ package org.apache.comet.serde import java.util.Locale import org.apache.spark.sql.catalyst.expressions.{AddMonths, Attribute, Cast, ConvertTimezone, DateAdd, DateDiff, DateFormatClass, DateFromUnixDate, DateSub, DayOfMonth, DayOfWeek, DayOfYear, Days, Expression, FromUTCTimestamp, GetDateField, GetTimestamp, Hour, Hours, LastDay, Literal, MakeDate, MakeDTInterval, MakeInterval, MakeTimestamp, MakeYMInterval, MicrosToTimestamp, MillisToTimestamp, Minute, Month, MonthsBetween, MultiplyDTInterval, NextDay, PreciseTimestampConversion, Quarter, Second, SecondsToTimestamp, TimestampAdd, TimestampDiff, ToUnixTimestamp, ToUTCTimestamp, TruncDate, TruncTimestamp, UnixDate, UnixMicros, UnixMillis, UnixSeconds, UnixTimestamp, WeekDay, WeekOfYear, Year} -import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types.{CalendarIntervalType, DataType, DateType, DoubleType, FloatType, IntegerType, LongType, StringType, TimestampNTZType, TimestampType} import org.apache.spark.unsafe.types.UTF8String @@ -820,8 +819,9 @@ object CometHours extends CometExpressionSerde[Hours] { * For DateType: dates are internally stored as days since epoch, so this is a simple cast to * integer (same as CometUnixDate). * - * For TimestampType: uses a timezone-aware Cast(Timestamp to Date) followed by Cast(Date to Int). - * The first cast respects the session timezone to correctly determine the date boundary. + * For TimestampType: uses Cast(Timestamp to Date) in UTC followed by Cast(Date to Int). Spark + * cannot evaluate `days` itself, so this counts days in UTC like [[CometHours]] and Iceberg's + * `days` transform, rather than in the session timezone. */ object CometDays extends CometExpressionSerde[Days] { @@ -844,9 +844,8 @@ object CometDays extends CometExpressionSerde[Days] { val dateExprOpt = expr.child.dataType match { case DateType => childExpr case TimestampType => - val timezone = SQLConf.get.sessionLocalTimeZone childExpr.flatMap { child => - CometCast.castToProto(expr, Some(timezone), DateType, child, CometEvalMode.LEGACY) + CometCast.castToProto(expr, Some("UTC"), DateType, child, CometEvalMode.LEGACY) } case _ => None } diff --git a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala index 4d950b8718f..d7c39cc56fb 100644 --- a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala @@ -777,7 +777,7 @@ class CometTemporalExpressionSuite extends CometTestBase with AdaptiveSparkPlanH withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> timezone) { checkDays( tsDF.select(col("ts"), getColumnFromExpression(Days(UnresolvedAttribute("ts")))), - tsDF.selectExpr("ts", "unix_date(cast(ts as date))")) + tsDF.selectExpr("ts", "cast(floor(unix_micros(ts) / 86400000000D) as int)")) } } } @@ -823,8 +823,8 @@ class CometTemporalExpressionSuite extends CometTestBase with AdaptiveSparkPlanH java.sql.Timestamp.valueOf("2024-06-15 10:30:00"), DataTypes.TimestampType)))), dummyDF.selectExpr( - "unix_date(cast(TIMESTAMP('1970-01-01 00:00:00') as date))", - "unix_date(cast(TIMESTAMP('2024-06-15 10:30:00') as date))")) + "cast(floor(unix_micros(TIMESTAMP('1970-01-01 00:00:00')) / 86400000000D) as int)", + "cast(floor(unix_micros(TIMESTAMP('2024-06-15 10:30:00')) / 86400000000D) as int)")) // Null handling checkDays( From 899cb5a537726e3bf18375e46340ede47629df79 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 30 Sep 2026 14:26:39 -0600 Subject: [PATCH 2/3] test: pin UTC flooring of days for a pre-epoch timestamp Check days one microsecond before the epoch in an Asia/Tokyo session. That instant is day -1 in UTC but already 1970-01-01 in Tokyo, so the session-timezone serde returned 0 and fails this check. The existing timestamp literal checks pass either way: they run in the default America/Los_Angeles session, away from a day boundary. --- .../org/apache/comet/CometTemporalExpressionSuite.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala index b3d99f3350c..f44070963b9 100644 --- a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala @@ -853,6 +853,14 @@ class CometTemporalExpressionSuite extends CometTestBase with AdaptiveSparkPlanH "cast(floor(unix_micros(TIMESTAMP('1970-01-01 00:00:00')) / 86400000000D) as int)", "cast(floor(unix_micros(TIMESTAMP('2024-06-15 10:30:00')) / 86400000000D) as int)")) + // One microsecond before the epoch. Days are counted in UTC and floored, so this is day -1 + // even though it is already 1970-01-01 in Asia/Tokyo. + withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "Asia/Tokyo") { + checkDays( + dummyDF.select(getColumnFromExpression(Days(Literal(-1L, DataTypes.TimestampType)))), + dummyDF.selectExpr("-1")) + } + // Null handling checkDays( dummyDF.select(getColumnFromExpression(Days(Literal.create(null, DataTypes.DateType)))), From 536b259da713f42eeb761a5886927d4631e51685 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Mon, 5 Oct 2026 09:11:09 -0600 Subject: [PATCH 3/3] docs: note where days differs from Iceberg for pre-1970 timestamps --- spark/src/main/scala/org/apache/comet/serde/datetime.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spark/src/main/scala/org/apache/comet/serde/datetime.scala b/spark/src/main/scala/org/apache/comet/serde/datetime.scala index 0854043c3dd..ed5faa019ec 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -824,6 +824,11 @@ object CometHours extends CometExpressionSerde[Hours] { * For TimestampType: uses Cast(Timestamp to Date) in UTC followed by Cast(Date to Int). Spark * cannot evaluate `days` itself, so this counts days in UTC like [[CometHours]] and Iceberg's * `days` transform, rather than in the session timezone. + * + * The cast is a true floor, as [[CometHours]] is. Iceberg's `DateTimeUtil.microsToDays` differs + * in one case: it puts a pre-1970 timestamp exactly 999999 microseconds past midnight, such as + * `1969-01-01 00:00:00.999999`, in the previous day. Comet's native kernel for Iceberg's own + * `days` function matches Iceberg there, and this cast does not. */ object CometDays extends CometExpressionSerde[Days] {