alamb commented on code in PR #14824:
URL: https://github.com/apache/datafusion/pull/14824#discussion_r1971606770
##########
datafusion/optimizer/tests/optimizer_integration.rs:
##########
@@ -198,7 +198,7 @@ fn between_date32_plus_interval() -> Result<()> {
WHERE col_date32 between '1998-03-18' AND cast('1998-03-18' as date) +
INTERVAL '90 days'";
let plan = test_sql(sql)?;
let expected =
- "Aggregate: groupBy=[[]], aggr=[[count(*)]]\
+ "Aggregate: groupBy=[[]], aggr=[[count(Int64(1))]]\
Review Comment:
this certainly seems an improvement
##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -2455,7 +2455,7 @@ async fn test_count_wildcard_on_sort() -> Result<()> {
let ctx = create_join_context()?;
let sql_results = ctx
- .sql("select b,count(*) from t1 group by b order by count(*)")
+ .sql("select b,count(1) from t1 group by b order by count(1)")
Review Comment:
I found I could avoid the double alias by adding a check in `Expr::alias`:
```diff
diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs
index f8baf9c94..2f3c2c575 100644
--- a/datafusion/expr/src/expr.rs
+++ b/datafusion/expr/src/expr.rs
@@ -1276,7 +1276,14 @@ impl Expr {
/// Return `self AS name` alias expression
pub fn alias(self, name: impl Into<String>) -> Expr {
- Expr::Alias(Alias::new(self, None::<&str>, name.into()))
+ let name = name.into();
+ // don't realias the same thing
+ if matches!(&self, Expr::Alias(Alias {name: existing_name, ..} ) if
existing_name == &name)
+ {
+ self
+ } else {
+ Expr::Alias(Alias::new(self, None::<&str>, name))
+ }
}
/// Return `self AS name` alias expression with a specific qualifier
@@ -1285,7 +1292,15 @@ impl Expr {
relation: Option<impl Into<TableReference>>,
name: impl Into<String>,
) -> Expr {
- Expr::Alias(Alias::new(self, relation, name.into()))
+ let relation = relation.map(|r| r.into());
+ let name = name.into();
+ // don't realias the same thing
+ if matches!(&self, Expr::Alias(Alias {name: existing_name,
relation: existing_relation, ..} ) if existing_name == &name &&
relation.as_ref()==existing_relation.as_ref() )
+ {
+ self
+ } else {
+ Expr::Alias(Alias::new(self, relation, name))
+ }
}
/// Remove an alias from an expression if one exists.
diff --git a/datafusion/functions-aggregate/src/count.rs
b/datafusion/functions-aggregate/src/count.rs
index a3339f0fc..1faf1968b 100644
--- a/datafusion/functions-aggregate/src/count.rs
+++ b/datafusion/functions-aggregate/src/count.rs
@@ -81,7 +81,7 @@ pub fn count_distinct(expr: Expr) -> Expr {
/// Creates aggregation to count all rows, equivalent to `COUNT(*)`,
`COUNT()`, `COUNT(1)`
pub fn count_all() -> Expr {
- count(Expr::Literal(COUNT_STAR_EXPANSION))
+ count(Expr::Literal(COUNT_STAR_EXPANSION)).alias("count(*)")
}
#[user_doc(
```
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
