Skip to content

feat: Add from_datetime method to Timestamp types#9345

Open
codephage2020 wants to merge 2 commits intoapache:mainfrom
codephage2020:issue-9337
Open

feat: Add from_datetime method to Timestamp types#9345
codephage2020 wants to merge 2 commits intoapache:mainfrom
codephage2020:issue-9337

Conversation

@codephage2020
Copy link
Contributor

@codephage2020 codephage2020 commented Feb 3, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

YES

Are there any user-facing changes?

YES, Now it is possible to directly convert from datatime to timestamp.

before:
DateTime<Tz> → .naive_local() /.naive_utc() → NaiveDateTime → make_value() → i64
now:
DateTime<Tz> → i64 (timestamp)

@github-actions github-actions bot added the arrow Changes to the arrow crate label Feb 3, 2026
@codephage2020 codephage2020 force-pushed the issue-9337 branch 2 times, most recently from 6a39b1c to 617a44d Compare February 4, 2026 01:34
Copy link
Contributor

@sdf-jkl sdf-jkl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything LGTM, but one comment below

@codephage2020 codephage2020 requested a review from sdf-jkl February 6, 2026 05:38
@codephage2020
Copy link
Contributor Author

@sdf-jkl Thank you for your review.
Added tests for LocalResult edge cases:

timestamp_from_naive_datetime_ambiguous - tests DST end transition (returns first match)
timestamp_from_naive_datetime_none - tests DST start transition with non-existent time (returns None)

CC @alamb

Copy link
Contributor

@sdf-jkl sdf-jkl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, everything LGTM

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @codephage2020 and @sdf-jkl -- this looks great to me, except for one question (on returning Option). Let me know what you think

…tampType (apache#9337)

Add two new methods to the ArrowTimestampType trait:
- from_datetime: creates a timestamp from a timezone-aware DateTime<Tz>
- from_naive_datetime: creates a timestamp from a NaiveDateTime with optional timezone

This simplifies the conversion path from DateTime<Tz> to i64 timestamp,
avoiding the need to manually call .naive_utc() + make_value().

Both methods return Option<i64> to safely handle overflow (e.g. nanosecond
precision with extreme datetime values). Uses checked arithmetic consistent
with existing make_value implementations.

Includes tests for UTC, fixed-offset, subsecond precision, DST ambiguity,
non-existent local times, and overflow boundaries.
@codephage2020
Copy link
Contributor Author

codephage2020 commented Feb 10, 2026

Thanks for the reviews @alamb @sdf-jkl! I've pushed an update addressing all feedback.

Could you take another look when you get a chance? Thanks!

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me -- thank you so much @codephage2020 and @sdf-jkl

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

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add some way to create a Timestamp from a DateTime

3 participants