Hi all, While fixing a bug in EXTRACT today [5] that yielded a performance improvement, I decided to look at the rest of date_part.rs:
`EXTRACT` and `date_part` are the same function in DataFusion. PR #26686 [1] fixes four problems in it: - `nanosecond` on a `Duration` or `Interval` counted the whole seconds twice (#26678 [2]). - Every sub-second part dropped the fraction once the value passed about 2.1 seconds (#26678 [2]). - `microsecond` on a long `Duration` wrapped in release builds and panicked in debug builds (#14738 [3]). - `date_part` failed with `declared as non-nullable` when a non-null input gave a NULL (#26685 [4]). The fix reuses arrow's `date_part` where it already computes the right value. It also makes `Duration` and `Interval` 4x to 40x faster. The full sqllogictest suite and the extended tests pass locally, and the standard suite passes in CI on my fork. No existing test expectation changed, because none covered the old behavior. The PR adds those regression tests. I'd like input on two choices before it merges: 1. A `Duration` whose total doesn't fit the `Int32` result of `millisecond` or `microsecond` now returns NULL, which matches what arrow already does for `hour` and `second`. For `microsecond` that is any duration over about 36 minutes. Should it be an error, a wider `Int64` result, or Postgres's within-the-minute meaning for these fields? 2. The integer results of `date_part` are now always nullable, even for non-null input, because arrow can return NULL for values it can't convert (#26685 [4]). Does anyone depend on the non-nullable schema that 53.0.0 introduced? Thanks for your input! -Matt [1] https://github.com/apache/datafusion/pull/26686 [2] https://github.com/apache/datafusion/issues/26678 [3] https://github.com/apache/datafusion/issues/14738 [4] https://github.com/apache/datafusion/issues/26685 [5] https://github.com/apache/datafusion/pull/26676
