Skip to content

GH-30036: [C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") #12865

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.

Sign up for GitHub

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

Jump to bottom
Merged
rok merged 23 commits into from
Oct 7, 2025
Merged

GH-30036: [C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") #12865

rok merged 23 commits into from
Oct 7, 2025

Conversation

@rok
Copy link
Member

@rok rok commented Apr 12, 2022 •
edited by github-actions bot
Loading

ARROW-14477: #30036

Currently timestamp arrays have unit timestamp(unit, zone name). This would add "offset timezones" where timestamp array would also support units like timestamp(unit, "+/-HH:MM").

@rok
Copy link
Member Author

rok commented Apr 12, 2022

@pitrou @lidavidm @jorisvandenbossche this is complete yet but I would like to validate the approach with you before I continue. Do you feel this makes enough sense to complete the PR?
Please note date.h recognises zones names like "Etc/GMT+10" but only in iterations of 1hr, while we might want to cover 30 minute iterations. I'm basing this off of the date.h recommendation for custom time zones.

@pitrou
Copy link
Member

pitrou commented Apr 12, 2022

Please note date.h recognises zones names like "Etc/GMT+10" but only in iterations of 1hr, while we might want to cover 30 minute iterations.

What is this comment in reference to? It seems this PR would handle 30 minute iterations correctly?

pitrou
pitrou reviewed Apr 12, 2022
View reviewed changes
@rok
Copy link
Member Author

rok commented Apr 12, 2022 •
edited
Loading

What is this comment in reference to? It seems this PR would handle 30 minute iterations correctly?

Current implementation can handle fixed offsets of iterations of one hour. This PR would enable offsets of arbitrary number of minutes. So primary use of that would indeed be to operate in GMT+/-HH:30 offsets. Otherwise we don't need an extra type just a translation utility for +/-HH:00 -> Etc/GMT+/-HH.

@github-actions
Copy link

github-actions bot commented Apr 12, 2022

https://issues.apache.org/jira/browse/ARROW-14477

@rok rok force-pushed the ARROW-14477 branch 2 times, most recently from ead6a33 to b7c3038 Compare April 22, 2022 03:08
pitrou
pitrou reviewed Apr 25, 2022
View reviewed changes
@rok rok mentioned this pull request Dec 18, 2023
Closed
@rok rok mentioned this pull request Jul 9, 2025
Closed
@github-actions github-actions bot added the awaiting review Awaiting review label Jul 11, 2025
@rok rok force-pushed the ARROW-14477 branch 9 times, most recently from 0f1f432 to 53ec435 Compare July 13, 2025 22:22
@rok rok changed the title ARROW-14477: [C++] Timezone-aware kernels should also handle offset strings ARROW-14477: [C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") Jul 13, 2025
@rok rok marked this pull request as ready for review July 13, 2025 22:31
@rok rok requested a review from pitrou July 13, 2025 23:39
@rok rok mentioned this pull request Jul 13, 2025
Open
@rok rok linked an issue Jul 14, 2025 that may be closed by this pull request
[C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") #30036
Closed
rok
rok commented Jul 14, 2025
View reviewed changes
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jul 14, 2025
@rok
Copy link
Member Author

rok commented Oct 7, 2025 •
edited
Loading

Thank you for the review @lidavidm !
I'll rebase and merge if CI passes.
edit: also added a minimal Python test

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 7, 2025
@rok rok requested review from AlenkaF and raulcd as code owners October 7, 2025 10:26
@rok
Copy link
Member Author

rok commented Oct 7, 2025

@github-actions crossbow submit -g cpp

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Revision: 936d020

Submitted crossbow builds: ursacomputing/crossbow @ actions-744a53bba9

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@rok rok changed the title ARROW-14477: [C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") GH-30036: [C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30") Oct 7, 2025
@asfimport asfimport mentioned this pull request Oct 13, 2022
Closed
@rok rok merged commit bf342b2 into apache:main Oct 7, 2025
42 of 44 checks passed
@rok rok deleted the ARROW-14477 branch October 7, 2025 11:31
@rok rok removed the awaiting change review Awaiting change review label Oct 7, 2025
@conbench-apache-arrow
Copy link

conbench-apache-arrow bot commented Oct 7, 2025

After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit bf342b2.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Oct 15, 2025
@rok @pitrou @zanmato1984
apacheGH-30036: [C++] Timezone-aware kernels should handle offset str…
afdca96 
…ings (e.g. "+04:30") (apache#12865)

ARROW-14477: apache#30036

Currently timestamp arrays have unit `timestamp(unit, zone name)`. This would add "offset timezones" where timestamp array would also support units like `timestamp(unit, "+/-HH:MM")`.
* GitHub Issue: apache#30036

Lead-authored-by: Rok Mihevc <[email protected]>
Co-authored-by: Rok <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Rok Mihevc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidavidm lidavidm lidavidm approved these changes

@pitrou pitrou Awaiting requested review from pitrou

@AlenkaF AlenkaF Awaiting requested review from AlenkaF AlenkaF is a code owner

@raulcd raulcd Awaiting requested review from raulcd raulcd is a code owner

+1 more reviewer

@zilto zilto zilto left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++ Component: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Timezone-aware kernels should handle offset strings (e.g. "+04:30")

4 participants

@rok @pitrou @zilto @lidavidm