Skip to content

feat: Add America/Coyhaique timezone support#16362

Open
n0r0shi wants to merge 1 commit intofacebookincubator:mainfrom
n0r0shi:add-timezone-america-coyhaique
Open

feat: Add America/Coyhaique timezone support#16362
n0r0shi wants to merge 1 commit intofacebookincubator:mainfrom
n0r0shi:add-timezone-america-coyhaique

Conversation

@n0r0shi
Copy link
Contributor

@n0r0shi n0r0shi commented Feb 12, 2026

Summary

  • Add America/Coyhaique (timezone ID 2234) to TimeZoneDatabase.cpp, following the Presto zone-index convention
  • Add display names to TimeZoneNames.cpp (Coyhaique Standard Time, Coyhaique Daylight Time)
  • Fix namespace in gen_timezone_database.py (facebook::velox::utilfacebook::velox::tz)
  • Add test coverage for the new timezone entry

America/Coyhaique was added to the IANA timezone database in tzdata 2025b (JDK 17+). Systems running newer tzdata include this zone, but Velox rejects it because it's not in the hardcoded timezone ID map, causing queries with session_timezone = 'America/Coyhaique' to fail.

The upstream Presto PR (prestodb/presto#26981) adding this zone to zone-index.properties has been merged.

How the files were generated

TimeZoneDatabase.cpp — generated from the Presto zone-index.properties:

./velox/type/tz/gen_timezone_database.py \
  -f /path/to/zone-index.properties \
  > velox/type/tz/TimeZoneDatabase.cpp

TimeZoneNames.cpp — short and long names obtained from Java's DateFormatSymbols (openjdk 17):

javac velox/type/tz/GenTimeZoneNames.java
java velox.type.tz.GenTimeZoneNames > velox/type/tz/TimeZoneNames.cpp

The entry was added manually to avoid a noisy full-file diff from formatting differences.

Related

Test plan

  • getTimeZoneName(2234) returns "America/Coyhaique"
  • getTimeZoneID("America/Coyhaique") returns 2234
  • Generated TimeZoneDatabase.cpp matches committed file exactly

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 12, 2026
@netlify
Copy link

netlify bot commented Feb 12, 2026

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit c82e833
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/69ac01b6ea91af0009ed1084

@kgpai
Copy link
Contributor

kgpai commented Feb 12, 2026

Hi @n0r0shi The Timezonedatabase file is autogenerated and based on what is supported by Presto. We can make this change after prestodb/presto#26981 lands in Presto.

@n0r0shi
Copy link
Contributor Author

n0r0shi commented Feb 14, 2026

@kgpai Thanks for the context. I understand that prestodb/presto#26981 needs to be merged first since Velox pulls its timezone database from Presto. That's exactly why I've kept this PR as a draft. I'm happy to wait until the Presto change lands, and I'll rebase and mark it ready for review once it does.

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
@n0r0shi n0r0shi force-pushed the add-timezone-america-coyhaique branch from 2ae9467 to c82e833 Compare March 7, 2026 10:45
@n0r0shi n0r0shi marked this pull request as ready for review March 7, 2026 10:48
@n0r0shi
Copy link
Contributor Author

n0r0shi commented Mar 7, 2026

Updated this PR — it's no longer a draft since the upstream Presto PR (prestodb/presto#26981) has been merged.

Changes since the original version:

  • Added America/Coyhaique display names to TimeZoneNames.cpp
  • Fixed namespace in gen_timezone_database.py (utiltz) to match the current codebase

Ready for review. @rui-mo @zhli1142015 @kgpai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Velox rejects valid IANA timezone America/Coyhaique present in system tzdb

2 participants