You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/07/24 16:23:15 UTC
(datafusion) 01/01: test: add functional_dependencies.slt covering functional dependency driven optimizations (#23821)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-23821-582453b680e7a9b927d659753edb8f33a444a3c8
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit efa84e8309ced6614e62d1607899c47ed0b6b08a
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Fri Jul 24 12:22:52 2026 -0400
test: add functional_dependencies.slt covering functional dependency driven optimizations (#23821)
## Which issue does this PR close?
This PR adds test coverage rather than closing an issue. It documents
the current behavior of these bugs so that fixing them shows up as a
test change:
- https://github.com/apache/datafusion/issues/23634
- https://github.com/apache/datafusion/issues/23818
- https://github.com/apache/datafusion/issues/23819
- https://github.com/apache/datafusion/issues/23820
## Rationale for this change
While reviewing #23636 @neilconway and I kept coming up with more
examples of bad plans, and it was not obvious which of the surrounding
wrong answers were pre-existing and which the PR introduced.
## What changes are included in this PR?
A new `datafusion/sqllogictest/test_files/functional_dependencies.slt`
with one section per consumer of functional dependencies:
1. `ReplaceDistinctWithAggregate` — removing `DISTINCT`
2. `eliminate_duplicated_expr` — dropping trailing `ORDER BY` keys
3. `optimize_projections` — dropping `GROUP BY` expressions
4. `add_group_by_exprs_from_dependencies` — selecting non-grouped
columns
5. `GROUP BY` derived keys on the NULL-padded side of an outer join
Cases that currently return wrong answers are labelled `BUG` with the
expected
result and a link to the issue:
| Case | Symptom | Issue |
| --- | --- | --- |
| 1.2 | `DISTINCT` over a nullable `UNIQUE` column returns both `NULL`
rows | #23634 |
| 2.2 | `ORDER BY x, y` drops the `y` key, so the `NULL` rows come back
unordered | #23818 |
| 3.2 | `GROUP BY x, y` drops `y`, merging the two `NULL` groups and
losing a row | #23819 |
| 4.2 | `SELECT x, y ... GROUP BY x` returns two rows for the `x = NULL`
group | #23820 |
## Are these changes tested?
CI
## Are there any user-facing changes?
No.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <no...@anthropic.com>
---
.../test_files/functional_dependencies.slt | 314 +++++++++++++++++++++
1 file changed, 314 insertions(+)
diff --git a/datafusion/sqllogictest/test_files/functional_dependencies.slt b/datafusion/sqllogictest/test_files/functional_dependencies.slt
new file mode 100644
index 0000000000..92aedf66e6
--- /dev/null
+++ b/datafusion/sqllogictest/test_files/functional_dependencies.slt
@@ -0,0 +1,314 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+##########
+# Tests for functional dependencies
+# (`datafusion/common/src/functional_dependencies.rs`)
+#
+# A functional dependency records that one set of columns (the *determinant*)
+# determines the values of the others. DataFusion derives them from PRIMARY
+# KEY / UNIQUE constraints and from GROUP BY keys, and four optimizer rules
+# consume them to remove redundant work, each tested here in a different section.
+#
+# NULL handling is (as always) important:
+#
+# * A PRIMARY KEY is unique AND not nullable.
+# * A `UNIQUE` constraint permits *multiple NULL rows*, because NULLs
+# compare distinct.
+#
+# It is important not to mix `UNIQUE` columns with `DISTINCT` or `GROUP BY`,
+# which treat NULLs as equal and can produce wrong answers.
+##########
+
+# These rules all run during logical optimization, so show only logical plans.
+statement ok
+set datafusion.explain.logical_plan_only = true;
+
+# Set target_partitions explicitly so query results are stable.
+statement ok
+set datafusion.execution.target_partitions = 4;
+
+##########
+## Test tables
+##########
+
+statement ok
+CREATE TABLE t_pk (x INT, y INT, PRIMARY KEY (x)) AS VALUES (1, 10), (2, 20);
+
+statement ok
+CREATE TABLE t_uniq (x INT UNIQUE, y INT) AS VALUES (NULL, 2), (NULL, 1), (1, 3);
+
+query II rowsort
+SELECT x, y FROM t_uniq;
+----
+1 3
+NULL 1
+NULL 2
+
+
+# 1.1 PRIMARY KEY: rows are unique; the DISTINCT is removed and no
+# Aggregate appears in the plan.
+query TT
+EXPLAIN SELECT DISTINCT x FROM t_pk;
+----
+logical_plan TableScan: t_pk projection=[x]
+
+# 1.2 Nullable UNIQUE: the DISTINCT must be KEPT. UNIQUE allows several NULL
+# rows, but DISTINCT treats NULLs as equal and has to collapse them into one.
+#
+# BUG: the DISTINCT is removed and both NULL rows are returned.
+# Expected: `1`, `NULL`.
+# Issue: https://github.com/apache/datafusion/issues/23634
+query I
+SELECT DISTINCT x FROM t_uniq ORDER BY x NULLS LAST;
+----
+1
+NULL
+NULL
+
+query TT
+EXPLAIN SELECT DISTINCT x FROM t_uniq;
+----
+logical_plan TableScan: t_uniq projection=[x]
+
+# 1.3 A PRIMARY KEY downgraded to a non-unique dependency by a LEFT JOIN
+# so the DISTINCT must be KEPT.
+# Fixed by: https://github.com/apache/datafusion/pull/23548
+statement ok
+CREATE TABLE t_orders (x INT, amount INT) AS VALUES (1, 10), (1, 20), (2, 30);
+
+query I
+SELECT DISTINCT p.x FROM t_pk p LEFT JOIN t_orders o ON p.x = o.x ORDER BY p.x;
+----
+1
+2
+
+query TT
+EXPLAIN SELECT DISTINCT p.x FROM t_pk p LEFT JOIN t_orders o ON p.x = o.x;
+----
+logical_plan
+01)Aggregate: groupBy=[[p.x]], aggr=[[]]
+02)--Projection: p.x
+03)----Left Join: p.x = o.x
+04)------SubqueryAlias: p
+05)--------TableScan: t_pk projection=[x]
+06)------SubqueryAlias: o
+07)--------TableScan: t_orders projection=[x]
+
+statement ok
+drop table t_orders;
+
+# 1.4 DISTINCT over a GROUP BY output. Grouping collapses the multiple NULL
+# rows, (NULL included) and the DISTINCT can be removed.
+query I
+SELECT DISTINCT x FROM (SELECT x FROM t_uniq GROUP BY x) ORDER BY x NULLS LAST;
+----
+1
+NULL
+
+query TT
+EXPLAIN SELECT DISTINCT x FROM (SELECT x FROM t_uniq GROUP BY x);
+----
+logical_plan
+01)Aggregate: groupBy=[[t_uniq.x]], aggr=[[]]
+02)--TableScan: t_uniq projection=[x]
+
+
+# 2.1 PRIMARY KEY: `x` determines `y`, so `ORDER BY x, y` is equivalent to
+# `ORDER BY x` and the `y` key is dropped from the plan.
+query TT
+EXPLAIN SELECT x, y FROM t_pk ORDER BY x, y;
+----
+logical_plan
+01)Sort: t_pk.x ASC NULLS LAST
+02)--TableScan: t_pk projection=[x, y]
+
+# 2.2 Nullable UNIQUE: `x` does NOT determine `y` across the two NULL rows,
+# so the `y` sort key must be kept.
+#
+# BUG:
+# Expected: `1 3`, `NULL 1`, `NULL 2`.
+# Issue: https://github.com/apache/datafusion/issues/23818
+query II
+SELECT x, y FROM t_uniq ORDER BY x NULLS LAST, y;
+----
+1 3
+NULL 2
+NULL 1
+
+query TT
+EXPLAIN SELECT x, y FROM t_uniq ORDER BY x NULLS LAST, y;
+----
+logical_plan
+01)Sort: t_uniq.x ASC NULLS LAST
+02)--TableScan: t_uniq projection=[x, y]
+
+# 2.3 After `GROUP BY x` the `x` does determine `cnt`, so can drop `cnt` from sort
+query TT
+EXPLAIN SELECT x, cnt FROM (SELECT x, count(*) AS cnt FROM t_uniq GROUP BY x) ORDER BY x, cnt;
+----
+logical_plan
+01)Sort: t_uniq.x ASC NULLS LAST
+02)--Projection: t_uniq.x, count(Int64(1)) AS cnt
+03)----Aggregate: groupBy=[[t_uniq.x]], aggr=[[count(Int64(1))]]
+04)------TableScan: t_uniq projection=[x]
+
+
+# 3.1 PRIMARY KEY: `x` determines `y`, and `y` is not selected, so grouping
+# by `x, y` is the same as grouping by `x`.
+query TT
+EXPLAIN SELECT x FROM t_pk GROUP BY x, y;
+----
+logical_plan
+01)Aggregate: groupBy=[[t_pk.x]], aggr=[[]]
+02)--TableScan: t_pk projection=[x]
+
+# 3.2 Nullable UNIQUE: grouping by `x, y` is NOT the same as grouping by
+# `x` -- two NULL rows differ in `y` and belong in separate groups.
+#
+# BUG: `y` is dropped from the GROUP BY and the two NULL groups are merged,
+# so one row goes missing.
+# Expected: `1`, `NULL`, `NULL` (three rows).
+# Issue: https://github.com/apache/datafusion/issues/23819
+query I rowsort
+SELECT x FROM t_uniq GROUP BY x, y;
+----
+1
+NULL
+
+query TT
+EXPLAIN SELECT x FROM t_uniq GROUP BY x, y;
+----
+logical_plan
+01)Aggregate: groupBy=[[t_uniq.x]], aggr=[[]]
+02)--TableScan: t_uniq projection=[x]
+
+# 3.3 The same grouping, but with `y` selected so the parent needs it: no
+# column can be dropped and the answer is right.
+query II rowsort
+SELECT x, y FROM t_uniq GROUP BY x, y;
+----
+1 3
+NULL 1
+NULL 2
+
+# 4.1 PRIMARY KEY: `x` determines `y`, so `y` has a single well-defined
+# value per group and one row is returned per `x`.
+query II rowsort
+SELECT x, y FROM t_pk GROUP BY x;
+----
+1 10
+2 20
+
+query TT
+EXPLAIN SELECT x, y FROM t_pk GROUP BY x;
+----
+logical_plan
+01)Aggregate: groupBy=[[t_pk.x, t_pk.y]], aggr=[[]]
+02)--TableScan: t_pk projection=[x, y]
+
+# 4.2 Nullable UNIQUE: `x` does NOT determine `y`, so there is no
+# well-defined `y` for the `x = NULL` group.
+#
+# BUG: `y` is appended to the GROUP BY anyway, so `GROUP BY x` returns TWO
+# rows for `x = NULL`.
+# Expected: one row per distinct `x` (or a planning error -- postgres
+# rejects this query, and accepts the 4.1 PRIMARY KEY form).
+# Issue: https://github.com/apache/datafusion/issues/23820
+query II rowsort
+SELECT x, y FROM t_uniq GROUP BY x;
+----
+1 3
+NULL 1
+NULL 2
+
+query TT
+EXPLAIN SELECT x, y FROM t_uniq GROUP BY x;
+----
+logical_plan
+01)Aggregate: groupBy=[[t_uniq.x, t_uniq.y]], aggr=[[]]
+02)--TableScan: t_uniq projection=[x, y]
+
+
+statement ok
+CREATE TABLE t_null (x INT) AS VALUES (NULL), (NULL);
+
+statement ok
+CREATE TABLE t_probe (z INT) AS VALUES (0), (2);
+
+# 5.1 Grouping by `g.x, g.cnt` must keep both columns: `g.x` alone does not
+# determine `g.cnt` after NULL padding.
+query II
+SELECT g.x, count(*) AS c
+ FROM t_probe a
+ LEFT JOIN (SELECT x, count(*) AS cnt FROM t_null GROUP BY x) g
+ ON a.z = g.cnt
+ GROUP BY g.x, g.cnt
+ ORDER BY c;
+----
+NULL 1
+NULL 1
+
+query TT
+EXPLAIN SELECT g.x, count(*) AS c
+ FROM t_probe a
+ LEFT JOIN (SELECT x, count(*) AS cnt FROM t_null GROUP BY x) g
+ ON a.z = g.cnt
+ GROUP BY g.x, g.cnt;
+----
+logical_plan
+01)Projection: g.x, count(Int64(1)) AS count(*) AS c
+02)--Aggregate: groupBy=[[g.x, g.cnt]], aggr=[[count(Int64(1))]]
+03)----Projection: g.x, g.cnt
+04)------Left Join: CAST(a.z AS Int64) = g.cnt
+05)--------SubqueryAlias: a
+06)----------TableScan: t_probe projection=[z]
+07)--------SubqueryAlias: g
+08)----------Projection: t_null.x, count(Int64(1)) AS count(*) AS cnt
+09)------------Aggregate: groupBy=[[t_null.x]], aggr=[[count(Int64(1))]]
+10)--------------TableScan: t_null projection=[x]
+
+# 5.2 The ORDER BY variant: `g.x` is NULL for both rows, so the `g.cnt`
+# tie-breaker is what orders them.
+query II
+SELECT g.x, g.cnt
+ FROM t_probe a
+ LEFT JOIN (SELECT x, count(*) AS cnt FROM t_null GROUP BY x) g
+ ON a.z = g.cnt
+ ORDER BY g.x, g.cnt;
+----
+NULL 2
+NULL NULL
+
+statement ok
+drop table t_null;
+
+statement ok
+drop table t_probe;
+
+##########
+## Cleanup
+##########
+
+statement ok
+drop table t_pk;
+
+statement ok
+drop table t_uniq;
+
+statement ok
+RESET datafusion.explain.logical_plan_only;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org