feat: Update timezone data to 2025b#26981
Merged
tdcmeehan merged 2 commits intoprestodb:masterfrom Feb 27, 2026
Merged
Conversation
rdtr
requested review from
a team,
czentgr,
elharo and
unidevel
as code owners
Contributor
Reviewer's GuideUpdates Presto’s timezone data to tzdata 2025b by bumping Joda-Time and adding the America/Coyhaique zone, adjusts the checksum-based regression test for zone-index, and refreshes all GitHub Actions workflows to use Temurin JDK 17.0.16 due to an upstream JDK issue. Sequence diagram for timezone resolution with updated tzdata_2025bsequenceDiagram
actor User
participant PrestoClient
participant PrestoCoordinator
participant TimeZoneKey
participant JodaTimeTimeZoneData
User->>PrestoClient: Submit query (timestamp, America/Coyhaique)
PrestoClient->>PrestoCoordinator: Send query
PrestoCoordinator->>TimeZoneKey: lookupTimeZoneKey(America/Coyhaique)
TimeZoneKey->>JodaTimeTimeZoneData: resolveZoneId(America/Coyhaique)
JodaTimeTimeZoneData-->>TimeZoneKey: zoneId(2234, tzdata 2025b rules)
TimeZoneKey-->>PrestoCoordinator: TimeZoneKey(2234)
PrestoCoordinator->>PrestoCoordinator: Evaluate expressions with timezone
PrestoCoordinator-->>PrestoClient: Query results
PrestoClient-->>User: Display timestamps in America/Coyhaique
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
7 tasks
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Given how many workflows hardcode the Java patch version, consider centralizing the Java version (e.g., via a shared reusable workflow, workflow-level env, or matrix variable) so future patch bumps only need to be changed in one place.
- The
zone-index.propertiesand associated checksum assertion are guarded by a strong "should not (normally) be changed" comment; it might be helpful to update that comment (or add a short note nearby) to explicitly call out the Joda/tzdata upgrade procedure, including updatingzone-index.propertiesand the expected hash.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Given how many workflows hardcode the Java patch version, consider centralizing the Java version (e.g., via a shared reusable workflow, workflow-level env, or matrix variable) so future patch bumps only need to be changed in one place.
- The `zone-index.properties` and associated checksum assertion are guarded by a strong "should not (normally) be changed" comment; it might be helpful to update that comment (or add a short note nearby) to explicitly call out the Joda/tzdata upgrade procedure, including updating `zone-index.properties` and the expected hash.
## Individual Comments
### Comment 1
<location> `presto-common/src/test/java/com/facebook/presto/common/type/TestTimeZoneKey.java:218-219` </location>
<code_context>
hasher.putString(timeZoneKey.getId(), StandardCharsets.UTF_8);
}
// Zone file should not (normally) be changed, so let's make this more difficult
- assertEquals(hasher.hash().asLong(), 4825838578917475630L, "zone-index.properties file contents changed!");
+ assertEquals(hasher.hash().asLong(), 3765670086753811806L, "zone-index.properties file contents changed!");
</code_context>
<issue_to_address>
**suggestion (testing):** Consider making the checksum assertion failure more actionable for future tzdata updates
Since tzdata updates like this are expected periodically, this assertion may fail without clearly telling the updater what to do. Consider either:
- Expanding the assertion message to note that the expected checksum should be updated after tzdata/Joda-Time changes, or
- Adding a brief comment on how to recompute the checksum (algorithm or helper method), so future maintainers don’t have to rediscover the process.
```suggestion
// Zone file should not (normally) be changed, so let's make this more difficult.
// If this assertion fails after an intentional tzdata/Joda-Time update that changes
// zone-index.properties, recompute the expected checksum by running this test and
// updating the constant below with the new value from hasher.hash().asLong().
assertEquals(
hasher.hash().asLong(),
3765670086753811806L,
"zone-index.properties file contents changed. If this is due to an intentional tzdata/Joda-Time or zone-index.properties update, update the expected checksum to match hasher.hash().asLong().");
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
218
to
+219
| // Zone file should not (normally) be changed, so let's make this more difficult | ||
| assertEquals(hasher.hash().asLong(), 4825838578917475630L, "zone-index.properties file contents changed!"); | ||
| assertEquals(hasher.hash().asLong(), 3765670086753811806L, "zone-index.properties file contents changed!"); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider making the checksum assertion failure more actionable for future tzdata updates
Since tzdata updates like this are expected periodically, this assertion may fail without clearly telling the updater what to do. Consider either:
- Expanding the assertion message to note that the expected checksum should be updated after tzdata/Joda-Time changes, or
- Adding a brief comment on how to recompute the checksum (algorithm or helper method), so future maintainers don’t have to rediscover the process.
Suggested change
| // Zone file should not (normally) be changed, so let's make this more difficult | |
| assertEquals(hasher.hash().asLong(), 4825838578917475630L, "zone-index.properties file contents changed!"); | |
| assertEquals(hasher.hash().asLong(), 3765670086753811806L, "zone-index.properties file contents changed!"); | |
| // Zone file should not (normally) be changed, so let's make this more difficult. | |
| // If this assertion fails after an intentional tzdata/Joda-Time update that changes | |
| // zone-index.properties, recompute the expected checksum by running this test and | |
| // updating the constant below with the new value from hasher.hash().asLong(). | |
| assertEquals( | |
| hasher.hash().asLong(), | |
| 3765670086753811806L, | |
| "zone-index.properties file contents changed. If this is due to an intentional tzdata/Joda-Time or zone-index.properties update, update the expected checksum to match hasher.hash().asLong()."); |
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
from
7a0e228 to
19d5d9f
Compare
Contributor
|
@rdtr Seems some CI not running, can you rebase to try gain? |
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
from
19d5d9f to
3816cb8
Compare
Contributor
Author
Contributor
|
@rdtr I guess this is a bug of github, since you upgraded JDK to 17.0.16, you probably can continue working on the original PR, and ignore the pending tests related to 17.0.15. Once reviewed and approved, you may ask admin like @tdcmeehan to merge this without full CI passed. |
Contributor
|
I'm actually a little worried this isn't transient, since it's happened for two PRs in a row. @rdtr can you try to quickly revert to 15 to see if the CI passes? If so, it would seem to be specific to the version upgrade. |
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
2 times, most recently
from
eff8774 to
54f7367
Compare
Contributor
Author
|
@tdcmeehan |
Contributor
|
@rdtr could you try to restore the JDK changes, but try a different distribution, to see if it's localized to the distribution? |
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
7 times, most recently
from
399cddf to
d1f2ebb
Compare
Contributor
Author
|
@tdcmeehan |
Contributor
|
@rdtr thanks for your patience, I see the issue now. To make this easier for the next timezone update, could you change our workflow files to change: |
7 tasks
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
from
d1f2ebb to
63022b4
Compare
tdcmeehan
previously requested changes
Feb 12, 2026
Contributor
tdcmeehan
left a comment
tdcmeehan
left a comment
There was a problem hiding this comment.
LGTM % the accidental file inclusion
.claude/settings.local.json
Outdated
| @@ -0,0 +1,8 @@ | |||
| { | |||
Contributor
There was a problem hiding this comment.
Please revert this and consider adding .claude to the gitignore file.
Contributor
Author
There was a problem hiding this comment.
Thanks for the catch, I removed it.
Contributor
Author
There was a problem hiding this comment.
@tdcmeehan It seems using 17 does not work? Many checks are failing.
Contributor
There was a problem hiding this comment.
It works, I just need to update our branch protection rules, will get to it soon.
rdtr
force-pushed
the
update-tzdata-2025b-v2
branch
from
63022b4 to
f1d7219
Compare
- Update Joda-Time from 2.13.1 to 2.14.0 (includes tzdata 2025b) - Add America/Coyhaique timezone (new zone for Chile's Aysén Region) - Update zone-index.properties checksum in test
tdcmeehan
force-pushed
the
update-tzdata-2025b-v2
branch
from
f1d7219 to
fcabced
Compare
Contributor
|
@rdtr I'm trying to fix the product tests, I think the JDK upgrade also made some changes to SSL. Let's see if CI can get green now. |
Contributor
Author
|
Thanks. Please let me know if anything needs to be done on my end. |
tdcmeehan
approved these changes
Feb 27, 2026
n0r0shi
added a commit
to n0r0shi/velox
that referenced
this pull request
Mar 7, 2026
Add America/Coyhaique (timezone ID 2234) to the timezone database and timezone names, following the Presto zone-index convention. Also fix the namespace in gen_timezone_database.py (util -> tz) to match the current codebase. America/Coyhaique was added to the IANA timezone database in tzdata 2025b. The upstream Presto PR (prestodb/presto#26981) has been merged. Fixes facebookincubator#16216
This was referenced Mar 31, 2026
15 tasks
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
3 participants
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.
This is a replacement for #26856. The CI was having persistent issues running tests on the original PR even after several rebases, so I am opening a fresh PR.
Description
This PR updates the timezone data to 2025b by upgrading Joda-Time from 2.13.1 to 2.14.0. The update adds the new
America/Coyhaiquetimezone (zone key 2234), which was introduced for Chile's Aysén Region. The zone-index.properties checksum in TestTimeZoneKey has been updated to reflect the new zone entry.Motivation and Context
Timezone databases are periodically updated to reflect changes in timezone rules around the world. The tzdata 2025b release includes the new America/Coyhaique zone for Chile's Aysén Region. Keeping Presto's timezone data current ensures accurate timestamp handling for users in affected regions.
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.