You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/28 18:54:57 UTC
[PR] fix: count the days partition transform in UTC [datafusion-comet]
andygrove opened a new pull request, #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348
## Which issue does this PR close?
Closes #6333.
## Rationale for this change
Spark's `Days` and `Hours` partition transforms are `Unevaluable`: evaluating either one in a projection throws `PARTITION_TRANSFORM_EXPRESSION_NOT_IN_PARTITIONED_BY`. Comet evaluates both natively, but it counted them differently:
- `CometDays` cast the timestamp to a date in the session timezone.
- `CometHours` divides the raw UTC microseconds.
Iceberg's own `days` and `hours` transforms are UTC-based, and so are Comet's native versions of them. So in a non-UTC session, Comet's `days` gave a different day than `hours` and Iceberg imply for the same timestamp. For example, in `America/Los_Angeles`, `days(TIMESTAMP'2024-07-01T03:00:00Z')` returned 19904 (2024-06-30) instead of 19905.
## What changes are included in this PR?
- `CometDays` casts a timestamp to a date in UTC instead of in the session timezone. Date inputs are unchanged.
- The `days - timestamp input` and `days - literal edge cases` tests in `CometTemporalExpressionSuite` now compare against a UTC day count, `floor(unix_micros(ts) / 86400000000)`, like the `hours` test does. Before, they compared against `unix_date(CAST(ts AS DATE))`, which depends on the session timezone.
## How are these changes tested?
- **Old serde, updated tests.** With the old serde, `days - timestamp input` fails against the new baseline in the `America/Los_Angeles` and `Asia/Tokyo` sessions it runs in.
- **With this change.** All 35 tests in `CometTemporalExpressionSuite` pass on Spark 4.1, including the `spotless` and `scalastyle` checks.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348#issuecomment-5997658144
Thanks @parthchandra, I added that note to the `CometDays` doc comment in 536b259da. The edge turned out to be narrower than any pre-1970 timestamp ending in 999999. Iceberg's wrap moves the instant back to the start of its second, so the day only changes when that second starts at midnight, for example `1969-01-01 00:00:00.999999`, which Iceberg puts on 1968-12-31. That's the same check `iceberg_div_floor` makes. `CometHours` floors the same way, so the comment says that too.
I also merged main, which only conflicted on the `datetime.scala` imports, and added `run-iceberg-tests`.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348#issuecomment-6027934155
Thanks for the reviews @parthchandra @comphead @sunchao. Added this to merge queue
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348#discussion_r4149062806
##########
spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala:
##########
@@ -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)"))
Review Comment:
Added in 899cb5a53: `Days(Literal(-1L, TimestampType))` under `Asia/Tokyo`, expecting `-1`. With the serde switched back to the session timezone it fails with `[0]` vs `[-1]`, and it passes with this change. One detail: `SparkFunSuite` pins the JVM default zone to `America/Los_Angeles`, so that block runs in LA rather than UTC on CI. Neither literal is near an LA day boundary, so as you expected it passed with the old serde too.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348#discussion_r4146851153
##########
spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala:
##########
@@ -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)"))
Review Comment:
Nit: this block uses the default session zone (I'd expect UTC on CI), so it probably passes with the old serde too, and no timestamp in the `days` tests is before 1970 (the fuzz data sit around year 3333). Would it make sense to add one check under `Asia/Tokyo` with a pre-epoch instant built from micros, such as `Days(Literal(-1L, DataTypes.TimestampType))` with expected `-1`? The old serde should return `0` there, and it would pin flooring for negative values.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348#issuecomment-5919289158
The one-line UTC change matches iceberg-java (`Timestamps.DAY` measures from a UTC epoch), and it doesn't conflict with #6456 (different code path). One note:
- **`spark/src/main/scala/org/apache/comet/serde/datetime.scala:850`** — for pre-1970 timestamps whose microsecond-of-second is 999999, this true-floor UTC cast lands one day off from iceberg's `microsToDays`, which is what #6456 corrects on the kernel path. Since #6456 doesn't touch this cast route, please add a short note that `CometDays` stays a true floor and mention that edge, so the two "days" paths' behavior is documented rather than surprising later.
Because this changes a transform Iceberg partitions by, worth applying `run-iceberg-tests` or running a local 3.4/3.5 job before merge.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: count the days partition transform in UTC [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #6348:
URL: https://github.com/apache/datafusion-comet/pull/6348
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org