viirya commented on PR #27111: URL: https://github.com/apache/datafusion/pull/27111#issuecomment-6671832683
Thanks @alamb. I cut the PR down as you suggested: - `null_aware_value_keys` stays a `usize` on `Join` and `HashJoinExec`, and the proto fields keep their type (only their comments change). `0` now means the count is unset, and it is the default of `Join::try_new`, `LogicalPlanBuilder`, `HashJoinExecBuilder::new` and `HashJoinExec::try_new`. An unset count is accepted only for a single-key null-aware join (a scalar `NOT IN`), and a multi-key one fails to plan, asking for `with_null_aware_value_keys`. The decoders drop their `.max(1)`, so a 55.x payload, which decodes the field as `0`, still reads as a scalar `NOT IN`. - The tests are down to one physical test for the count (unset, out of range, `try_new`, and the error a join gets when no count can fix it), the existing proto round trip with an unset count added, and the decorrelation test. - The upgrade guide section is cut from about 430 words to about 160. The changes from the earlier review rounds that still apply are kept: the join type, mode and empty `on` are checked before the count, and decorrelation sets the count only on a null-aware join. On reverting #19857: multi-column `NOT IN` has had several follow-ups since (#26213, #26214, #26647), and #26569 is only reachable by code that builds null-aware joins itself, not from SQL. With this PR such a join fails to plan instead of returning wrong rows, so I would rather keep the feature. I agree that keeping the correlation keys in the same `on` list as the value keys is hard to follow. Separating them is a larger refactor, and I can open an issue for it. -- 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]
