Support converting large dates (i.e. +10999-12-31) from string to Date32 by phillipleblanc · Pull Request #7074 · apache/arrow-rs

phillipleblanc · 2025-02-04T17:20:02Z

Which issue does this PR close?

Closes Support casting strings to Date32 that contain large dates arrow-rs-object-store#22

Rationale for this change

Support for casting large dates from string to Date32.

What changes are included in this PR?

Extend the parse_date method, which is used in the impl Parser for Date32Type, to handle dates which are prefixed with + or -. If the date is not prefixed with + or -, the existing logic is used unmodified.

This code isn't as optimized as the code for processing more common date formats - but given that these extended dates are relatively rare in practice, I don't think it matters all that much.

Are there any user-facing changes?

Aside from the desired fix, no.

alamb

Thank you @phillipleblanc -- this looks readonable to me. I only think the PR needs a few more tests and we can merge.

I am sure we could make parsing dates like this faster but we can do that type of optimization as a follow up.

I am also running the cast benchmarks just to be sure this doesn't accidentally introduce a regression and will post the results to this PR

arrow-cast/src/cast/mod.rs

arrow-cast/src/parse.rs

alamb · 2025-02-06T19:03:53Z

++ critcmp main phillip_250205-handle-large-dates
group         main                                   phillip_250205-handle-large-dates
-----         ----                                   ---------------------------------
2020-09-08    1.00     21.5±0.05ns        ? ?/sec    1.04     22.2±0.05ns        ? ?/sec
2020-09-8     1.01     19.0±0.08ns        ? ?/sec    1.00     18.8±0.03ns        ? ?/sec
2020-9-08     1.00     18.6±0.04ns        ? ?/sec    1.04     19.4±0.15ns        ? ?/sec
2020-9-8      1.00     17.4±0.02ns        ? ?/sec    1.01     17.5±0.02ns        ? ?/sec

Seems ok to me

Co-authored-by: Andrew Lamb <[email protected]>

