Add fallible versions of temporal functions that may panic #7737

adriangb · 2025-06-22T13:59:10Z

Fixes Temporal Arithmetic Can Panic #4456

adriangb · 2025-06-22T13:59:22Z

adriangb · 2025-06-22T14:00:31Z

arrow-arith/src/numeric.rs

 /// Arithmetic trait for date arrays
-///
-/// Note: these should be fallible (#4456)
 trait DateOp: ArrowTemporalType {


I felt that it was better to just break this trait than introduce all of the new methods. It should be pretty easy for users to update the trait implementation itself, and it's always possible for them to just .expect(...) in their code to at least acknowledge the fallibility

I double checked that this is not a public trait and thus this change is not a breaking API change

https://docs.rs/arrow/latest/arrow/index.html?search=DateOp

alamb · 2025-06-23T20:06:34Z

🤖 ./gh_compare_arrow.sh Benchmark Script Running
Linux aal-dev 6.11.0-1015-gcp #15~24.04.1-Ubuntu SMP Thu Apr 24 20:41:05 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing fix-possible-panics (3659a9e) to 469c7ee diff
BENCH_NAME=arithmetic_kernels
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental --bench arithmetic_kernels
BENCH_FILTER=
BENCH_BRANCH_NAME=fix-possible-panics
Results will be posted here when complete

alamb

Thanks @adriangb -- this looks good to me -- I have kicked off some performance tests just to verify this doesn't change anything

I do think we should try and add some tests to this ideally showing that we can now catch overflows as errors rather than panics. Would you have time to do that?

alamb · 2025-06-23T20:04:13Z

arrow-arith/src/numeric.rs

 /// Arithmetic trait for date arrays
-///
-/// Note: these should be fallible (#4456)
 trait DateOp: ArrowTemporalType {


I double checked that this is not a public trait and thus this change is not a breaking API change

https://docs.rs/arrow/latest/arrow/index.html?search=DateOp

alamb · 2025-06-23T20:09:45Z

Additional context is we hit this panic in DataFusion (and found a workaround)

re-enable sort_query_fuzzer_runner datafusion#16491

adriangb · 2025-06-23T20:20:30Z

Would you have time to do that?

Yes I think I should have time in the next couple days!

alamb · 2025-06-23T20:27:48Z

🤖: Benchmark completed

Details

group                    fix-possible-panics                    main
-----                    -------------------                    ----
add(0)                   1.04     13.7±0.07µs        ? ?/sec    1.00     13.2±0.07µs        ? ?/sec
add(0.1)                 1.00     15.4±0.11µs        ? ?/sec    1.02     15.7±0.09µs        ? ?/sec
add(0.5)                 1.26     15.4±0.08µs        ? ?/sec    1.00     12.3±0.07µs        ? ?/sec
add(0.9)                 1.02     12.3±0.10µs        ? ?/sec    1.00     12.0±0.10µs        ? ?/sec
add(1)                   1.26     14.9±0.14µs        ? ?/sec    1.00     11.8±0.04µs        ? ?/sec
add_checked(0)           1.04     13.6±0.07µs        ? ?/sec    1.00     13.1±0.08µs        ? ?/sec
add_checked(0.1)         1.00     15.5±0.11µs        ? ?/sec    1.00     15.5±0.12µs        ? ?/sec
add_checked(0.5)         1.02     12.1±0.06µs        ? ?/sec    1.00     11.8±0.06µs        ? ?/sec
add_checked(0.9)         1.05     12.5±0.11µs        ? ?/sec    1.00     11.9±0.06µs        ? ?/sec
add_checked(1)           1.02     12.3±0.09µs        ? ?/sec    1.00     12.1±0.08µs        ? ?/sec
add_scalar(0)            1.06      9.3±0.11µs        ? ?/sec    1.00      8.8±0.09µs        ? ?/sec
add_scalar(0.1)          1.00      8.9±0.13µs        ? ?/sec    1.00      8.9±0.11µs        ? ?/sec
add_scalar(0.5)          1.00      7.0±0.01µs        ? ?/sec    1.00      7.0±0.01µs        ? ?/sec
add_scalar(0.9)          1.00      7.0±0.01µs        ? ?/sec    1.00      7.0±0.01µs        ? ?/sec
add_scalar(1)            1.00      7.0±0.04µs        ? ?/sec    1.00      7.0±0.01µs        ? ?/sec
divide(0)                1.06     14.2±0.05µs        ? ?/sec    1.00     13.5±0.03µs        ? ?/sec
divide(0.1)              1.00     14.5±0.02µs        ? ?/sec    1.03     14.9±0.10µs        ? ?/sec
divide(0.5)              1.00     14.6±0.04µs        ? ?/sec    1.00     14.5±0.03µs        ? ?/sec
divide(0.9)              1.02     14.8±0.22µs        ? ?/sec    1.00     14.6±0.06µs        ? ?/sec
divide(1)                1.00     14.5±0.04µs        ? ?/sec    1.00     14.6±0.04µs        ? ?/sec
divide_scalar(0)         1.05     14.0±0.04µs        ? ?/sec    1.00     13.3±0.02µs        ? ?/sec
divide_scalar(0.1)       1.00     13.3±0.03µs        ? ?/sec    1.00     13.3±0.01µs        ? ?/sec
divide_scalar(0.5)       1.00     13.3±0.06µs        ? ?/sec    1.00     13.3±0.02µs        ? ?/sec
divide_scalar(0.9)       1.01     13.4±0.02µs        ? ?/sec    1.00     13.3±0.02µs        ? ?/sec
divide_scalar(1)         1.00     13.3±0.09µs        ? ?/sec    1.00     13.3±0.02µs        ? ?/sec
modulo(0)                1.03    353.3±0.69µs        ? ?/sec    1.00    341.8±0.40µs        ? ?/sec
modulo(0.1)              1.00    391.4±0.68µs        ? ?/sec    1.00    389.7±0.46µs        ? ?/sec
modulo(0.5)              1.00    531.4±2.62µs        ? ?/sec    1.00    530.8±0.99µs        ? ?/sec
modulo(0.9)              1.00    297.6±0.63µs        ? ?/sec    1.00    297.5±0.97µs        ? ?/sec
modulo(1)                1.00    243.6±0.49µs        ? ?/sec    1.00    242.5±0.28µs        ? ?/sec
modulo_scalar(0)         1.00    499.5±0.91µs        ? ?/sec    1.00    501.4±0.83µs        ? ?/sec
modulo_scalar(0.1)       1.00    476.6±1.29µs        ? ?/sec    1.00    478.0±1.01µs        ? ?/sec
modulo_scalar(0.5)       1.00    322.1±0.54µs        ? ?/sec    1.00    322.5±0.94µs        ? ?/sec
modulo_scalar(0.9)       1.00    160.0±0.24µs        ? ?/sec    1.00    159.7±0.28µs        ? ?/sec
modulo_scalar(1)         1.00    118.0±0.23µs        ? ?/sec    1.00    118.1±0.28µs        ? ?/sec
multiply(0)              1.00     13.4±0.43µs        ? ?/sec    1.00     13.4±0.07µs        ? ?/sec
multiply(0.1)            1.00     15.6±0.10µs        ? ?/sec    1.02     15.9±0.10µs        ? ?/sec
multiply(0.5)            1.00     12.1±0.06µs        ? ?/sec    1.01     12.2±0.15µs        ? ?/sec
multiply(0.9)            1.03     12.3±0.13µs        ? ?/sec    1.00     12.0±0.08µs        ? ?/sec
multiply(1)              1.04     12.4±0.12µs        ? ?/sec    1.00     11.9±0.06µs        ? ?/sec
multiply_checked(0)      1.04     13.6±0.06µs        ? ?/sec    1.00     13.1±0.07µs        ? ?/sec
multiply_checked(0.1)    1.01     15.6±0.13µs        ? ?/sec    1.00     15.5±0.07µs        ? ?/sec
multiply_checked(0.5)    1.01     12.3±0.11µs        ? ?/sec    1.00     12.2±0.11µs        ? ?/sec
multiply_checked(0.9)    1.00     12.1±0.06µs        ? ?/sec    1.01     12.2±0.15µs        ? ?/sec
multiply_checked(1)      1.03     12.2±0.09µs        ? ?/sec    1.00     11.9±0.04µs        ? ?/sec
multiply_scalar(0)       1.09      9.3±0.12µs        ? ?/sec    1.00      8.6±0.50µs        ? ?/sec
multiply_scalar(0.1)     1.03      8.9±0.13µs        ? ?/sec    1.00      8.7±0.55µs        ? ?/sec
multiply_scalar(0.5)     1.00      7.0±0.02µs        ? ?/sec    1.01      7.0±0.02µs        ? ?/sec
multiply_scalar(0.9)     1.03      8.9±0.10µs        ? ?/sec    1.00      8.6±0.48µs        ? ?/sec
multiply_scalar(1)       1.00      7.2±0.03µs        ? ?/sec    1.30      9.4±0.06µs        ? ?/sec
subtract(0)              1.05     14.2±0.28µs        ? ?/sec    1.00     13.5±0.06µs        ? ?/sec
subtract(0.1)            1.01     15.8±0.20µs        ? ?/sec    1.00     15.7±0.09µs        ? ?/sec
subtract(0.5)            1.04     12.3±0.10µs        ? ?/sec    1.00     11.8±0.09µs        ? ?/sec
subtract(0.9)            1.03     12.3±0.07µs        ? ?/sec    1.00     11.9±0.06µs        ? ?/sec
subtract(1)              1.08     12.8±0.14µs        ? ?/sec    1.00     11.8±0.05µs        ? ?/sec
subtract_checked(0)      1.03     13.9±0.17µs        ? ?/sec    1.00     13.5±0.07µs        ? ?/sec
subtract_checked(0.1)    1.00     15.5±0.11µs        ? ?/sec    1.00     15.5±0.12µs        ? ?/sec
subtract_checked(0.5)    1.00     12.0±0.06µs        ? ?/sec    1.00     12.1±0.25µs        ? ?/sec
subtract_checked(0.9)    1.03     12.4±0.09µs        ? ?/sec    1.00     12.1±0.07µs        ? ?/sec
subtract_checked(1)      1.00     12.0±0.31µs        ? ?/sec    1.00     11.9±0.06µs        ? ?/sec
subtract_scalar(0)       1.07      9.4±0.10µs        ? ?/sec    1.00      8.8±0.12µs        ? ?/sec
subtract_scalar(0.1)     1.00      8.9±0.13µs        ? ?/sec    1.00      8.9±0.11µs        ? ?/sec
subtract_scalar(0.5)     1.00      7.0±0.02µs        ? ?/sec    1.00      7.0±0.04µs        ? ?/sec
subtract_scalar(0.9)     1.34      9.4±0.08µs        ? ?/sec    1.00      7.0±0.01µs        ? ?/sec
subtract_scalar(1)       1.00      7.1±0.03µs        ? ?/sec    1.32      9.3±0.06µs        ? ?/sec

alamb · 2025-06-23T20:27:51Z

🤖 ./gh_compare_arrow.sh Benchmark Script Running
Linux aal-dev 6.11.0-1015-gcp #15~24.04.1-Ubuntu SMP Thu Apr 24 20:41:05 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing fix-possible-panics (3659a9e) to 469c7ee diff
BENCH_NAME=cast_kernels
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental --bench cast_kernels
BENCH_FILTER=
BENCH_BRANCH_NAME=fix-possible-panics
Results will be posted here when complete

alamb · 2025-06-23T20:42:12Z

🤖: Benchmark completed

Details

group                                                              fix-possible-panics                    main
-----                                                              -------------------                    ----
cast binary view to string                                         1.01     76.1±0.12µs        ? ?/sec    1.00     75.5±0.11µs        ? ?/sec
cast binary view to string view                                    1.00     99.9±0.24µs        ? ?/sec    1.00     99.9±0.14µs        ? ?/sec
cast binary view to wide string                                    1.00     76.7±0.26µs        ? ?/sec    1.01     77.8±0.13µs        ? ?/sec
cast date32 to date64 512                                          1.00    287.7±0.37ns        ? ?/sec    1.04    298.5±0.69ns        ? ?/sec
cast date64 to date32 512                                          1.01    503.6±0.76ns        ? ?/sec    1.00    499.3±0.95ns        ? ?/sec
cast decimal128 to decimal128 512                                  1.00    609.4±0.70ns        ? ?/sec    1.02    621.9±1.00ns        ? ?/sec
cast decimal128 to decimal128 512 lower precision                  1.00      4.5±0.01µs        ? ?/sec    1.00      4.5±0.01µs        ? ?/sec
cast decimal128 to decimal128 512 with lower scale (infallible)    1.00      6.7±0.01µs        ? ?/sec    1.00      6.7±0.01µs        ? ?/sec
cast decimal128 to decimal128 512 with same scale                  1.00     72.0±0.13ns        ? ?/sec    1.00     72.1±0.11ns        ? ?/sec
cast decimal128 to decimal256 512                                  1.00      2.6±0.01µs        ? ?/sec    1.00      2.6±0.01µs        ? ?/sec
cast decimal256 to decimal128 512                                  1.00     48.8±0.10µs        ? ?/sec    1.00     48.8±0.06µs        ? ?/sec
cast decimal256 to decimal256 512                                  1.02     11.4±0.03µs        ? ?/sec    1.00     11.2±0.03µs        ? ?/sec
cast decimal256 to decimal256 512 with same scale                  1.00     71.8±0.10ns        ? ?/sec    1.01     72.2±0.11ns        ? ?/sec
cast dict to string view                                           1.00     48.4±0.12µs        ? ?/sec    1.04     50.5±0.69µs        ? ?/sec
cast f32 to string 512                                             1.01     19.4±0.04µs        ? ?/sec    1.00     19.3±0.05µs        ? ?/sec
cast f64 to string 512                                             1.00     22.4±0.03µs        ? ?/sec    1.00     22.5±0.04µs        ? ?/sec
cast float32 to int32 512                                          1.00   1652.3±4.78ns        ? ?/sec    1.00   1650.7±2.02ns        ? ?/sec
cast float64 to float32 512                                        1.01   1437.5±1.52ns        ? ?/sec    1.00   1428.4±1.26ns        ? ?/sec
cast float64 to uint64 512                                         1.01   1943.8±4.99ns        ? ?/sec    1.00  1930.4±32.65ns        ? ?/sec
cast i64 to string 512                                             1.00     14.7±0.03µs        ? ?/sec    1.01     14.9±0.03µs        ? ?/sec
cast int32 to float32 512                                          1.00   1453.2±1.99ns        ? ?/sec    1.00   1455.9±1.87ns        ? ?/sec
cast int32 to float64 512                                          1.01   1403.7±4.67ns        ? ?/sec    1.00   1394.0±6.68ns        ? ?/sec
cast int32 to int32 512                                            1.03    206.3±0.80ns        ? ?/sec    1.00    200.5±1.92ns        ? ?/sec
cast int32 to int64 512                                            1.00   1475.2±1.44ns        ? ?/sec    1.01   1485.0±3.76ns        ? ?/sec
cast int32 to uint32 512                                           1.00   1477.4±7.81ns        ? ?/sec    1.02   1507.6±7.91ns        ? ?/sec
cast int64 to int32 512                                            1.00  1623.6±14.31ns        ? ?/sec    1.00   1630.1±3.88ns        ? ?/sec
cast string to binary view 512                                     1.00      3.1±0.01µs        ? ?/sec    1.00      3.1±0.01µs        ? ?/sec
cast string view to binary view                                    1.00     95.0±0.15ns        ? ?/sec    1.07    101.2±0.23ns        ? ?/sec
cast string view to dict                                           1.00    197.9±0.32µs        ? ?/sec    1.04    206.3±0.29µs        ? ?/sec
cast string view to string                                         1.00     58.2±0.11µs        ? ?/sec    1.00     58.0±0.08µs        ? ?/sec
cast string view to wide string                                    1.00     59.1±0.12µs        ? ?/sec    1.00     59.2±0.16µs        ? ?/sec
cast time32s to time32ms 512                                       1.00    284.9±0.38ns        ? ?/sec    1.00    284.6±0.91ns        ? ?/sec
cast time32s to time64us 512                                       1.04    300.4±2.17ns        ? ?/sec    1.00    289.8±0.31ns        ? ?/sec
cast time64ns to time32s 512                                       1.01    504.8±0.58ns        ? ?/sec    1.00    501.3±0.88ns        ? ?/sec
cast timestamp_ms to i64 512                                       1.00    440.0±1.29ns        ? ?/sec    1.04    456.4±0.65ns        ? ?/sec
cast timestamp_ms to timestamp_ns 512                              1.06      2.6±0.01µs        ? ?/sec    1.00      2.5±0.01µs        ? ?/sec
cast timestamp_ns to timestamp_s 512                               1.01    198.2±0.23ns        ? ?/sec    1.00    197.1±0.28ns        ? ?/sec
cast utf8 to date32 512                                            1.01     11.4±0.02µs        ? ?/sec    1.00     11.3±0.02µs        ? ?/sec
cast utf8 to date64 512                                            1.00     45.4±0.05µs        ? ?/sec    1.01     45.8±0.05µs        ? ?/sec
cast utf8 to f32                                                   1.01     11.7±0.02µs        ? ?/sec    1.00     11.6±0.02µs        ? ?/sec
cast wide string to binary view 512                                1.00      5.5±0.01µs        ? ?/sec    1.01      5.5±0.01µs        ? ?/sec

…panic due to overflows

adriangb · 2025-06-24T17:41:17Z

@alamb I've added quite a bit of tests

alamb

Looks really nice to me -- thank you @adriangb

adriangb · 2025-06-25T12:41:39Z

@alamb I don't have commit rights here, could you merge this if you think it's ready? thanks!

github-actions bot added the arrow Changes to the arrow crate label Jun 22, 2025

adriangb commented Jun 22, 2025

View reviewed changes

adriangb changed the title ~~Add fallible versions of temporal functions that may panci~~ Add fallible versions of temporal functions that may panic Jun 22, 2025

alamb reviewed Jun 23, 2025

View reviewed changes

adriangb added 3 commits June 24, 2025 11:32

add _opt versions of various functions that risk producing a runtime …

d9f6928

…panic due to overflows

add boundary tests

03820b4

fmt

897fec6

adriangb force-pushed the fix-possible-panics branch from 3659a9e to 897fec6 Compare June 24, 2025 17:41

fix tests

508daca

alamb approved these changes Jun 24, 2025

View reviewed changes

alamb merged commit 4d3906c into apache:main Jun 25, 2025
26 checks passed

liamzwbao mentioned this pull request Jul 17, 2025

Date64::to_naive_date panics on larger values #7728

Open

Add fallible versions of temporal functions that may panic #7737

Add fallible versions of temporal functions that may panic #7737

Uh oh!

Conversation

adriangb commented Jun 22, 2025 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriangb commented Jun 22, 2025

Uh oh!

adriangb Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

alamb commented Jun 23, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

alamb commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriangb commented Jun 23, 2025

Uh oh!

alamb commented Jun 23, 2025

Uh oh!

alamb commented Jun 23, 2025

Uh oh!

alamb commented Jun 23, 2025

Uh oh!

adriangb commented Jun 24, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

adriangb commented Jun 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adriangb commented Jun 22, 2025 •

edited by alamb

Loading

alamb commented Jun 23, 2025 •

edited

Loading