Repository navigation
Conversation
MaxGekk
commented
Nov 11, 2020
| ------------------------------------------------------------------------------------------------------------------------ | ||
| date_trunc MINUTE wholestage off 400 419 26 25.0 40.0 1.0X | ||
| date_trunc MINUTE wholestage on 401 405 4 24.9 40.1 1.0X | ||
| date_trunc MINUTE wholestage off 2230 2238 11 4.5 223.0 1.0X |
Member
Author
There was a problem hiding this comment.
Slow down is around 5.5 times.
dongjoon-hyun
approved these changes
Nov 11, 2020
dongjoon-hyun
left a comment
Member
There was a problem hiding this comment.
+1, LGTM. Thank you for updates, @MaxGekk .
Merged to master.
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #130945 has finished for PR 30338 at commit
|
Member
|
late LGTM. Thanks for the update, Max. |
cloud-fan
pushed a commit
that referenced
this pull request
May 7, 2026
Closes [SPARK-56663](https://issues.apache.org/jira/browse/SPARK-56663). ### What changes were proposed in this pull request? This PR re-introduces a fast path in `DateTimeUtils.truncTimestamp` for `TRUNC_TO_MINUTE`, `TRUNC_TO_HOUR`, and `TRUNC_TO_DAY`. A new private `truncToUnitFast(micros, zoneId, unitMicros, fallbackUnit)` helper: 1. Resolves the zone offset for the input instant once (`zoneId.getRules.getOffset(...)`). 2. Truncates by `Math.floorMod` in local time: `local = micros + offsetMicros`, then `truncatedLocal = local - floorMod(local, unitMicros)`, then back: `candidate = truncatedLocal - offsetMicros`. 3. Re-checks the offset at `candidate` and falls back to the existing `truncatedTo()`-based slow path if it differs from the original offset (the SPARK-30766 / SPARK-30857 DST guard). 4. The DST re-check is skipped for `ZoneRules.isFixedOffset` zones. 5. `Math.addExact` / `subtractExact` guard the offset arithmetic; on overflow we also fall back to the slow path. The arithmetic also handles sub-minute LMT offsets (e.g. America/Los_Angeles `-07:52:58` pre-1883 - the SPARK-33404 case) and 30/45-minute zones (Asia/Kolkata `+05:30`, Asia/Kathmandu `+05:45`) without an offset-alignment guard, because the offset is applied as part of the floorMod. Date-level units (WEEK/MONTH/QUARTER/YEAR + `trunc(date, ...)`) are not touched in this PR; they still go through `microsToDays` / `daysToMicros`. Those can follow up in a separate ticket. ### Why are the changes needed? SPARK-33404 (Nov 2020) fixed a correctness bug - `date_trunc('minute', '1769-10-17 17:10:02')` in `America/Los_Angeles` returned the original timestamp because the existing fast path ignored the time-zone offset and the LMT offset has second granularity. The fix routed MINUTE/HOUR/DAY through `microsToInstant().atZone(zoneId).truncatedTo(unit)`, which allocates `ZonedDateTime` per row. The author of that fix flagged the regression on the follow-up benchmark PR: > "Slow down is around 5.5 times" - MaxGekk, #30338 That regression has stood for ~5.5 years with no follow-up ticket. This PR restores most of the lost throughput without re-introducing the LMT bug, by applying the offset before/after the floorMod and verifying the offset is unchanged at the truncated candidate (handling DST transitions that motivated SPARK-30766 / SPARK-30857). `DateTimeBenchmark` Truncation results, wholestage on, ns/row on a 12th Gen Intel i7-1260P: | level | baseline | this PR | speedup | |---------|---------:|--------:|--------:| | MINUTE | 136.0 | 69.5 | 1.96× | | HOUR | 136.2 | 69.8 | 1.95× | | DAY | 136.5 | 69.8 | 1.96× | | DD | 136.7 | 70.1 | 1.95× | Other levels are unchanged (within noise). ### Does this PR introduce _any_ user-facing change? No. The output of `date_trunc` for `MINUTE`/`HOUR`/`DAY` is identical to master in all cases; only the internal implementation changes. ### How was this patch tested? - `DateTimeUtilsSuite` - all 63 tests pass, including: - `SPARK-33404: test truncTimestamp when time zone offset from UTC has a granularity of seconds`, which loops MINUTE/SECOND/MILLI/MICRO truncation across **every** available zone (covers historical LMT zones). - The existing `truncTimestamp` test, which loops HOUR/DAY/MINUTE/etc. across every available zone. - Five new tests added in this PR: - `truncTimestamp with sub-hour zone offsets` - Asia/Kolkata `+05:30` and Asia/Kathmandu `+05:45` HOUR/DAY truncation. - `truncTimestamp across DST transitions` - America/Los_Angeles spring-forward (2024-03-10) and fall-back (2024-11-03) for HOUR/DAY, exercising the offset-equality fallback. - `SPARK-30766/30857: truncTimestamp before the epoch in HOUR/DAY` - pre-1970 instant in America/Los_Angeles for HOUR and DAY. - `truncTimestamp at America/Sao_Paulo midnight DST gap` - 2018-11-04 (the last Brazilian DST start, jumping from -3 to -2 at exactly midnight local), exercising the case where the local day boundary itself falls in a gap and DAY truncation must resolve forward. - `truncTimestamp at Pacific/Apia after the 2011 calendar shift` - mid-2012 instant in a +13:00 zone whose history skipped a calendar date, verifying the fast path under a non-DST historical offset change. - `DateExpressionsSuite` - all 68 tests pass. - `DateTimeBenchmark` re-run locally with the numbers shown above. Per the contributor guide, the official `benchmarks/DateTimeBenchmark-results.txt` should be regenerated on the standard CI runner - happy to do that via the GitHub Actions benchmark workflow on request. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored by Claude Code. Closes #55610 from Licht-T/date-trunc-fastpath. Authored-by: Rito Takeuchi <licht-t@outlook.jp> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
cloud-fan
pushed a commit
that referenced
this pull request
May 7, 2026
Closes [SPARK-56663](https://issues.apache.org/jira/browse/SPARK-56663). ### What changes were proposed in this pull request? This PR re-introduces a fast path in `DateTimeUtils.truncTimestamp` for `TRUNC_TO_MINUTE`, `TRUNC_TO_HOUR`, and `TRUNC_TO_DAY`. A new private `truncToUnitFast(micros, zoneId, unitMicros, fallbackUnit)` helper: 1. Resolves the zone offset for the input instant once (`zoneId.getRules.getOffset(...)`). 2. Truncates by `Math.floorMod` in local time: `local = micros + offsetMicros`, then `truncatedLocal = local - floorMod(local, unitMicros)`, then back: `candidate = truncatedLocal - offsetMicros`. 3. Re-checks the offset at `candidate` and falls back to the existing `truncatedTo()`-based slow path if it differs from the original offset (the SPARK-30766 / SPARK-30857 DST guard). 4. The DST re-check is skipped for `ZoneRules.isFixedOffset` zones. 5. `Math.addExact` / `subtractExact` guard the offset arithmetic; on overflow we also fall back to the slow path. The arithmetic also handles sub-minute LMT offsets (e.g. America/Los_Angeles `-07:52:58` pre-1883 - the SPARK-33404 case) and 30/45-minute zones (Asia/Kolkata `+05:30`, Asia/Kathmandu `+05:45`) without an offset-alignment guard, because the offset is applied as part of the floorMod. Date-level units (WEEK/MONTH/QUARTER/YEAR + `trunc(date, ...)`) are not touched in this PR; they still go through `microsToDays` / `daysToMicros`. Those can follow up in a separate ticket. ### Why are the changes needed? SPARK-33404 (Nov 2020) fixed a correctness bug - `date_trunc('minute', '1769-10-17 17:10:02')` in `America/Los_Angeles` returned the original timestamp because the existing fast path ignored the time-zone offset and the LMT offset has second granularity. The fix routed MINUTE/HOUR/DAY through `microsToInstant().atZone(zoneId).truncatedTo(unit)`, which allocates `ZonedDateTime` per row. The author of that fix flagged the regression on the follow-up benchmark PR: > "Slow down is around 5.5 times" - MaxGekk, #30338 That regression has stood for ~5.5 years with no follow-up ticket. This PR restores most of the lost throughput without re-introducing the LMT bug, by applying the offset before/after the floorMod and verifying the offset is unchanged at the truncated candidate (handling DST transitions that motivated SPARK-30766 / SPARK-30857). `DateTimeBenchmark` Truncation results, wholestage on, ns/row on a 12th Gen Intel i7-1260P: | level | baseline | this PR | speedup | |---------|---------:|--------:|--------:| | MINUTE | 136.0 | 69.5 | 1.96× | | HOUR | 136.2 | 69.8 | 1.95× | | DAY | 136.5 | 69.8 | 1.96× | | DD | 136.7 | 70.1 | 1.95× | Other levels are unchanged (within noise). ### Does this PR introduce _any_ user-facing change? No. The output of `date_trunc` for `MINUTE`/`HOUR`/`DAY` is identical to master in all cases; only the internal implementation changes. ### How was this patch tested? - `DateTimeUtilsSuite` - all 63 tests pass, including: - `SPARK-33404: test truncTimestamp when time zone offset from UTC has a granularity of seconds`, which loops MINUTE/SECOND/MILLI/MICRO truncation across **every** available zone (covers historical LMT zones). - The existing `truncTimestamp` test, which loops HOUR/DAY/MINUTE/etc. across every available zone. - Five new tests added in this PR: - `truncTimestamp with sub-hour zone offsets` - Asia/Kolkata `+05:30` and Asia/Kathmandu `+05:45` HOUR/DAY truncation. - `truncTimestamp across DST transitions` - America/Los_Angeles spring-forward (2024-03-10) and fall-back (2024-11-03) for HOUR/DAY, exercising the offset-equality fallback. - `SPARK-30766/30857: truncTimestamp before the epoch in HOUR/DAY` - pre-1970 instant in America/Los_Angeles for HOUR and DAY. - `truncTimestamp at America/Sao_Paulo midnight DST gap` - 2018-11-04 (the last Brazilian DST start, jumping from -3 to -2 at exactly midnight local), exercising the case where the local day boundary itself falls in a gap and DAY truncation must resolve forward. - `truncTimestamp at Pacific/Apia after the 2011 calendar shift` - mid-2012 instant in a +13:00 zone whose history skipped a calendar date, verifying the fast path under a non-DST historical offset change. - `DateExpressionsSuite` - all 68 tests pass. - `DateTimeBenchmark` re-run locally with the numbers shown above. Per the contributor guide, the official `benchmarks/DateTimeBenchmark-results.txt` should be regenerated on the standard CI runner - happy to do that via the GitHub Actions benchmark workflow on request. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored by Claude Code. Closes #55610 from Licht-T/date-trunc-fastpath. Authored-by: Rito Takeuchi <licht-t@outlook.jp> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d6a39df) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
cloud-fan
pushed a commit
that referenced
this pull request
May 7, 2026
Closes [SPARK-56663](https://issues.apache.org/jira/browse/SPARK-56663). ### What changes were proposed in this pull request? This PR re-introduces a fast path in `DateTimeUtils.truncTimestamp` for `TRUNC_TO_MINUTE`, `TRUNC_TO_HOUR`, and `TRUNC_TO_DAY`. A new private `truncToUnitFast(micros, zoneId, unitMicros, fallbackUnit)` helper: 1. Resolves the zone offset for the input instant once (`zoneId.getRules.getOffset(...)`). 2. Truncates by `Math.floorMod` in local time: `local = micros + offsetMicros`, then `truncatedLocal = local - floorMod(local, unitMicros)`, then back: `candidate = truncatedLocal - offsetMicros`. 3. Re-checks the offset at `candidate` and falls back to the existing `truncatedTo()`-based slow path if it differs from the original offset (the SPARK-30766 / SPARK-30857 DST guard). 4. The DST re-check is skipped for `ZoneRules.isFixedOffset` zones. 5. `Math.addExact` / `subtractExact` guard the offset arithmetic; on overflow we also fall back to the slow path. The arithmetic also handles sub-minute LMT offsets (e.g. America/Los_Angeles `-07:52:58` pre-1883 - the SPARK-33404 case) and 30/45-minute zones (Asia/Kolkata `+05:30`, Asia/Kathmandu `+05:45`) without an offset-alignment guard, because the offset is applied as part of the floorMod. Date-level units (WEEK/MONTH/QUARTER/YEAR + `trunc(date, ...)`) are not touched in this PR; they still go through `microsToDays` / `daysToMicros`. Those can follow up in a separate ticket. ### Why are the changes needed? SPARK-33404 (Nov 2020) fixed a correctness bug - `date_trunc('minute', '1769-10-17 17:10:02')` in `America/Los_Angeles` returned the original timestamp because the existing fast path ignored the time-zone offset and the LMT offset has second granularity. The fix routed MINUTE/HOUR/DAY through `microsToInstant().atZone(zoneId).truncatedTo(unit)`, which allocates `ZonedDateTime` per row. The author of that fix flagged the regression on the follow-up benchmark PR: > "Slow down is around 5.5 times" - MaxGekk, #30338 That regression has stood for ~5.5 years with no follow-up ticket. This PR restores most of the lost throughput without re-introducing the LMT bug, by applying the offset before/after the floorMod and verifying the offset is unchanged at the truncated candidate (handling DST transitions that motivated SPARK-30766 / SPARK-30857). `DateTimeBenchmark` Truncation results, wholestage on, ns/row on a 12th Gen Intel i7-1260P: | level | baseline | this PR | speedup | |---------|---------:|--------:|--------:| | MINUTE | 136.0 | 69.5 | 1.96× | | HOUR | 136.2 | 69.8 | 1.95× | | DAY | 136.5 | 69.8 | 1.96× | | DD | 136.7 | 70.1 | 1.95× | Other levels are unchanged (within noise). ### Does this PR introduce _any_ user-facing change? No. The output of `date_trunc` for `MINUTE`/`HOUR`/`DAY` is identical to master in all cases; only the internal implementation changes. ### How was this patch tested? - `DateTimeUtilsSuite` - all 63 tests pass, including: - `SPARK-33404: test truncTimestamp when time zone offset from UTC has a granularity of seconds`, which loops MINUTE/SECOND/MILLI/MICRO truncation across **every** available zone (covers historical LMT zones). - The existing `truncTimestamp` test, which loops HOUR/DAY/MINUTE/etc. across every available zone. - Five new tests added in this PR: - `truncTimestamp with sub-hour zone offsets` - Asia/Kolkata `+05:30` and Asia/Kathmandu `+05:45` HOUR/DAY truncation. - `truncTimestamp across DST transitions` - America/Los_Angeles spring-forward (2024-03-10) and fall-back (2024-11-03) for HOUR/DAY, exercising the offset-equality fallback. - `SPARK-30766/30857: truncTimestamp before the epoch in HOUR/DAY` - pre-1970 instant in America/Los_Angeles for HOUR and DAY. - `truncTimestamp at America/Sao_Paulo midnight DST gap` - 2018-11-04 (the last Brazilian DST start, jumping from -3 to -2 at exactly midnight local), exercising the case where the local day boundary itself falls in a gap and DAY truncation must resolve forward. - `truncTimestamp at Pacific/Apia after the 2011 calendar shift` - mid-2012 instant in a +13:00 zone whose history skipped a calendar date, verifying the fast path under a non-DST historical offset change. - `DateExpressionsSuite` - all 68 tests pass. - `DateTimeBenchmark` re-run locally with the numbers shown above. Per the contributor guide, the official `benchmarks/DateTimeBenchmark-results.txt` should be regenerated on the standard CI runner - happy to do that via the GitHub Actions benchmark workflow on request. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored by Claude Code. Closes #55610 from Licht-T/date-trunc-fastpath. Authored-by: Rito Takeuchi <licht-t@outlook.jp> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d6a39df) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
MaxGekk
pushed a commit
that referenced
this pull request
Jun 23, 2026
### What changes were proposed in this pull request? This PR extends the offset-arithmetic + DST-equality-guard fast path introduced in SPARK-56663 from MIN/HR/DAY to the date-level units WEEK / MONTH / QUARTER / YEAR. The framework for offset-based truncation -- resolve offset once, apply, truncate in the local frame, re-apply, DST guard, fall back on DST-cross or arithmetic overflow -- is identical for every level above SECOND. Only the "truncate in local frame" step varies. This PR inlines SPARK-56663's `truncToUnitFast` together with the new date-level path directly into `truncTimestamp`, and keeps a single private `truncTimestampSlow` as a complete reference implementation that the fast path falls back to: ```scala def truncTimestamp(micros: Long, level: Int, zoneId: ZoneId): Long = { // MICROSECOND / MILLISECOND / SECOND short-circuits (no zone work). // Offset arithmetic for every other level. // DST guard, fallback to truncTimestampSlow. } private def truncTimestampSlow(micros: Long, level: Int, zoneId: ZoneId): Long ``` The local-frame truncation step is the only thing the fast path branches on: - `MICROSECOND` / `MILLISECOND` / `SECOND` - pure UTC `floorMod` (zone offsets have at most second precision per `java.time.ZoneOffset`; no zone information needed). - `MINUTE` / `HOUR` / `DAY` - shifted-local `floorMod` against the unit micros. - `WEEK` / `MONTH` / `QUARTER` / `YEAR` - compute local epoch-day by integer division, run `truncDate` in the local-day frame, multiply back to local micros. Everything else (offset resolve via `rules.getOffset`, `addExact` / `subtractExact`, DST guard via offset-equality at the candidate, slow-path fallback) is shared. The DST guard fires correctly for the new date-level cases - for example, YEAR truncation of a March instant in `America/Los_Angeles` produces a candidate at Jan 1 (which is in PST, offset -8) while the original is in PDT (offset -7); the offsets differ, so the path falls back to the slow `microsToDays` / `daysToMicros` route which uses `ZonedDateTime.resolveLocal` to land on Jan 1 00:00 PST. This PR also rewrites `TRUNC_TO_QUARTER` from `IsoFields.DAY_OF_QUARTER` (a `TemporalAdjuster` that produces a fresh `LocalDate`) to a direct `withMonth(firstMonthOfQuarter).withDayOfMonth(1)` chain on the existing `LocalDate`. Saves one allocation + the adjuster overhead per call. `truncTimestampSlow` covers every level explicitly so it serves as a self-contained reference implementation - the fast path's correctness can be verified against it case-by-case. ### Why are the changes needed? SPARK-33404 (Nov 2020) routed every `date_trunc` level above SECOND through `microsToInstant().atZone(zoneId).truncatedTo(unit)` for correctness, costing ~5.5× throughput per the follow-up benchmark PR (#30338). SPARK-56663 recovered most of that for MIN/HR/DAY using the offset-arithmetic + DST-guard pattern. This PR extends the same recovery to WEEK / MONTH / QUARTER / YEAR - the levels that drive monthly/quarterly aggregations in analytics workloads. `DateTimeBenchmark` Truncation results, wholestage on, ns/row on a 12th Gen Intel i7-1260P (master = pre-SPARK-56663): | level | master baseline | this PR | speedup | |---------|---------------:|--------:|--------:| | WEEK | 165.2 | 78.2 | 2.11× | | MONTH | 181.9 | 92.2 | 1.97× | | MM | 182.2 | 92.5 | 1.97× | | MON | 182.9 | 92.7 | 1.97× | | QUARTER | 216.8 | 108.8 | 1.99× | | YEAR | 205.2 | 96.7 | 2.12× | | YYYY | 205.8 | 96.9 | 2.12× | | YY | 206.3 | 96.0 | 2.15× | Time-level units (MIN/HR/DAY/SECOND) and `trunc(date, ...)` are unchanged within noise; the hot path for those levels is byte-identical to SPARK-56663 after the unification. ### Does this PR introduce _any_ user-facing change? No. The output of `date_trunc` is identical to master in all cases, including DST-spanning truncations (verified by the offset-equality guard + slow-path fallback, plus the new tests). Only the internal implementation changes. ### How was this patch tested? - `DateTimeUtilsSuite` - all 66 tests pass, including: - `SPARK-33404: test truncTimestamp when time zone offset from UTC has a granularity of seconds`, extended to also exercise WEEK / MONTH / QUARTER / YEAR with the 1769-10-17 LMT timestamp across **every** available zone (the existing loop already covered SECOND/MILLI/MICRO; SPARK-56663 added HOUR/DAY; this PR completes the matrix). - The existing `truncTimestamp` test, which loops WEEK / MONTH / QUARTER / YEAR for 2015 timestamps across every zone. - New test `truncTimestamp date-level units across DST boundaries` - covers YEAR / QUARTER truncation that crosses the LA spring-forward (DST guard fires, fallback path runs) and MONTH truncation entirely within DST (fast path stays). - `DateExpressionsSuite` - all tests pass (no changes to expression-level code, only the underlying `DateTimeUtils` helpers). - `DateTimeBenchmark` re-run via the GitHub Actions `Run benchmarks` workflow on this fork for JDK 17, 21, and 25; results committed back to the branch. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored with Claude Code. Closes #55736 from Licht-T/date-trunc-fastpath-phase3. Authored-by: Rito Takeuchi <licht-t@outlook.jp> Signed-off-by: Max Gekk <max.gekk@gmail.com>
MaxGekk
pushed a commit
that referenced
this pull request
Jun 23, 2026
### What changes were proposed in this pull request? This PR extends the offset-arithmetic + DST-equality-guard fast path introduced in SPARK-56663 from MIN/HR/DAY to the date-level units WEEK / MONTH / QUARTER / YEAR. The framework for offset-based truncation -- resolve offset once, apply, truncate in the local frame, re-apply, DST guard, fall back on DST-cross or arithmetic overflow -- is identical for every level above SECOND. Only the "truncate in local frame" step varies. This PR inlines SPARK-56663's `truncToUnitFast` together with the new date-level path directly into `truncTimestamp`, and keeps a single private `truncTimestampSlow` as a complete reference implementation that the fast path falls back to: ```scala def truncTimestamp(micros: Long, level: Int, zoneId: ZoneId): Long = { // MICROSECOND / MILLISECOND / SECOND short-circuits (no zone work). // Offset arithmetic for every other level. // DST guard, fallback to truncTimestampSlow. } private def truncTimestampSlow(micros: Long, level: Int, zoneId: ZoneId): Long ``` The local-frame truncation step is the only thing the fast path branches on: - `MICROSECOND` / `MILLISECOND` / `SECOND` - pure UTC `floorMod` (zone offsets have at most second precision per `java.time.ZoneOffset`; no zone information needed). - `MINUTE` / `HOUR` / `DAY` - shifted-local `floorMod` against the unit micros. - `WEEK` / `MONTH` / `QUARTER` / `YEAR` - compute local epoch-day by integer division, run `truncDate` in the local-day frame, multiply back to local micros. Everything else (offset resolve via `rules.getOffset`, `addExact` / `subtractExact`, DST guard via offset-equality at the candidate, slow-path fallback) is shared. The DST guard fires correctly for the new date-level cases - for example, YEAR truncation of a March instant in `America/Los_Angeles` produces a candidate at Jan 1 (which is in PST, offset -8) while the original is in PDT (offset -7); the offsets differ, so the path falls back to the slow `microsToDays` / `daysToMicros` route which uses `ZonedDateTime.resolveLocal` to land on Jan 1 00:00 PST. This PR also rewrites `TRUNC_TO_QUARTER` from `IsoFields.DAY_OF_QUARTER` (a `TemporalAdjuster` that produces a fresh `LocalDate`) to a direct `withMonth(firstMonthOfQuarter).withDayOfMonth(1)` chain on the existing `LocalDate`. Saves one allocation + the adjuster overhead per call. `truncTimestampSlow` covers every level explicitly so it serves as a self-contained reference implementation - the fast path's correctness can be verified against it case-by-case. ### Why are the changes needed? SPARK-33404 (Nov 2020) routed every `date_trunc` level above SECOND through `microsToInstant().atZone(zoneId).truncatedTo(unit)` for correctness, costing ~5.5× throughput per the follow-up benchmark PR (#30338). SPARK-56663 recovered most of that for MIN/HR/DAY using the offset-arithmetic + DST-guard pattern. This PR extends the same recovery to WEEK / MONTH / QUARTER / YEAR - the levels that drive monthly/quarterly aggregations in analytics workloads. `DateTimeBenchmark` Truncation results, wholestage on, ns/row on a 12th Gen Intel i7-1260P (master = pre-SPARK-56663): | level | master baseline | this PR | speedup | |---------|---------------:|--------:|--------:| | WEEK | 165.2 | 78.2 | 2.11× | | MONTH | 181.9 | 92.2 | 1.97× | | MM | 182.2 | 92.5 | 1.97× | | MON | 182.9 | 92.7 | 1.97× | | QUARTER | 216.8 | 108.8 | 1.99× | | YEAR | 205.2 | 96.7 | 2.12× | | YYYY | 205.8 | 96.9 | 2.12× | | YY | 206.3 | 96.0 | 2.15× | Time-level units (MIN/HR/DAY/SECOND) and `trunc(date, ...)` are unchanged within noise; the hot path for those levels is byte-identical to SPARK-56663 after the unification. ### Does this PR introduce _any_ user-facing change? No. The output of `date_trunc` is identical to master in all cases, including DST-spanning truncations (verified by the offset-equality guard + slow-path fallback, plus the new tests). Only the internal implementation changes. ### How was this patch tested? - `DateTimeUtilsSuite` - all 66 tests pass, including: - `SPARK-33404: test truncTimestamp when time zone offset from UTC has a granularity of seconds`, extended to also exercise WEEK / MONTH / QUARTER / YEAR with the 1769-10-17 LMT timestamp across **every** available zone (the existing loop already covered SECOND/MILLI/MICRO; SPARK-56663 added HOUR/DAY; this PR completes the matrix). - The existing `truncTimestamp` test, which loops WEEK / MONTH / QUARTER / YEAR for 2015 timestamps across every zone. - New test `truncTimestamp date-level units across DST boundaries` - covers YEAR / QUARTER truncation that crosses the LA spring-forward (DST guard fires, fallback path runs) and MONTH truncation entirely within DST (fast path stays). - `DateExpressionsSuite` - all tests pass (no changes to expression-level code, only the underlying `DateTimeUtils` helpers). - `DateTimeBenchmark` re-run via the GitHub Actions `Run benchmarks` workflow on this fork for JDK 17, 21, and 25; results committed back to the branch. ### Was this patch authored or co-authored using generative AI tooling? Yes, co-authored with Claude Code. Closes #55736 from Licht-T/date-trunc-fastpath-phase3. Authored-by: Rito Takeuchi <licht-t@outlook.jp> Signed-off-by: Max Gekk <max.gekk@gmail.com> (cherry picked from commit 3dc736c) Signed-off-by: Max Gekk <max.gekk@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Updated results of
DateTimeBenchmarkin the environment:sudo add-apt-repository ppa:openjdk-r/ppa&sudo apt install openjdk-11-jdkWhy are the changes needed?
The fix #30303 slowed down
date_trunc. This PR updates benchmark results to have actual info about performance ofdate_trunc.Does this PR introduce any user-facing change?
No
How was this patch tested?
By regenerating benchmark results: