kosiew commented on code in PR #25772:
URL: https://github.com/apache/datafusion/pull/25772#discussion_r4284301283
##########
datafusion/sqllogictest/test_files/join_is_not_distinct_from.slt:
##########
@@ -321,6 +321,36 @@ JOIN t4 ON (t3.id = t4.id) AND (t3.val1 IS NOT DISTINCT
FROM t4.val1) AND (t3.va
2 2 NULL NULL 200 200
3 3 30 30 NULL NULL
+# An IS NOT DISTINCT FROM join nested under an inner join on other keys keeps
+# matching NULL keys (EliminateCrossJoin used to rebuild it with the outer
+# join's null equality)
+statement ok
+CREATE TABLE nd1(a INT, b INT) AS VALUES (NULL, 1), (5, 2)
+
+statement ok
+CREATE TABLE nd2(a INT) AS VALUES (NULL), (5)
+
+statement ok
+CREATE TABLE nd3(b INT) AS VALUES (1), (2)
+
+query IIII
+SELECT nd1.a, nd1.b, nd2.a, nd3.b
Review Comment:
Could we also test the reverse case using `nd1 JOIN nd2 ON nd1.a = nd2.a
JOIN nd3 ON nd1.b IS NOT DISTINCT FROM nd3.b`, expecting only `5 2 5 2`? This
would confirm that ordinary equality does not start matching NULL keys when the
outer join uses NULL-safe equality. The current fix already handles this, so
it's optional.
##########
datafusion/optimizer/src/eliminate_cross_join.rs:
##########
@@ -258,26 +260,36 @@ fn rewrite_children(
/// Recursively collect inputs, join keys, and filters from inner joins
/// (including cross joins). Other nodes are retained as inputs without
/// flattening them.
+///
+/// The joins are rebuilt with `null_equality`, so an inner join whose keys
+/// compare NULLs differently is kept as an input: flattening it would change
+/// whether its NULL keys match.
fn flatten_join_inputs(
plan: LogicalPlan,
+ null_equality: NullEquality,
possible_join_keys: &mut JoinKeySet,
all_inputs: &mut Vec<LogicalPlan>,
all_filters: &mut Vec<Expr>,
) {
match plan {
- LogicalPlan::Join(join) if join.join_type == JoinType::Inner => {
+ LogicalPlan::Join(join)
+ if join.join_type == JoinType::Inner
+ && (join.on.is_empty() || join.null_equality == null_equality)
=>
Review Comment:
Could we add a test with a nested keyless `NullEqualsNothing` inner join
beneath a keyed `NullEqualsNull` join, with the outer keys connecting both
nested inputs to the third table? It would verify that the keyless join can
still be flattened and its cross product eliminated despite the different NULL
equality settings. This is optional since the existing guard looks correct.
--
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]