…e in decimal conversion (apache#7070) * fix <= check for scale in decimal conversion * Update arrow-cast/src/cast/mod.rs name change Co-authored-by: Arttu <[email protected]> * remove incorrect comment --------- Co-authored-by: Arttu <[email protected]>

* Add another decimal cast edge test case Before 1019f5b this test would fail, as the cast produced 1. 0 is an edge case worth explicitly testing for. * typo/fmt Co-authored-by: Felipe Oliveira Carvalho <[email protected]> --------- Co-authored-by: Felipe Oliveira Carvalho <[email protected]>

…adata (apache#7052) * Support both 0x01 and 0x02 as type for list of booleans * Also support 0 for false inside boolean collections * Use hex notation in tests

…pache#6751) * Fix LocalFileSystem with range request that ends beyond end of file * fix windows * add comment * Seek error * fix seek check * remove windows flag * Get file length from file metadata

…ache#7027) * Introduce UnsafeFlag to manage disabling validation * fix docs

…che#7028) * Rename `ArrayReader` to `RecordBatchDecoder` * Remove alias for `self`

* Minor: Update release schedule * realism

* Refactor some decimal-related code and tests in preparation for adding Decimal32 and Decimal64 support * Fixed symbol * Apply PR feedback * Fixed format problem * Fixed logical merge conflicts * PR feedback

…coder` (apache#7029) * Move `create_primitive_array` into RecordBatchReader * Move `create_list-array` into RecordBatchReader * Move `create_dictionay_array` into RecordBatchReader

* Print Parquet BasicTypeInfo id when present * Improve print_schema documentation * tiny cleanup

…he#7019) * Initial change from Daniel. * Upgrade unit test to be more generic. * Add comments on why we have filter * Cleanup unit tests. * Update object_store/src/local.rs Co-authored-by: Adam Reeve <[email protected]> * Add changes suggested by Adam. * Cleanup match error. * Apply formatting changes suggested by cargo +stable fmt --all. * Apply cosmetic changes suggested by clippy. * Upgrade test_path_with_offset to create temporary directory + files for testing rather than pointing to existing dir. --------- Co-authored-by: Adam Reeve <[email protected]>

…s` (apache#7065) * fix: first none in `ListArray` panics in `cast_with_options` * simplify * fix * Update arrow-cast/src/cast/list.rs Co-authored-by: Jeffrey Vo <[email protected]> --------- Co-authored-by: Jeffrey Vo <[email protected]>

* Add benchmarks for Arrow IPC writer * Add benchmarks for Arrow IPC writer * reuse target buffer * rename, etc * Add compression type * update --------- Co-authored-by: Andy Grove <[email protected]>

…pache#7089) * Minor: Clarify documentaiton on NullBufferBuilder::allocated_size * add note about why allocations are 64 bytes

phillipleblanc · 2025-02-10T04:05:33Z

Thanks @alamb for the review. I've pushed up fixes for your comments.

…-large-dates

alamb

Looks good to me -- thanks @phillipleblanc and @sgrebnov

alamb · 2025-02-10T20:38:50Z

arrow-cast/src/cast/mod.rs

+        assert_eq!(3298139, c.value(0)); // 10999-12-31
+        assert_eq!(-723122, c.value(1)); // -0010-02-28
+        assert_eq!(-715817, c.value(2)); // 0010-02-28
+        assert_eq!(c.value(3), c.value(4)); // Expect 0000-01-01 and -0000-01-01 to be parsed the same


alamb · 2025-02-12T11:26:30Z

Thanks again @phillipleblanc

Upgrade to the latest delta-rs main branch, which has a workaround for this: apache/arrow-rs#7074 This triggered apache/datafusion#14862 and while we're waiting for the [fix](apache/datafusion#14862) to land and make it into the next datafusion release, I had to add a [patch] section to Cargo.toml to use the fixed-up version of datafusion 45.0. Signed-off-by: Leonid Ryzhyk <[email protected]>

Support converting large dates (i.e. +10999-12-31) from string to Date32

93934bf

github-actions bot added the arrow Changes to the arrow crate label Feb 4, 2025

Fix lint

4041947

sgrebnov approved these changes Feb 4, 2025

View reviewed changes

phillipleblanc mentioned this pull request Feb 5, 2025

Handle delta tables partitioned by a date column with large date values spiceai/spiceai#4672

Merged

alamb reviewed Feb 6, 2025

View reviewed changes

arrow-cast/src/cast/mod.rs Show resolved Hide resolved

arrow-cast/src/cast/mod.rs Show resolved Hide resolved

arrow-cast/src/parse.rs Show resolved Hide resolved

phillipleblanc and others added 17 commits February 10, 2025 12:22

Update arrow-cast/src/parse.rs

a584828

Co-authored-by: Andrew Lamb <[email protected]>

minor: re-export OffsetBufferBuilder in arrow crate (apache#7077)

3a8a001

Support both 0x01 and 0x02 as type for list of booleans in thrift met…

7e91c46

…adata (apache#7052) * Support both 0x01 and 0x02 as type for list of booleans * Also support 0 for false inside boolean collections * Use hex notation in tests

Fix LocalFileSystem with range request that ends beyond end of file (a…

bb5f3ae

…pache#6751) * Fix LocalFileSystem with range request that ends beyond end of file * fix windows * add comment * Seek error * fix seek check * remove windows flag * Get file length from file metadata

Introduce UnsafeFlag to manage disabling ArrayData validation (ap…

e199ccc

…ache#7027) * Introduce UnsafeFlag to manage disabling validation * fix docs

Refactor arrow-ipc: Rename ArrayReader to RecodeBatchDecoder (apa…

6ec6cd9

…che#7028) * Rename `ArrayReader` to `RecordBatchDecoder` * Remove alias for `self`

Minor: Update release schedule (apache#7086)

02ee7d2

* Minor: Update release schedule * realism

Refactor some decimal-related code and tests (apache#7062)

ec1d17a

* Refactor some decimal-related code and tests in preparation for adding Decimal32 and Decimal64 support * Fixed symbol * Apply PR feedback * Fixed format problem * Fixed logical merge conflicts * PR feedback

Refactor arrow-ipc: Move create_*_array methods into `RecordBatchDe…

706a523

…coder` (apache#7029) * Move `create_primitive_array` into RecordBatchReader * Move `create_list-array` into RecordBatchReader * Move `create_dictionay_array` into RecordBatchReader

Print Parquet BasicTypeInfo id when present (apache#7094)

0d943b9

* Print Parquet BasicTypeInfo id when present * Improve print_schema documentation * tiny cleanup

Benchmarks for Arrow IPC writer (apache#7090)

d74be2c

* Add benchmarks for Arrow IPC writer * Add benchmarks for Arrow IPC writer * reuse target buffer * rename, etc * Add compression type * update --------- Co-authored-by: Andy Grove <[email protected]>

Minor: Clarify documentation on NullBufferBuilder::allocated_size (a…

5f69b6e

…pache#7089) * Minor: Clarify documentaiton on NullBufferBuilder::allocated_size * add note about why allocations are 64 bytes

Add more tests for edge cases

6dcdde6

github-actions bot added parquet Changes to the parquet crate object-store labels Feb 10, 2025

Add negative test case for incorrectly formatted large dates

05d500f

Merge remote-tracking branch 'origin/main' into phillip/250205-handle…

f0bcaf1

…-large-dates

github-actions bot removed the parquet Changes to the parquet crate label Feb 10, 2025

github-actions bot removed the object-store label Feb 10, 2025

alamb approved these changes Feb 10, 2025

View reviewed changes

alamb merged commit 2bce568 into apache:main Feb 12, 2025
26 checks passed

ryzhyk mentioned this pull request Feb 25, 2025

[adpaters] Use the latest unreleased delta.rs. feldera/feldera#3607

Merged

swanandx mentioned this pull request Apr 17, 2025

Error casting strings with large dates to Timestamp #7208

Open

phillipleblanc deleted the phillip/250205-handle-large-dates branch December 6, 2025 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Support converting large dates (i.e. +10999-12-31) from string to Date32#7074

Support converting large dates (i.e. +10999-12-31) from string to Date32#7074
alamb merged 21 commits intoapache:mainfrom
spiceai:phillip/250205-handle-large-dates

phillipleblanc commented Feb 4, 2025 •

edited by alamb

Loading

Uh oh!

alamb left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alamb commented Feb 6, 2025

Uh oh!

phillipleblanc commented Feb 10, 2025

Uh oh!

alamb left a comment

Uh oh!

alamb Feb 10, 2025

Uh oh!

Uh oh!

alamb commented Feb 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Comments

Conversation

phillipleblanc commented Feb 4, 2025 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alamb commented Feb 6, 2025

Uh oh!

phillipleblanc commented Feb 10, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb commented Feb 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

phillipleblanc commented Feb 4, 2025 •

edited by alamb

Loading