-
Notifications
You must be signed in to change notification settings - Fork 5.5k
feat: Update timezone data to 2025b #26856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates Presto’s bundled timezone data to tzdata 2025b by bumping the Joda-Time dependency, adding the new America/Coyhaique zone entry to zone-index.properties, and updating the checksum-based regression test accordingly. Sequence diagram for handling queries with America_Coyhaique timezonesequenceDiagram
actor User
participant PrestoCLI
participant PrestoCoordinator
participant TimeZoneKey
participant ZoneIndexProperties
participant JodaTime
User->>PrestoCLI: Submit query with timezone America/Coyhaique
PrestoCLI->>PrestoCoordinator: Send query text and session properties
PrestoCoordinator->>TimeZoneKey: getTimeZoneKey(America/Coyhaique)
TimeZoneKey->>ZoneIndexProperties: Lookup zone id for America/Coyhaique
ZoneIndexProperties-->>TimeZoneKey: Return zone key 2234
TimeZoneKey->>JodaTime: Resolve timezone rules for zone key 2234
JodaTime-->>TimeZoneKey: Return timezone data from tzdata 2025b
TimeZoneKey-->>PrestoCoordinator: Return resolved TimeZoneKey
PrestoCoordinator-->>PrestoCLI: Return query results with converted timestamps
PrestoCLI-->>User: Display results with America/Coyhaique offset and rules
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Given the strong warning around
dep.joda.versionmatching deployed JVM tzdata, consider explicitly noting in the PR description or a code comment which tzdata/JDK version this was validated against to make future upgrades safer. - Since
America/Coyhaiqueis a newly supported zone with a specific key (2234), it may be worth adding a small assertion inTestTimeZoneKey(or a nearby test) that this ID resolves correctly to the expected key to catch accidental regressions in future tz updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Given the strong warning around `dep.joda.version` matching deployed JVM tzdata, consider explicitly noting in the PR description or a code comment which tzdata/JDK version this was validated against to make future upgrades safer.
- Since `America/Coyhaique` is a newly supported zone with a specific key (2234), it may be worth adding a small assertion in `TestTimeZoneKey` (or a nearby test) that this ID resolves correctly to the expected key to catch accidental regressions in future tz updates.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b6fdcc5 to
2841112
Compare
2841112 to
b1ce1e8
Compare
|
Thanks for the release note! Phrasing nit: |
Thanks, I've updated the release notes. Let me know if anything else needs to be changed. |
tdcmeehan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @rdtr
|
@rdtr it seems there's a problem with GH actions not triggering. Please rebase and force push when you can to retrigger them, thanks! |
ac99940 to
9e82254
Compare
|
I pushed the change again. |
9e82254 to
500c684
Compare
500c684 to
e8198c3
Compare
steveburnett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! (docs)
Reviewing on request - no actual docs in this, but the release note and the PR itself look good to me.
- 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
e8198c3 to
7421453
Compare
|
@rdtr the CI strangely seems to be having trouble running the tests here, even after several rebases. Would you be able to open a new PR for these changes? |
|
@tdcmeehan Thanks, I opened a new PR #26981 Please let me know if I should close this one. |
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.