2010YOUY01 commented on PR #23828: URL: https://github.com/apache/datafusion/pull/23828#issuecomment-5291909130
Great! I got one tricky bug found by AI, otherwise it should be good to go. ---- Found one blocking correctness issue. - **[P1] Keep float equality groups contiguous under the declared ordering.** [`[try_new](https://github.com/apache/datafusion/blob/70b81458e96c5c5a7e18636b5f13638c19f92715/datafusion/physical-plan/src/joins/asof_join.rs#L187-L208)`](https://github.com/apache/datafusion/blob/70b81458e96c5c5a7e18636b5f13638c19f92715/datafusion/physical-plan/src/joins/asof_join.rs#L187-L208) sorts raw equality keys, where Arrow orders `-0.0` and `+0.0` separately. However, [`[JoinKeyComparator](https://github.com/apache/datafusion/blob/70b81458e96c5c5a7e18636b5f13638c19f92715/datafusion/physical-plan/src/joins/utils.rs#L2380-L2389)`](https://github.com/apache/datafusion/blob/70b81458e96c5c5a7e18636b5f13638c19f92715/datafusion/physical-plan/src/joins/utils.rs#L2380-L2389) normalizes them as SQL-equal. This can break the required secondary match ordering. For example, right rows `(-0, ts=10), (+0, ts=1)` sort in that order; left `(+0, ts=5)` stops at `ts=10` and never reaches the eligible `ts=1`, prod ucing NULL incorrectly. Normalize equality keys consistently for both sorting and comparison, or initially reject float equality keys, and add this regression case. ---- I'm not so sure if there is an easy fix, it it's hard we could first reject floats in the planning, and fix that in a separate PR. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
