[arrow-datafusion] branch main updated: fix: allow group by same expr in Aggregate (#6097)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jakevin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new dfe3534e05 fix: allow group by same expr in Aggregate (#6097)
dfe3534e05 is described below

commit dfe3534e0557cdbd8d06dd338f86847342b0877e
Author: jakevin <ja...@gmail.com>
AuthorDate: Sun Apr 23 21:33:02 2023 +0800

    fix: allow group by same expr in Aggregate (#6097)
---
 datafusion/common/src/dfschema.rs                        | 16 +---------------
 .../core/tests/sqllogictests/test_files/groupby.slt      |  6 ++++++
 datafusion/expr/src/utils.rs                             |  9 ++++-----
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/datafusion/common/src/dfschema.rs b/datafusion/common/src/dfschema.rs
index 23833e8e13..8105ada59f 100644
--- a/datafusion/common/src/dfschema.rs
+++ b/datafusion/common/src/dfschema.rs
@@ -24,7 +24,6 @@ use std::hash::Hash;
 use std::sync::Arc;
 
 use crate::error::{unqualified_field_not_found, DataFusionError, Result, SchemaError};
-use crate::utils::quote_identifier;
 use crate::{field_not_found, Column, OwnedTableReference, TableReference};
 
 use arrow::compute::can_cast_types;
@@ -217,20 +216,7 @@ impl DFSchema {
                 (None, Some(_)) | (None, None) => field.name() == name,
             })
             .map(|(idx, _)| idx);
-        match matches.next() {
-            None => Ok(None),
-            Some(idx) => match matches.next() {
-                None => Ok(Some(idx)),
-                // found more than one matches
-                Some(_) => Err(DataFusionError::Internal(format!(
-                    "Ambiguous reference to qualified field named {}.{}",
-                    qualifier
-                        .map(|q| q.to_quoted_string())
-                        .unwrap_or("<unqualified>".to_string()),
-                    quote_identifier(name)
-                ))),
-            },
-        }
+        Ok(matches.next())
     }
 
     /// Find the index of the column with the given qualifier and name
diff --git a/datafusion/core/tests/sqllogictests/test_files/groupby.slt b/datafusion/core/tests/sqllogictests/test_files/groupby.slt
index bd081048f8..83981095bf 100644
--- a/datafusion/core/tests/sqllogictests/test_files/groupby.slt
+++ b/datafusion/core/tests/sqllogictests/test_files/groupby.slt
@@ -58,3 +58,9 @@ SELECT 38 FROM tab0 AS cor0 GROUP BY cor0.col1, cor0.col1;
 ----
 38
 38
+
+query I rowsort
+SELECT cor0.col1 AS col2 FROM tab0 AS cor0 GROUP BY cor0.col1, cor0.col1;
+----
+0
+81
diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs
index 3aec826d81..e6b0cc43f1 100644
--- a/datafusion/expr/src/utils.rs
+++ b/datafusion/expr/src/utils.rs
@@ -956,13 +956,12 @@ pub fn from_plan(
 }
 
 /// Find all columns referenced from an aggregate query
-fn agg_cols(agg: &Aggregate) -> Result<Vec<Column>> {
-    Ok(agg
-        .aggr_expr
+fn agg_cols(agg: &Aggregate) -> Vec<Column> {
+    agg.aggr_expr
         .iter()
         .chain(&agg.group_expr)
         .flat_map(find_columns_referenced_by_expr)
-        .collect())
+        .collect()
 }
 
 fn exprlist_to_fields_aggregate(
@@ -970,7 +969,7 @@ fn exprlist_to_fields_aggregate(
     plan: &LogicalPlan,
     agg: &Aggregate,
 ) -> Result<Vec<DFField>> {
-    let agg_cols = agg_cols(agg)?;
+    let agg_cols = agg_cols(agg);
     let mut fields = vec![];
     for expr in exprs {
         match expr {