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

Convert ApproxPercentileCont and ApproxPercentileContWithWeight to UDAF #10917

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ pub(crate) mod tests {
&[self.column()],
&[],
&[],
&[],
schema,
self.column_name(),
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ mod tests {
&[expr],
&[],
&[],
&[],
schema,
name,
false,
Expand Down Expand Up @@ -404,6 +405,7 @@ mod tests {
&[col("b", &schema)?],
&[],
&[],
&[],
&schema,
"Sum(b)",
false,
Expand Down
1 change: 1 addition & 0 deletions datafusion/core/src/physical_optimizer/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub fn bounded_window_exec(
"count".to_owned(),
&[col(col_name, &schema).unwrap()],
&[],
&[],
&sort_exprs,
Arc::new(WindowFrame::new(Some(false))),
schema.as_ref(),
Expand Down
16 changes: 9 additions & 7 deletions datafusion/core/src/physical_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,8 @@ pub fn create_window_expr_with_name(
window_frame,
null_treatment,
}) => {
let args = create_physical_exprs(args, logical_schema, execution_props)?;
let physical_args =
create_physical_exprs(args, logical_schema, execution_props)?;
let partition_by =
create_physical_exprs(partition_by, logical_schema, execution_props)?;
let order_by =
Expand All @@ -1780,13 +1781,13 @@ pub fn create_window_expr_with_name(
}

let window_frame = Arc::new(window_frame.clone());
let ignore_nulls = null_treatment
.unwrap_or(sqlparser::ast::NullTreatment::RespectNulls)
let ignore_nulls = null_treatment.unwrap_or(NullTreatment::RespectNulls)
== NullTreatment::IgnoreNulls;
windows::create_window_expr(
fun,
name,
&args,
&physical_args,
args,
&partition_by,
&order_by,
window_frame,
Expand Down Expand Up @@ -1837,7 +1838,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
order_by,
null_treatment,
}) => {
let args =
let physical_args =
create_physical_exprs(args, logical_input_schema, execution_props)?;
let filter = match filter {
Some(e) => Some(create_physical_expr(
Expand Down Expand Up @@ -1867,7 +1868,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
let agg_expr = aggregates::create_aggregate_expr(
fun,
*distinct,
&args,
&physical_args,
&ordering_reqs,
physical_input_schema,
name,
Expand All @@ -1889,7 +1890,8 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter(
physical_sort_exprs.clone().unwrap_or(vec![]);
let agg_expr = udaf::create_aggregate_expr(
fun,
&args,
&physical_args,
args,
&sort_exprs,
&ordering_reqs,
physical_input_schema,
Expand Down
6 changes: 3 additions & 3 deletions datafusion/core/tests/dataframe/dataframe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use datafusion::assert_batches_eq;
use datafusion_common::{DFSchema, ScalarValue};
use datafusion_expr::expr::Alias;
use datafusion_expr::ExprSchemable;
use datafusion_functions_aggregate::expr_fn::approx_median;
use datafusion_functions_aggregate::expr_fn::{approx_median, approx_percentile_cont};

fn test_schema() -> SchemaRef {
Arc::new(Schema::new(vec![
Expand Down Expand Up @@ -363,7 +363,7 @@ async fn test_fn_approx_percentile_cont() -> Result<()> {

let expected = [
"+---------------------------------------------+",
"| APPROX_PERCENTILE_CONT(test.b,Float64(0.5)) |",
"| approx_percentile_cont(test.b,Float64(0.5)) |",
"+---------------------------------------------+",
"| 10 |",
"+---------------------------------------------+",
Expand All @@ -384,7 +384,7 @@ async fn test_fn_approx_percentile_cont() -> Result<()> {
let df = create_test_table().await?;
let expected = [
"+--------------------------------------+",
"| APPROX_PERCENTILE_CONT(test.b,arg_2) |",
"| approx_percentile_cont(test.b,arg_2) |",
"+--------------------------------------+",
"| 10 |",
"+--------------------------------------+",
Expand Down
1 change: 1 addition & 0 deletions datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ async fn run_aggregate_test(input1: Vec<RecordBatch>, group_by_columns: Vec<&str
&[col("d", &schema).unwrap()],
&[],
&[],
&[],
&schema,
"sum1",
false,
Expand Down
4 changes: 4 additions & 0 deletions datafusion/core/tests/fuzz_cases/window_fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ async fn bounded_window_causal_non_causal() -> Result<()> {

let partitionby_exprs = vec![];
let orderby_exprs = vec![];
let logical_exprs = vec![];
// Window frame starts with "UNBOUNDED PRECEDING":
let start_bound = WindowFrameBound::Preceding(ScalarValue::UInt64(None));

Expand Down Expand Up @@ -283,6 +284,7 @@ async fn bounded_window_causal_non_causal() -> Result<()> {
&window_fn,
fn_name.to_string(),
&args,
&logical_exprs,
&partitionby_exprs,
&orderby_exprs,
Arc::new(window_frame),
Expand Down Expand Up @@ -699,6 +701,7 @@ async fn run_window_test(
&window_fn,
fn_name.clone(),
&args,
&[],
&partitionby_exprs,
&orderby_exprs,
Arc::new(window_frame.clone()),
Expand All @@ -717,6 +720,7 @@ async fn run_window_test(
&window_fn,
fn_name,
&args,
&[],
&partitionby_exprs,
&orderby_exprs,
Arc::new(window_frame.clone()),
Expand Down
50 changes: 1 addition & 49 deletions datafusion/expr/src/aggregate_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::sync::Arc;
use std::{fmt, str::FromStr};

use crate::utils;
use crate::{type_coercion::aggregates::*, Signature, TypeSignature, Volatility};
use crate::{type_coercion::aggregates::*, Signature, Volatility};

use arrow::datatypes::{DataType, Field};
use datafusion_common::{plan_datafusion_err, plan_err, DataFusionError, Result};
Expand All @@ -45,10 +45,6 @@ pub enum AggregateFunction {
NthValue,
/// Correlation
Correlation,
/// Approximate continuous percentile function
ApproxPercentileCont,
/// Approximate continuous percentile function with weight
ApproxPercentileContWithWeight,
/// Grouping
Grouping,
/// Bit And
Expand All @@ -75,8 +71,6 @@ impl AggregateFunction {
ArrayAgg => "ARRAY_AGG",
NthValue => "NTH_VALUE",
Correlation => "CORR",
ApproxPercentileCont => "APPROX_PERCENTILE_CONT",
ApproxPercentileContWithWeight => "APPROX_PERCENTILE_CONT_WITH_WEIGHT",
Grouping => "GROUPING",
BitAnd => "BIT_AND",
BitOr => "BIT_OR",
Expand Down Expand Up @@ -113,11 +107,6 @@ impl FromStr for AggregateFunction {
"string_agg" => AggregateFunction::StringAgg,
// statistical
"corr" => AggregateFunction::Correlation,
// approximate
"approx_percentile_cont" => AggregateFunction::ApproxPercentileCont,
"approx_percentile_cont_with_weight" => {
AggregateFunction::ApproxPercentileContWithWeight
}
// other
"grouping" => AggregateFunction::Grouping,
_ => {
Expand Down Expand Up @@ -170,10 +159,6 @@ impl AggregateFunction {
coerced_data_types[0].clone(),
true,
)))),
AggregateFunction::ApproxPercentileCont => Ok(coerced_data_types[0].clone()),
AggregateFunction::ApproxPercentileContWithWeight => {
Ok(coerced_data_types[0].clone())
}
AggregateFunction::Grouping => Ok(DataType::Int32),
AggregateFunction::NthValue => Ok(coerced_data_types[0].clone()),
AggregateFunction::StringAgg => Ok(DataType::LargeUtf8),
Expand Down Expand Up @@ -230,39 +215,6 @@ impl AggregateFunction {
AggregateFunction::Correlation => {
Signature::uniform(2, NUMERICS.to_vec(), Volatility::Immutable)
}
AggregateFunction::ApproxPercentileCont => {
let mut variants =
Vec::with_capacity(NUMERICS.len() * (INTEGERS.len() + 1));
// Accept any numeric value paired with a float64 percentile
for num in NUMERICS {
variants
.push(TypeSignature::Exact(vec![num.clone(), DataType::Float64]));
// Additionally accept an integer number of centroids for T-Digest
for int in INTEGERS {
variants.push(TypeSignature::Exact(vec![
num.clone(),
DataType::Float64,
int.clone(),
]))
}
}

Signature::one_of(variants, Volatility::Immutable)
}
AggregateFunction::ApproxPercentileContWithWeight => Signature::one_of(
// Accept any numeric value paired with a float64 percentile
NUMERICS
.iter()
.map(|t| {
TypeSignature::Exact(vec![
t.clone(),
t.clone(),
DataType::Float64,
])
})
.collect(),
Volatility::Immutable,
),
AggregateFunction::StringAgg => {
Signature::uniform(2, STRINGS.to_vec(), Volatility::Immutable)
}
Expand Down
28 changes: 0 additions & 28 deletions datafusion/expr/src/expr_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,34 +242,6 @@ pub fn in_list(expr: Expr, list: Vec<Expr>, negated: bool) -> Expr {
Expr::InList(InList::new(Box::new(expr), list, negated))
}

/// Calculate an approximation of the specified `percentile` for `expr`.
pub fn approx_percentile_cont(expr: Expr, percentile: Expr) -> Expr {
Expr::AggregateFunction(AggregateFunction::new(
aggregate_function::AggregateFunction::ApproxPercentileCont,
vec![expr, percentile],
false,
None,
None,
None,
))
}

/// Calculate an approximation of the specified `percentile` for `expr` and `weight_expr`.
pub fn approx_percentile_cont_with_weight(
expr: Expr,
weight_expr: Expr,
percentile: Expr,
) -> Expr {
Expr::AggregateFunction(AggregateFunction::new(
aggregate_function::AggregateFunction::ApproxPercentileContWithWeight,
vec![expr, weight_expr, percentile],
false,
None,
None,
None,
))
}

/// Create an EXISTS subquery expression
pub fn exists(subquery: Arc<LogicalPlan>) -> Expr {
let outer_ref_columns = subquery.all_out_ref_exprs();
Expand Down
4 changes: 2 additions & 2 deletions datafusion/expr/src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ pub struct AccumulatorArgs<'a> {
/// The input type of the aggregate function.
pub input_type: &'a DataType,

/// The number of arguments the aggregate function takes.
pub args_num: usize,
/// The logical expression of arguments the aggregate function takes.
pub input_exprs: &'a [Expr],
}

/// [`StateFieldsArgs`] contains information about the fields that an
Expand Down
Loading