Skip to content

ARROW-11563: [Rust] Support Cast(Utf8, TimeStamp(Nanoseconds, None)) - #9449

Closed
ovr wants to merge 1 commit into
apache:masterfrom
ovr:issue-11563
Closed

ARROW-11563: [Rust] Support Cast(Utf8, TimeStamp(Nanoseconds, None))#9449
ovr wants to merge 1 commit into
apache:masterfrom
ovr:issue-11563

Conversation

@ovr

@ovrovr commented Feb 8, 2021

Copy link
Copy Markdown
Contributor

DataFushion already has this cast with its own implementation inside DF based on top of the chrono library. In this PR, I moved functions called string_to_timestamp_nanos and naive_datetime_to_timestamp (helpers) to Arrow's compute::kernels crate to make it possible to do Cast(Utf8, Timestamp(Nanoseconds, None)) and usage these helpers inside DF for to_timestamp function.

@github-actions

Copy link
Copy Markdown

@ovrovr changed the title ARROW-11563: [Rust] Implement Cast(Utf8, TimeStamp(Nanoseconds, None))ARROW-11563: [Rust] Support Cast(Utf8, TimeStamp(Nanoseconds, None))Feb 8, 2021

@alambalamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work @ovr -- I have an additional suggested test but this PR could also be merged in as is I think

Comment threadrust/arrow/src/compute/kernels/cast.rs Outdated
Comment threadrust/arrow/src/error.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding a new Arrow Error variant seems reasonable to me -- what do you think @jorgecarleitao and @nevi-me and @andygrove ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm happy with the addition

@alamb

Copy link
Copy Markdown
Contributor

I think this PR is ready to go after a rebase -- any thoughts @Dandandan@jorgecarleitao or @andygrove ?

@alambalamb added the needs-rebase A PR that needs to be rebased by the author label Feb 11, 2021

@nevi-menevi-me left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment threadrust/arrow/src/error.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm happy with the addition

@ovr

ovr commented Feb 12, 2021

Copy link
Copy Markdown
ContributorAuthor

Just a notice: Rebased

@alamb

Copy link
Copy Markdown
Contributor

Thanks again @ovr !

@alambalamb closed this in 2be54a5Feb 12, 2021
@ovr
ovr deleted the issue-11563 branch February 12, 2021 11:16
sgnkc pushed a commit to sgnkc/arrow that referenced this pull request Feb 17, 2021
DataFushion already has this cast with its own implementation inside DF based on top of the chrono library. In this PR, I moved functions called string_to_timestamp_nanos and naive_datetime_to_timestamp (helpers) to Arrow's compute::kernels crate to make it possible to do Cast(Utf8, Timestamp(Nanoseconds, None)) and usage these helpers inside DF for to_timestamp function.
Closesapache#9449 from ovr/issue-11563
Authored-by: Dmitry Patsura <zaets28rus@gmail.com>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Rust - DataFusionComponent: Rustneeds-rebaseA PR that needs to be rebased by the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ovr@alamb@nevi-me