ozankabak commented on code in PR #14271:
URL: https://github.com/apache/datafusion/pull/14271#discussion_r1933977485
##########
datafusion/physical-expr/src/window/standard.rs:
##########
@@ -65,33 +65,19 @@ impl StandardWindowExpr {
&self.expr
}
- /// Adds any equivalent orderings generated by the `self.expr`
- /// to `builder`.
+ /// Adds any equivalent orderings generated by the `self.expr` to
`builder`.
Review Comment:
```suggestion
/// Adds any equivalent orderings generated by `self.expr` to `builder`.
```
##########
datafusion/physical-expr/src/window/aggregate.rs:
##########
@@ -21,16 +21,17 @@ use std::any::Any;
use std::ops::Range;
use std::sync::Arc;
-use arrow::array::Array;
-use arrow::record_batch::RecordBatch;
-use arrow::{array::ArrayRef, datatypes::Field};
-
use crate::aggregate::AggregateFunctionExpr;
+use crate::window::standard::add_new_ordering_expr_with_partition_by;
use crate::window::window_expr::AggregateWindowExpr;
use crate::window::{
PartitionBatches, PartitionWindowAggStates, SlidingAggregateWindowExpr,
WindowExpr,
};
-use crate::{reverse_order_bys, PhysicalExpr};
+use crate::{reverse_order_bys, EquivalenceProperties, PhysicalExpr};
+
+use arrow::array::Array;
+use arrow::record_batch::RecordBatch;
+use arrow::{array::ArrayRef, datatypes::Field};
use datafusion_common::ScalarValue;
use datafusion_common::{DataFusionError, Result};
Review Comment:
```suggestion
use datafusion_common::{DataFusionError, ScalarValue, Result};
```
##########
datafusion/physical-expr/src/aggregate.rs:
##########
@@ -533,6 +536,29 @@ impl AggregateFunctionExpr {
pub fn default_value(&self, data_type: &DataType) -> Result<ScalarValue> {
self.fun.default_value(data_type)
}
+
+ /// Indicates whether the aggregation function is monotonic as a set
function. A set
+ /// function is monotonically increasing if its value increases as its
argument grows
+ /// (as a set). Formally, `f` is a monotonically increasing set function
if `f(S) >= f(T)`
+ /// whenever `S` is a superset of `T`.
+ pub fn set_monotonicity(&self) -> AggregateExprSetMonotonicity {
+ let field = self.field();
+ let data_type = field.data_type();
+ self.fun.inner().set_monotonicity(data_type)
+ }
+
+ /// Returns PhysicalSortExpr based on monotonicity of the function
Review Comment:
```suggestion
/// Returns `PhysicalSortExpr` based on the set monotonicity of the
function.
```
##########
datafusion/physical-expr/src/aggregate.rs:
##########
@@ -35,24 +35,27 @@ pub mod utils {
};
}
+use std::fmt::Debug;
+use std::sync::Arc;
+
+use crate::expressions::Column;
+
use arrow::datatypes::{DataType, Field, Schema, SchemaRef};
+use arrow_schema::SortOptions;
use datafusion_common::ScalarValue;
use datafusion_common::{internal_err, not_impl_err, Result};
Review Comment:
```suggestion
use datafusion_common::{internal_err, not_impl_err, ScalarValue, Result};
```
##########
datafusion/physical-expr/src/window/standard.rs:
##########
@@ -283,3 +269,27 @@ impl WindowExpr for StandardWindowExpr {
}
}
}
+
+/// Adds new ordering expression into the existing ordering equivalence class
based on partition by information.
Review Comment:
```suggestion
/// Adds a new ordering expression into existing ordering equivalence
class(es) based on
/// PARTITION BY information (if it exists).
```
##########
datafusion/physical-expr/src/aggregate.rs:
##########
@@ -35,24 +35,27 @@ pub mod utils {
};
}
+use std::fmt::Debug;
+use std::sync::Arc;
+
+use crate::expressions::Column;
+
use arrow::datatypes::{DataType, Field, Schema, SchemaRef};
+use arrow_schema::SortOptions;
use datafusion_common::ScalarValue;
use datafusion_common::{internal_err, not_impl_err, Result};
-use datafusion_expr::AggregateUDF;
use datafusion_expr::ReversedUDAF;
+use datafusion_expr::{AggregateExprSetMonotonicity, AggregateUDF};
Review Comment:
```suggestion
use datafusion_expr::{AggregateExprSetMonotonicity, AggregateUDF,
ReversedUDAF};
```
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -648,11 +649,32 @@ impl AggregateExec {
group_expr_mapping: &ProjectionMapping,
mode: &AggregateMode,
input_order_mode: &InputOrderMode,
+ aggr_exprs: Vec<Arc<AggregateFunctionExpr>>,
Review Comment:
A cursory look suggests this can be a slice. If that is true, let's avoid
copying.
--
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
