This is an automated email from the ASF dual-hosted git repository.
jayzhan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 016ed03cf1 Remove redundant result of AggregateFunctionExpr::field
(#12258)
016ed03cf1 is described below
commit 016ed03cf18c3f60594cf6f8fedd202561c27a91
Author: 张林伟 <[email protected]>
AuthorDate: Sun Sep 1 15:58:53 2024 +0800
Remove redundant result of AggregateFunctionExpr::field (#12258)
---
datafusion/physical-expr/src/aggregate.rs | 12 +++---------
datafusion/physical-expr/src/window/aggregate.rs | 4 ++--
datafusion/physical-expr/src/window/sliding_aggregate.rs | 4 ++--
datafusion/physical-optimizer/src/aggregate_statistics.rs | 4 ++--
datafusion/physical-plan/src/aggregates/mod.rs | 2 +-
datafusion/proto/src/physical_plan/mod.rs | 7 ++-----
6 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/datafusion/physical-expr/src/aggregate.rs
b/datafusion/physical-expr/src/aggregate.rs
index 5c1216f2a3..d62dc27ece 100644
--- a/datafusion/physical-expr/src/aggregate.rs
+++ b/datafusion/physical-expr/src/aggregate.rs
@@ -253,12 +253,8 @@ impl AggregateFunctionExpr {
}
/// the field of the final result of this aggregation.
- pub fn field(&self) -> Result<Field> {
- Ok(Field::new(
- &self.name,
- self.data_type.clone(),
- self.is_nullable,
- ))
+ pub fn field(&self) -> Field {
+ Field::new(&self.name, self.data_type.clone(), self.is_nullable)
}
/// the accumulator used to accumulate values from the expressions.
@@ -523,9 +519,7 @@ impl AggregateFunctionExpr {
///
/// Note: this is used to use special aggregate implementations in certain
conditions
pub fn get_minmax_desc(&self) -> Option<(Field, bool)> {
- self.fun
- .is_descending()
- .and_then(|flag| self.field().ok().map(|f| (f, flag)))
+ self.fun.is_descending().map(|flag| (self.field(), flag))
}
/// Returns default value of the function given the input is Null
diff --git a/datafusion/physical-expr/src/window/aggregate.rs
b/datafusion/physical-expr/src/window/aggregate.rs
index 5439e14050..1cc08a4e99 100644
--- a/datafusion/physical-expr/src/window/aggregate.rs
+++ b/datafusion/physical-expr/src/window/aggregate.rs
@@ -79,7 +79,7 @@ impl WindowExpr for PlainAggregateWindowExpr {
}
fn field(&self) -> Result<Field> {
- self.aggregate.field()
+ Ok(self.aggregate.field())
}
fn name(&self) -> &str {
@@ -177,7 +177,7 @@ impl AggregateWindowExpr for PlainAggregateWindowExpr {
) -> Result<ScalarValue> {
if cur_range.start == cur_range.end {
self.aggregate
- .default_value(self.aggregate.field()?.data_type())
+ .default_value(self.aggregate.field().data_type())
} else {
// Accumulate any new rows that have entered the window:
let update_bound = cur_range.end - last_range.end;
diff --git a/datafusion/physical-expr/src/window/sliding_aggregate.rs
b/datafusion/physical-expr/src/window/sliding_aggregate.rs
index ac3a4f4c09..b3848e15ee 100644
--- a/datafusion/physical-expr/src/window/sliding_aggregate.rs
+++ b/datafusion/physical-expr/src/window/sliding_aggregate.rs
@@ -81,7 +81,7 @@ impl WindowExpr for SlidingAggregateWindowExpr {
}
fn field(&self) -> Result<Field> {
- self.aggregate.field()
+ Ok(self.aggregate.field())
}
fn name(&self) -> &str {
@@ -183,7 +183,7 @@ impl AggregateWindowExpr for SlidingAggregateWindowExpr {
) -> Result<ScalarValue> {
if cur_range.start == cur_range.end {
self.aggregate
- .default_value(self.aggregate.field()?.data_type())
+ .default_value(self.aggregate.field().data_type())
} else {
// Accumulate any new rows that have entered the window:
let update_bound = cur_range.end - last_range.end;
diff --git a/datafusion/physical-optimizer/src/aggregate_statistics.rs
b/datafusion/physical-optimizer/src/aggregate_statistics.rs
index 2b8725b5ba..863c5ab2d2 100644
--- a/datafusion/physical-optimizer/src/aggregate_statistics.rs
+++ b/datafusion/physical-optimizer/src/aggregate_statistics.rs
@@ -183,7 +183,7 @@ fn take_optimizable_min(
// MIN/MAX with 0 rows is always null
if is_min(agg_expr) {
if let Ok(min_data_type) =
-
ScalarValue::try_from(agg_expr.field().unwrap().data_type())
+ ScalarValue::try_from(agg_expr.field().data_type())
{
return Some((min_data_type,
agg_expr.name().to_string()));
}
@@ -229,7 +229,7 @@ fn take_optimizable_max(
// MIN/MAX with 0 rows is always null
if is_max(agg_expr) {
if let Ok(max_data_type) =
-
ScalarValue::try_from(agg_expr.field().unwrap().data_type())
+ ScalarValue::try_from(agg_expr.field().data_type())
{
return Some((max_data_type,
agg_expr.name().to_string()));
}
diff --git a/datafusion/physical-plan/src/aggregates/mod.rs
b/datafusion/physical-plan/src/aggregates/mod.rs
index 0f33a9d7b9..2f974f1ef4 100644
--- a/datafusion/physical-plan/src/aggregates/mod.rs
+++ b/datafusion/physical-plan/src/aggregates/mod.rs
@@ -821,7 +821,7 @@ fn create_schema(
| AggregateMode::SinglePartitioned => {
// in final mode, the field with the final result of the
accumulator
for expr in aggr_expr {
- fields.push(expr.field()?)
+ fields.push(expr.field())
}
}
}
diff --git a/datafusion/proto/src/physical_plan/mod.rs
b/datafusion/proto/src/physical_plan/mod.rs
index 04cbf8b537..e622af7450 100644
--- a/datafusion/proto/src/physical_plan/mod.rs
+++ b/datafusion/proto/src/physical_plan/mod.rs
@@ -1444,11 +1444,8 @@ impl AsExecutionPlan for protobuf::PhysicalPlanNode {
let agg_names = exec
.aggr_expr()
.iter()
- .map(|expr| match expr.field() {
- Ok(field) => Ok(field.name().clone()),
- Err(e) => Err(e),
- })
- .collect::<Result<_>>()?;
+ .map(|expr| expr.name().to_string())
+ .collect::<Vec<_>>();
let agg_mode = match exec.mode() {
AggregateMode::Partial => protobuf::AggregateMode::Partial,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
