adriangb commented on code in PR #26150:
URL: https://github.com/apache/datafusion/pull/26150#discussion_r4273918944
##########
datafusion/common/src/functional_dependencies.rs:
##########
@@ -480,24 +485,21 @@ pub fn aggregate_functional_dependencies(
{
// Indices into the GROUP BY list for this determinant:
let mut new_source_indices = vec![];
- let mut new_source_field_names = vec![];
- let source_field_names = source_indices
- .iter()
- .map(|&idx| &aggr_input_fields[idx])
- .collect::<Vec<_>>();
-
- for (idx, group_by_expr_name) in
group_by_expr_names.iter().enumerate() {
- // When one of the input determinant expressions matches with
- // the GROUP BY expression, add the index of the GROUP BY
- // expression as a new determinant key:
- if source_field_names.contains(&group_by_expr_name) {
+ let mut new_source_input_indices = vec![];
+ for (idx, input_idx) in group_by_input_indices.iter().enumerate() {
+ // When one of the input determinant columns is a GROUP BY
+ // expression, add the index of the GROUP BY expression as a
new
+ // determinant key:
+ if let Some(input_idx) = input_idx
+ && source_indices.contains(input_idx)
+ {
Review Comment:
addressed
##########
datafusion/sqllogictest/test_files/functional_dependencies.slt:
##########
@@ -358,6 +358,89 @@ drop table t_null;
statement ok
drop table t_probe;
+
+# 6. Only a column reference carries a dependency. `CAST(x AS INT)` is named
+# `t_cast.x`, like the PRIMARY KEY column, but it does not determine `y`:
+# 1.1 and 1.2 both cast to 1.
+statement ok
+CREATE TABLE t_cast (x DOUBLE, y VARCHAR, PRIMARY KEY (x)) AS VALUES (1.1,
'b'), (1.2, 'a'), (2.1, 'c');
+
+# 6.1 ORDER BY: `y` is the tie-breaker within `CAST(x AS INT) = 1` and is kept.
+query RT
+SELECT x, y FROM t_cast ORDER BY CAST(x AS INT), y;
+----
+1.2 a
+1.1 b
+2.1 c
+
+query TT
+EXPLAIN SELECT x, y FROM t_cast ORDER BY CAST(x AS INT), y;
+----
+logical_plan
+01)Sort: CAST(t_cast.x AS Int32) ASC NULLS LAST, t_cast.y ASC NULLS LAST
+02)--TableScan: t_cast projection=[x, y]
+
+# 6.2 GROUP BY: `y` is not selected but still splits the groups.
+query II rowsort
+SELECT CAST(x AS INT) k, count(*) n FROM t_cast GROUP BY CAST(x AS INT), y;
+----
+1 1
+1 1
+2 1
+
+# 6.3 GROUP BY output: the dependency `k -> y` does not exist, so `y` still
+# orders the rows within `k = 1`.
+query IT
+SELECT CAST(x AS INT) k, y FROM t_cast GROUP BY CAST(x AS INT), y ORDER BY k,
y;
+----
+1 a
+1 b
+2 c
+
+# 6.4 `y` is not determined by the GROUP BY expression, so it can't be
selected.
+query error DataFusion error: Error during planning: Column in SELECT must be
in GROUP BY or an aggregate function
+SELECT y, count(*) FROM t_cast GROUP BY CAST(x AS INT);
+
+statement ok
+drop table t_cast;
+
+# 6.5 Only the key column itself carries the dependency, even when the cast
+# keeps distinct values distinct, like INT to BIGINT. PostgreSQL and DuckDB
+# reject this too.
+statement ok
+CREATE TABLE t_wide (x INT, y VARCHAR, PRIMARY KEY (x)) AS VALUES (1, 'b'),
(2, 'a');
+
+query error DataFusion error: Error during planning: Column in SELECT must be
in GROUP BY or an aggregate function
+SELECT y, count(*) FROM t_wide GROUP BY CAST(x AS BIGINT);
+
+statement ok
+drop table t_wide;
+
+# 6.6 The same with `TRY_CAST` over a `UNIQUE NOT NULL` key: 'bad-a' and
+# 'bad-b' both cast to NULL.
+statement ok
+CREATE TABLE t_try_cast (x VARCHAR NOT NULL UNIQUE, y VARCHAR) AS VALUES
('bad-a', 'b'), ('bad-b', 'a'), ('1', 'c');
+
+query TT
+SELECT x, y FROM t_try_cast ORDER BY TRY_CAST(x AS INT), y;
+----
+1 c
+bad-b a
+bad-a b
+
+query IT rowsort
+SELECT TRY_CAST(x AS INT) k, y FROM t_try_cast GROUP BY TRY_CAST(x AS INT), y;
+----
+1 c
+NULL a
+NULL b
+
+query error DataFusion error: Error during planning: Column in SELECT must be
in GROUP BY or an aggregate function
+SELECT y, count(*) FROM t_try_cast GROUP BY TRY_CAST(x AS INT);
+
+statement ok
+drop table t_try_cast;
Review Comment:
addressed
--
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]
