Skip to content

Commit e3af174

Browse files
authored
Remove expr_fn::sum and replace them with function stub (#10816)
* introduce stub for test Signed-off-by: jayzhan211 <[email protected]> * fix err msg Signed-off-by: jayzhan211 <[email protected]> * dont compare error msg, ci is not consistent with local Signed-off-by: jayzhan211 <[email protected]> * comment and cli update Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> --------- Signed-off-by: jayzhan211 <[email protected]>
1 parent c012e9c commit e3af174

15 files changed

+246
-82
lines changed

datafusion-cli/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/expr/src/expr_fn.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,6 @@ pub fn max(expr: Expr) -> Expr {
168168
))
169169
}
170170

171-
/// Create an expression to represent the sum() aggregate function
172-
///
173-
/// TODO: Remove this function and use `sum` from `datafusion_functions_aggregate::expr_fn` instead
174-
pub fn sum(expr: Expr) -> Expr {
175-
Expr::AggregateFunction(AggregateFunction::new(
176-
aggregate_function::AggregateFunction::Sum,
177-
vec![expr],
178-
false,
179-
None,
180-
None,
181-
None,
182-
))
183-
}
184-
185171
/// Create an expression to represent the array_agg() aggregate function
186172
pub fn array_agg(expr: Expr) -> Expr {
187173
Expr::AggregateFunction(AggregateFunction::new(

datafusion/expr/src/logical_plan/builder.rs

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,7 +1719,7 @@ pub fn unnest_with_options(
17191719
mod tests {
17201720
use super::*;
17211721
use crate::logical_plan::StringifiedPlan;
1722-
use crate::{col, expr, expr_fn::exists, in_subquery, lit, scalar_subquery, sum};
1722+
use crate::{col, expr, expr_fn::exists, in_subquery, lit, scalar_subquery};
17231723

17241724
use datafusion_common::SchemaError;
17251725

@@ -1775,28 +1775,6 @@ mod tests {
17751775
);
17761776
}
17771777

1778-
#[test]
1779-
fn plan_builder_aggregate() -> Result<()> {
1780-
let plan =
1781-
table_scan(Some("employee_csv"), &employee_schema(), Some(vec![3, 4]))?
1782-
.aggregate(
1783-
vec![col("state")],
1784-
vec![sum(col("salary")).alias("total_salary")],
1785-
)?
1786-
.project(vec![col("state"), col("total_salary")])?
1787-
.limit(2, Some(10))?
1788-
.build()?;
1789-
1790-
let expected = "Limit: skip=2, fetch=10\
1791-
\n Projection: employee_csv.state, total_salary\
1792-
\n Aggregate: groupBy=[[employee_csv.state]], aggr=[[SUM(employee_csv.salary) AS total_salary]]\
1793-
\n TableScan: employee_csv projection=[state, salary]";
1794-
1795-
assert_eq!(expected, format!("{plan:?}"));
1796-
1797-
Ok(())
1798-
}
1799-
18001778
#[test]
18011779
fn plan_builder_sort() -> Result<()> {
18021780
let plan =
@@ -2037,36 +2015,6 @@ mod tests {
20372015
}
20382016
}
20392017

2040-
#[test]
2041-
fn aggregate_non_unique_names() -> Result<()> {
2042-
let plan = table_scan(
2043-
Some("employee_csv"),
2044-
&employee_schema(),
2045-
// project state and salary by column index
2046-
Some(vec![3, 4]),
2047-
)?
2048-
// two columns with the same name => error
2049-
.aggregate(vec![col("state")], vec![sum(col("salary")).alias("state")]);
2050-
2051-
match plan {
2052-
Err(DataFusionError::SchemaError(
2053-
SchemaError::AmbiguousReference {
2054-
field:
2055-
Column {
2056-
relation: Some(TableReference::Bare { table }),
2057-
name,
2058-
},
2059-
},
2060-
_,
2061-
)) => {
2062-
assert_eq!(*"employee_csv", *table);
2063-
assert_eq!("state", &name);
2064-
Ok(())
2065-
}
2066-
_ => plan_err!("Plan should have returned an DataFusionError::SchemaError"),
2067-
}
2068-
}
2069-
20702018
fn employee_schema() -> Schema {
20712019
Schema::new(vec![
20722020
Field::new("id", DataType::Int32, false),

datafusion/optimizer/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ hashbrown = { workspace = true }
5050
indexmap = { workspace = true }
5151
itertools = { workspace = true }
5252
log = { workspace = true }
53+
paste = "1.0.14"
5354
regex-syntax = "0.8.0"
5455

5556
[dev-dependencies]

datafusion/optimizer/src/analyzer/count_wildcard_rule.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,14 @@ fn analyze_internal(plan: LogicalPlan) -> Result<Transformed<LogicalPlan>> {
117117
#[cfg(test)]
118118
mod tests {
119119
use super::*;
120+
use crate::test::function_stub::sum;
120121
use crate::test::*;
121122
use arrow::datatypes::DataType;
122123
use datafusion_common::ScalarValue;
123124
use datafusion_expr::expr::Sort;
124125
use datafusion_expr::{
125126
col, count, exists, expr, in_subquery, logical_plan::LogicalPlanBuilder, max,
126-
out_ref_col, scalar_subquery, sum, wildcard, AggregateFunction, WindowFrame,
127+
out_ref_col, scalar_subquery, wildcard, AggregateFunction, WindowFrame,
127128
WindowFrameBound, WindowFrameUnits,
128129
};
129130
use std::sync::Arc;
@@ -275,11 +276,9 @@ mod tests {
275276
#[test]
276277
fn test_count_wildcard_on_non_count_aggregate() -> Result<()> {
277278
let table_scan = test_table_scan()?;
278-
let err = LogicalPlanBuilder::from(table_scan)
279-
.aggregate(Vec::<Expr>::new(), vec![sum(wildcard())])
280-
.unwrap_err()
281-
.to_string();
282-
assert!(err.contains("Error during planning: No function matches the given name and argument types 'SUM(Null)'."), "{err}");
279+
let res = LogicalPlanBuilder::from(table_scan)
280+
.aggregate(Vec::<Expr>::new(), vec![sum(wildcard())]);
281+
assert!(res.is_err());
283282
Ok(())
284283
}
285284

datafusion/optimizer/src/common_subexpr_eliminate.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,13 +840,15 @@ mod test {
840840
use arrow::datatypes::Schema;
841841

842842
use datafusion_expr::logical_plan::{table_scan, JoinType};
843-
use datafusion_expr::{avg, lit, logical_plan::builder::LogicalPlanBuilder, sum};
843+
844+
use datafusion_expr::{avg, lit, logical_plan::builder::LogicalPlanBuilder};
844845
use datafusion_expr::{
845846
grouping_set, AccumulatorFactoryFunction, AggregateUDF, Signature,
846847
SimpleAggregateUDF, Volatility,
847848
};
848849

849850
use crate::optimizer::OptimizerContext;
851+
use crate::test::function_stub::sum;
850852
use crate::test::*;
851853

852854
use super::*;

datafusion/optimizer/src/eliminate_filter.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,11 @@ mod tests {
9191

9292
use datafusion_common::{Result, ScalarValue};
9393
use datafusion_expr::{
94-
col, lit, logical_plan::builder::LogicalPlanBuilder, sum, Expr, LogicalPlan,
94+
col, lit, logical_plan::builder::LogicalPlanBuilder, Expr, LogicalPlan,
9595
};
9696

9797
use crate::eliminate_filter::EliminateFilter;
98+
use crate::test::function_stub::sum;
9899
use crate::test::*;
99100

100101
fn assert_optimized_plan_equal(plan: LogicalPlan, expected: &str) -> Result<()> {

datafusion/optimizer/src/eliminate_limit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ mod tests {
100100
use datafusion_expr::{
101101
col,
102102
logical_plan::{builder::LogicalPlanBuilder, JoinType},
103-
sum,
104103
};
105104
use std::sync::Arc;
106105

107106
use crate::push_down_limit::PushDownLimit;
107+
use crate::test::function_stub::sum;
108108

109109
fn observe(_plan: &LogicalPlan, _rule: &dyn OptimizerRule) {}
110110
fn assert_optimized_plan_eq(plan: LogicalPlan, expected: &str) -> Result<()> {

datafusion/optimizer/src/push_down_filter.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1090,13 +1090,14 @@ mod tests {
10901090
use datafusion_expr::expr::ScalarFunction;
10911091
use datafusion_expr::logical_plan::table_scan;
10921092
use datafusion_expr::{
1093-
col, in_list, in_subquery, lit, sum, ColumnarValue, Extension, ScalarUDF,
1093+
col, in_list, in_subquery, lit, ColumnarValue, Extension, ScalarUDF,
10941094
ScalarUDFImpl, Signature, TableSource, TableType, UserDefinedLogicalNodeCore,
10951095
Volatility,
10961096
};
10971097

10981098
use crate::optimizer::Optimizer;
10991099
use crate::rewrite_disjunctive_predicate::RewriteDisjunctivePredicate;
1100+
use crate::test::function_stub::sum;
11001101
use crate::test::*;
11011102
use crate::OptimizerContext;
11021103

datafusion/optimizer/src/scalar_subquery_to_join.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,9 @@ mod tests {
400400
use super::*;
401401
use crate::test::*;
402402

403+
use crate::test::function_stub::sum;
403404
use arrow::datatypes::DataType;
404-
use datafusion_expr::{
405-
col, lit, max, min, out_ref_col, scalar_subquery, sum, Between,
406-
};
405+
use datafusion_expr::{col, lit, max, min, out_ref_col, scalar_subquery, Between};
407406

408407
/// Test multiple correlated subqueries
409408
#[test]

datafusion/optimizer/src/single_distinct_to_groupby.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,13 @@ impl OptimizerRule for SingleDistinctToGroupBy {
360360
#[cfg(test)]
361361
mod tests {
362362
use super::*;
363+
use crate::test::function_stub::sum;
363364
use crate::test::*;
364365
use datafusion_expr::expr;
365366
use datafusion_expr::expr::GroupingSet;
366367
use datafusion_expr::{
367368
count, count_distinct, lit, logical_plan::builder::LogicalPlanBuilder, max, min,
368-
sum, AggregateFunction,
369+
AggregateFunction,
369370
};
370371

371372
fn assert_optimized_plan_equal(plan: LogicalPlan, expected: &str) -> Result<()> {

0 commit comments

Comments
 (0)