jayzhan211 commented on code in PR #26150:
URL: https://github.com/apache/datafusion/pull/26150#discussion_r4271135350


##########
datafusion/expr/src/utils.rs:
##########
@@ -1020,6 +1020,20 @@ pub(crate) fn find_columns_referenced_by_expr(e: &Expr) 
-> Vec<Column> {
     exprs
 }
 
+/// Returns the index of the `schema` field that `expr` passes through
+/// unchanged, or `None` if `expr` computes a new value.
+///
+/// Only a column reference (possibly aliased) passes a field through. Use this
+/// instead of comparing [`Expr::schema_name`] with field names: different
+/// expressions can have the same name, e.g. `CAST(t.a AS INT)` is named `t.a`.
+pub fn passthrough_field_index(expr: &Expr, schema: &DFSchema) -> 
Option<usize> {
+    match expr {
+        Expr::Column(col) => schema.maybe_index_of_column(col),

Review Comment:
   Fixed in 9d9f9f3805. `passthrough_field_index` now resolves the column with 
`qualified_field_from_column`, like any other column reference, so an ambiguous 
`id` over `a.id` and `b.id` passes nothing through. The unit test 
`passthrough_field_index_resolves_like_a_column_reference` covers it.



##########
datafusion/sqllogictest/test_files/functional_dependencies.slt:
##########
@@ -358,6 +358,52 @@ 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);

Review Comment:
   Thanks! Added as 6.6 in 9d9f9f3805: `'bad-a'` and `'bad-b'` both cast to 
NULL, ORDER BY keeps `y`, GROUP BY keeps both NULL groups, and selecting `y` 
while grouping only by the cast fails planning.



-- 
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]

Reply via email to