Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant result of AggregateFunctionExpr::field #12258

Merged
merged 1 commit into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions datafusion/physical-expr/src/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-expr/src/window/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl WindowExpr for PlainAggregateWindowExpr {
}

fn field(&self) -> Result<Field> {
self.aggregate.field()
Ok(self.aggregate.field())
}

fn name(&self) -> &str {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-expr/src/window/sliding_aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl WindowExpr for SlidingAggregateWindowExpr {
}

fn field(&self) -> Result<Field> {
self.aggregate.field()
Ok(self.aggregate.field())
}

fn name(&self) -> &str {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-optimizer/src/aggregate_statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down Expand Up @@ -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()));
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-plan/src/aggregates/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions datafusion/proto/src/physical_plan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down