Repository navigation
Conversation
…ime on Windows The vendored musl strptime, used on Windows, handles %a, %A, %b, %B, %h, %c, %p, %r, %x and %X only when nl_langinfo() is available, which it is not on Windows. Provide a C-locale replacement for the items used. ARROW_TEST_FALLBACK_LANGINFO compiles the replacement on other platforms. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
…trptime timestamp parser strptime checks the day of month against 31 at most, and some implementations (macOS, the vendored musl used on Windows) accept day 00. The parser rolled impossible dates such as February 30 over into the next month and took a zero day as the first of the month. Initialize the day to the first before parsing, so that a format without a day directive still gives the first of the month, and reject the result if it is not a valid date, as the ISO-8601 parser does. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
…arser strptime accepts a seconds value of 60 (glibc also 61), and the parser added it to the time of day, wrapping it into the next minute. Arrow cannot represent leap seconds, and the ISO-8601 parser rejects them, so reject them here too. The check applies only when the time of day is used, so Gandiva's to_date, which ignores the time, still accepts such values. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
… CSV date columns Columns explicitly typed as date32 or date64 were parsed only from the ISO-8601 format. Try ConvertOptions::timestamp_parsers after the ISO-8601 parser fails, and floor the parsed timestamp to the day boundary, as a cast from timestamp to date does. Strptime parsers without a date directive are skipped, so that a time is not accepted with a default date. Type inference of date columns is not affected. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
… CSV time columns Columns explicitly typed as time32 or time64 were parsed only from the ISO-8601 format. Try ConvertOptions::timestamp_parsers after the ISO-8601 parser fails, and take the time of day of the parsed timestamp, as a cast from timestamp to time does. Strptime parsers without a time directive, and ISO-8601 values without a time of day, are skipped, so that a date is not accepted as midnight. Type inference of time columns is not affected. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
|
|
github-actions
Bot
added
Component: C++
Component: Python
Component: Gandiva
Component: Documentation
awaiting review
|
Thanks for opening a pull request! This pull request has been automatically closed because you currently have 4 open pull requests, which is more than the limit of 3. Due to the increase in pull requests opened by AI bots, and in order to keep the review queue manageable, Apache Arrow limits contributors without repository access to at most 3 concurrently open pull requests. This helps make sure each pull request gets the attention it needs and that work in progress does not go stale. Once one of your other open pull requests has been merged or closed, you are welcome to reopen this one. See also: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
CSV columns typed as
time32ortime64are parsed only from the ISO-8601 format, andConvertOptions::timestamp_parsersapplies only to timestamp columns, so other time formats need reading as a timestamp and a cast.What changes are included in this PR?
timestamp_parsersafter the ISO-8601 parser fails, and take the time of day of the parsed timestamp, as a cast from timestamp to time does. This also works for dates outside the range of the time unit's timestamps (e.g. year 9999 with nanoseconds).timestamp_parsers, fractional seconds are kept, and the value is rejected if the time unit cannot represent them.strptimeparsers without a time directive are skipped, and ISO-8601 values without a time of day are rejected, so that a date does not become midnight.This PR is the time part of #50146. It builds on #54661 (date columns) and its dependencies #54656, #54657 and #54659, which must land first.
Are these changes tested?
Yes,
Time32Conversion.UserDefinedParsersandTime64Conversion.UserDefinedParsersin C++, andtest_times_with_timestamp_parsersin Python.Are there any user-facing changes?
Yes,
timestamp_parsersnow also applies totime32/time64columns given incolumn_types. This is documented in the C++ CSV docs and theConvertOptionsdocstring.AI usage disclosure
Developed with AI assistance (Claude Code): the implementation, tests, documentation and this description were AI-generated under my direction and reviewed line by line by me. I own and can debug these changes. No copyrighted material is included.