andygrove commented on PR #6836: URL: https://github.com/apache/arrow-rs/pull/6836#issuecomment-2524156050
> Have we run any benchmarks (I'm not sure if any actually exist) to confirm this doesn't significantly regress performance. > > It seems unfortunate to be always performing overflow checks, when in many cases it should be possible to prove that precision overflow can't occur and need not be checked for I created a simple benchmark for decimal casting in https://github.com/apache/arrow-rs/pull/6850. Unsurprisingly, validating that the results are correct is slower than not validating the results. ## before ``` cast_decimal time: [45.281 ns 45.549 ns 45.871 ns] ``` ## after (this PR) ``` cast_decimal time: [247.97 ns 248.78 ns 249.78 ns] change: [+435.06% +439.47% +443.15%] (p = 0.00 < 0.05) ``` We currently have the config option of `safe` on or off: ``` pub struct CastOptions<'a> { /// how to handle cast failures, either return NULL (safe=true) or return ERR (safe=false) pub safe: bool, ``` So, yes, it is a performance regression, but the previous behavior was incorrect. This PR now makes this work as advertised. @tustvold Is there a use case we need to support for faster casts without validating results per the CastOptions? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
