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 BuiltInWindowFunction::{Lead, Lag} to a user defined window function #12857

Merged
merged 79 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
9efb417
Move `lead-lag` to `functions-window` package
jcsherin Oct 2, 2024
6f05a9c
Builds with warnings
jcsherin Oct 2, 2024
d2ebd3a
Adds `PartitionEvaluatorArgs`
jcsherin Oct 2, 2024
e5e5ab9
Extracts `shift_offset` from input expressions
jcsherin Oct 3, 2024
303b74b
Computes shift offset
jcsherin Oct 3, 2024
689ed3a
Get default value from input expression
jcsherin Oct 3, 2024
0774a58
Implements `partition_evaluator`
jcsherin Oct 3, 2024
009a1be
Fixes compiler warnings
jcsherin Oct 3, 2024
8dc161f
Comments out failing tests
jcsherin Oct 3, 2024
45e259e
Fixes `cargo test` errors and warnings
jcsherin Oct 3, 2024
0d2aa99
Minor: taplo formatting
jcsherin Oct 3, 2024
5ccb795
Delete code
jcsherin Oct 3, 2024
7d8b3e6
Define `lead`, `lag` user-defined window functions
jcsherin Oct 3, 2024
ba9d24a
Fixes `cargo build` errors
jcsherin Oct 3, 2024
402dcac
Export udwf and expression public APIs
jcsherin Oct 3, 2024
2ce0883
Mark result field as nullable
jcsherin Oct 3, 2024
95e8f87
Delete `return_type` tests for `lead` and `lag`
jcsherin Oct 3, 2024
04f30ec
Disables test: window function case insensitive
jcsherin Oct 3, 2024
d034082
Fixes: lowercase name in logical plan
jcsherin Oct 3, 2024
513df2a
Reverts to old methods for computing `shift_offset`, `default_value`
jcsherin Oct 3, 2024
bb0bd8b
Implements expression reversal
jcsherin Oct 3, 2024
0e576bb
Fixes: lowercase name in logical plans
jcsherin Oct 3, 2024
5490860
Fixes: doc test compilation errors
jcsherin Oct 3, 2024
f0c0e72
Temporarily quite clippy errors
jcsherin Oct 3, 2024
9051d2f
Fixes proto defintion
jcsherin Oct 3, 2024
b4cdc10
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 3, 2024
be74322
Minor: fixes formatting
jcsherin Oct 3, 2024
c9ac517
Fixes: doc tests
jcsherin Oct 4, 2024
e37752f
Uses macro for defining `lag_udwf()` and `leag_udwf()`
jcsherin Oct 4, 2024
6fb12e6
Fixes: window fuzz test cases
jcsherin Oct 7, 2024
ae26cb6
Copies doc comments verbatim from `BuiltInWindowFunction` enum
jcsherin Oct 7, 2024
51652d7
Deletes from window function case insensitive test
jcsherin Oct 7, 2024
723ca68
Deletes `BuiltInWindowFunction` expression APIs
jcsherin Oct 7, 2024
742c196
Delete from `create_built_in_window_expr`
jcsherin Oct 7, 2024
99093f5
Deletes proto serialization
jcsherin Oct 7, 2024
0925785
Delete from `BuiltInWindowFunction` enum
jcsherin Oct 7, 2024
6f05818
Deletes test for finding built-in window function
jcsherin Oct 7, 2024
0be8500
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 8, 2024
5854f77
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 10, 2024
e69463f
Fixes build errors + deletes redundant code
jcsherin Oct 10, 2024
d0baa94
Deletes more code
jcsherin Oct 10, 2024
9199943
Delete unnecessary structs
jcsherin Oct 10, 2024
000ceb7
Refactors shift offset computation
jcsherin Oct 10, 2024
ae0b91b
Passes range unit test
jcsherin Oct 10, 2024
a0973f9
Fixes: clippy::get-first error
jcsherin Oct 10, 2024
9ddf1c9
Rewrite unit tests for WindowUDF
jcsherin Oct 10, 2024
3a084ed
Fixes: unit test for lag with default value
jcsherin Oct 10, 2024
82abc5c
Consistent input expressions and data types in unit tests
jcsherin Oct 10, 2024
d1a35d7
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 10, 2024
e648033
Minor: fixes formatting
jcsherin Oct 10, 2024
ea811c6
Restore original helper method for unit tests
jcsherin Oct 11, 2024
3b6bc5d
Revert "Refactors shift offset computation"
jcsherin Oct 11, 2024
ab8a83c
Moves helper functions into `functions-window-common` package
jcsherin Oct 11, 2024
66c4f5e
Uses common helper functions in `{lead, lag}`
jcsherin Oct 11, 2024
c25ba86
Minor: formatting
jcsherin Oct 11, 2024
5f3df05
Revert "Moves helper functions into `functions-window-common` package"
jcsherin Oct 11, 2024
6b16550
Moves common functions to utils
jcsherin Oct 11, 2024
a65ef26
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 11, 2024
072d748
Minor: formatting fixes
jcsherin Oct 11, 2024
952dee0
Update lowercase names in explain output
jcsherin Oct 11, 2024
4ba4a62
Adds doc for `lead()` and `lag()` expression functions
jcsherin Oct 11, 2024
873736a
Add doc for `WindowShiftKind::shift_offset`
jcsherin Oct 11, 2024
3eb0985
Remove `arrow` dev dependency
jcsherin Oct 11, 2024
0f9ad76
Minor: formatting
jcsherin Oct 11, 2024
df7cff4
Update inner doc comment
jcsherin Oct 11, 2024
5550015
Serialize 1 or more window function arguments
jcsherin Oct 12, 2024
9e5916d
Adds logical plan roundtrip test cases
jcsherin Oct 12, 2024
c6e8235
Refactor: readability of unit tests
jcsherin Oct 12, 2024
d7ee287
Minor: rename variable bindings
jcsherin Oct 12, 2024
4fe2135
Minor: copy edit
jcsherin Oct 12, 2024
7dbb7e8
Revert "Remove `arrow` dev dependency"
jcsherin Oct 12, 2024
7892e05
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 14, 2024
7d00e3a
Merge remote-tracking branch 'apache/main' into convert-lead-lag-udwf
alamb Oct 16, 2024
47c2b9c
Merge branch 'main' into convert-lead-lag-udwf
jcsherin Oct 16, 2024
4385f21
Move null argument handling helper to utils
jcsherin Oct 16, 2024
270a203
Disable failing sqllogic tests for handling NULL input
jcsherin Oct 16, 2024
93569ed
Revert "Disable failing sqllogic tests for handling NULL input"
jcsherin Oct 17, 2024
814a9b7
Fixes: incorrect NULL handling in `lead`/`lag` window function
jcsherin Oct 17, 2024
c4b8840
Adds more tests cases
jcsherin Oct 17, 2024
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
1 change: 1 addition & 0 deletions datafusion-cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 5 additions & 8 deletions datafusion/core/tests/fuzz_cases/window_fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use datafusion_physical_expr::{PhysicalExpr, PhysicalSortExpr};
use test_utils::add_empty_batches;

use datafusion::functions_window::row_number::row_number_udwf;
use datafusion_functions_window::lead_lag::{lag_udwf, lead_udwf};
use datafusion_functions_window::rank::{dense_rank_udwf, rank_udwf};
use hashbrown::HashMap;
use rand::distributions::Alphanumeric;
Expand Down Expand Up @@ -197,7 +198,7 @@ async fn bounded_window_causal_non_causal() -> Result<()> {
// )
(
// Window function
WindowFunctionDefinition::BuiltInWindowFunction(BuiltInWindowFunction::Lag),
WindowFunctionDefinition::WindowUDF(lag_udwf()),
// its name
"LAG",
// no argument
Expand All @@ -211,7 +212,7 @@ async fn bounded_window_causal_non_causal() -> Result<()> {
// )
(
// Window function
WindowFunctionDefinition::BuiltInWindowFunction(BuiltInWindowFunction::Lead),
WindowFunctionDefinition::WindowUDF(lead_udwf()),
// its name
"LEAD",
// no argument
Expand Down Expand Up @@ -393,9 +394,7 @@ fn get_random_function(
window_fn_map.insert(
"lead",
(
WindowFunctionDefinition::BuiltInWindowFunction(
BuiltInWindowFunction::Lead,
),
WindowFunctionDefinition::WindowUDF(lead_udwf()),
vec![
arg.clone(),
lit(ScalarValue::Int64(Some(rng.gen_range(1..10)))),
Expand All @@ -406,9 +405,7 @@ fn get_random_function(
window_fn_map.insert(
"lag",
(
WindowFunctionDefinition::BuiltInWindowFunction(
BuiltInWindowFunction::Lag,
),
WindowFunctionDefinition::WindowUDF(lag_udwf()),
vec![
arg.clone(),
lit(ScalarValue::Int64(Some(rng.gen_range(1..10)))),
Expand Down
32 changes: 3 additions & 29 deletions datafusion/expr/src/built_in_window_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::str::FromStr;

use crate::type_coercion::functions::data_types;
use crate::utils;
use crate::{Signature, TypeSignature, Volatility};
use crate::{Signature, Volatility};
use datafusion_common::{plan_datafusion_err, plan_err, DataFusionError, Result};

use arrow::datatypes::DataType;
Expand All @@ -44,17 +44,7 @@ pub enum BuiltInWindowFunction {
CumeDist,
/// Integer ranging from 1 to the argument value, dividing the partition as equally as possible
Ntile,
/// Returns value evaluated at the row that is offset rows before the current row within the partition;
/// If there is no such row, instead return default (which must be of the same type as value).
/// Both offset and default are evaluated with respect to the current row.
/// If omitted, offset defaults to 1 and default to null
Lag,
/// Returns value evaluated at the row that is offset rows after the current row within the partition;
/// If there is no such row, instead return default (which must be of the same type as value).
/// Both offset and default are evaluated with respect to the current row.
/// If omitted, offset defaults to 1 and default to null
Lead,
/// Returns value evaluated at the row that is the first row of the window frame
/// returns value evaluated at the row that is the first row of the window frame
FirstValue,
/// Returns value evaluated at the row that is the last row of the window frame
LastValue,
Expand All @@ -68,8 +58,6 @@ impl BuiltInWindowFunction {
match self {
CumeDist => "CUME_DIST",
Ntile => "NTILE",
Lag => "LAG",
Lead => "LEAD",
FirstValue => "first_value",
LastValue => "last_value",
NthValue => "NTH_VALUE",
Expand All @@ -83,8 +71,6 @@ impl FromStr for BuiltInWindowFunction {
Ok(match name.to_uppercase().as_str() {
"CUME_DIST" => BuiltInWindowFunction::CumeDist,
"NTILE" => BuiltInWindowFunction::Ntile,
"LAG" => BuiltInWindowFunction::Lag,
"LEAD" => BuiltInWindowFunction::Lead,
"FIRST_VALUE" => BuiltInWindowFunction::FirstValue,
"LAST_VALUE" => BuiltInWindowFunction::LastValue,
"NTH_VALUE" => BuiltInWindowFunction::NthValue,
Expand Down Expand Up @@ -117,9 +103,7 @@ impl BuiltInWindowFunction {
match self {
BuiltInWindowFunction::Ntile => Ok(DataType::UInt64),
BuiltInWindowFunction::CumeDist => Ok(DataType::Float64),
BuiltInWindowFunction::Lag
| BuiltInWindowFunction::Lead
| BuiltInWindowFunction::FirstValue
BuiltInWindowFunction::FirstValue
| BuiltInWindowFunction::LastValue
| BuiltInWindowFunction::NthValue => Ok(input_expr_types[0].clone()),
}
Expand All @@ -130,16 +114,6 @@ impl BuiltInWindowFunction {
// Note: The physical expression must accept the type returned by this function or the execution panics.
match self {
BuiltInWindowFunction::CumeDist => Signature::any(0, Volatility::Immutable),
BuiltInWindowFunction::Lag | BuiltInWindowFunction::Lead => {
Signature::one_of(
vec![
TypeSignature::Any(1),
TypeSignature::Any(2),
TypeSignature::Any(3),
],
Volatility::Immutable,
)
}
BuiltInWindowFunction::FirstValue | BuiltInWindowFunction::LastValue => {
Signature::any(1, Volatility::Immutable)
}
Expand Down
38 changes: 0 additions & 38 deletions datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2560,30 +2560,6 @@ mod test {
Ok(())
}

#[test]
fn test_lead_return_type() -> Result<()> {
let fun = find_df_window_func("lead").unwrap();
let observed = fun.return_type(&[DataType::Utf8], &[true], "")?;
assert_eq!(DataType::Utf8, observed);

let observed = fun.return_type(&[DataType::Float64], &[true], "")?;
assert_eq!(DataType::Float64, observed);

Ok(())
}

#[test]
fn test_lag_return_type() -> Result<()> {
let fun = find_df_window_func("lag").unwrap();
let observed = fun.return_type(&[DataType::Utf8], &[true], "")?;
assert_eq!(DataType::Utf8, observed);

let observed = fun.return_type(&[DataType::Float64], &[true], "")?;
assert_eq!(DataType::Float64, observed);

Ok(())
}

#[test]
fn test_nth_value_return_type() -> Result<()> {
let fun = find_df_window_func("nth_value").unwrap();
Expand Down Expand Up @@ -2621,8 +2597,6 @@ mod test {
let names = vec![
"cume_dist",
"ntile",
"lag",
"lead",
"first_value",
"last_value",
"nth_value",
Expand Down Expand Up @@ -2660,18 +2634,6 @@ mod test {
built_in_window_function::BuiltInWindowFunction::LastValue
))
);
assert_eq!(
find_df_window_func("LAG"),
Some(WindowFunctionDefinition::BuiltInWindowFunction(
built_in_window_function::BuiltInWindowFunction::Lag
))
);
assert_eq!(
find_df_window_func("LEAD"),
Some(WindowFunctionDefinition::BuiltInWindowFunction(
built_in_window_function::BuiltInWindowFunction::Lead
))
);
assert_eq!(find_df_window_func("not_exist"), None)
}

Expand Down
23 changes: 23 additions & 0 deletions datafusion/expr/src/udwf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ use crate::{
Signature,
};
use datafusion_common::{not_impl_err, Result};
use datafusion_functions_window_common::expr::ExpressionArgs;
use datafusion_functions_window_common::field::WindowUDFFieldArgs;
use datafusion_functions_window_common::partition::PartitionEvaluatorArgs;
use datafusion_physical_expr_common::physical_expr::PhysicalExpr;

/// Logical representation of a user-defined window function (UDWF)
/// A UDWF is different from a UDF in that it is stateful across batches.
Expand Down Expand Up @@ -149,6 +151,12 @@ impl WindowUDF {
self.inner.simplify()
}

/// Expressions that are passed to the [`PartitionEvaluator`].
///
/// See [`WindowUDFImpl::expressions`] for more details.
pub fn expressions(&self, expr_args: ExpressionArgs) -> Vec<Arc<dyn PhysicalExpr>> {
self.inner.expressions(expr_args)
}
/// Return a `PartitionEvaluator` for evaluating this window function
pub fn partition_evaluator_factory(
&self,
Expand Down Expand Up @@ -302,6 +310,14 @@ pub trait WindowUDFImpl: Debug + Send + Sync {
/// types are accepted and the function's Volatility.
fn signature(&self) -> &Signature;

/// Returns the expressions that are passed to the [`PartitionEvaluator`].
fn expressions(&self, expr_args: ExpressionArgs) -> Vec<Arc<dyn PhysicalExpr>> {
expr_args
.input_exprs()
.first()
.map_or(vec![], |expr| vec![Arc::clone(expr)])
}

/// Invoke the function, returning the [`PartitionEvaluator`] instance
fn partition_evaluator(
&self,
Expand Down Expand Up @@ -480,6 +496,13 @@ impl WindowUDFImpl for AliasedWindowUDFImpl {
self.inner.signature()
}

fn expressions(&self, expr_args: ExpressionArgs) -> Vec<Arc<dyn PhysicalExpr>> {
expr_args
.input_exprs()
.first()
.map_or(vec![], |expr| vec![Arc::clone(expr)])
}

fn partition_evaluator(
&self,
partition_evaluator_args: PartitionEvaluatorArgs,
Expand Down
34 changes: 0 additions & 34 deletions datafusion/expr/src/window_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
// specific language governing permissions and limitations
// under the License.

use datafusion_common::ScalarValue;

use crate::{expr::WindowFunction, BuiltInWindowFunction, Expr, Literal};

/// Create an expression to represent the `cume_dist` window function
Expand All @@ -29,38 +27,6 @@ pub fn ntile(arg: Expr) -> Expr {
Expr::WindowFunction(WindowFunction::new(BuiltInWindowFunction::Ntile, vec![arg]))
}

/// Create an expression to represent the `lag` window function
pub fn lag(
arg: Expr,
shift_offset: Option<i64>,
default_value: Option<ScalarValue>,
) -> Expr {
let shift_offset_lit = shift_offset
.map(|v| v.lit())
.unwrap_or(ScalarValue::Null.lit());
let default_lit = default_value.unwrap_or(ScalarValue::Null).lit();
Expr::WindowFunction(WindowFunction::new(
BuiltInWindowFunction::Lag,
vec![arg, shift_offset_lit, default_lit],
))
}

/// Create an expression to represent the `lead` window function
pub fn lead(
arg: Expr,
shift_offset: Option<i64>,
default_value: Option<ScalarValue>,
) -> Expr {
let shift_offset_lit = shift_offset
.map(|v| v.lit())
.unwrap_or(ScalarValue::Null.lit());
let default_lit = default_value.unwrap_or(ScalarValue::Null).lit();
Expr::WindowFunction(WindowFunction::new(
BuiltInWindowFunction::Lead,
vec![arg, shift_offset_lit, default_lit],
))
}

/// Create an expression to represent the `nth_value` window function
pub fn nth_value(arg: Expr, n: i64) -> Expr {
Expr::WindowFunction(WindowFunction::new(
Expand Down
64 changes: 64 additions & 0 deletions datafusion/functions-window-common/src/expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

use datafusion_common::arrow::datatypes::DataType;
use datafusion_physical_expr_common::physical_expr::PhysicalExpr;
use std::sync::Arc;

/// Arguments passed to user-defined window function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

#[derive(Debug, Default)]
pub struct ExpressionArgs<'a> {
/// The expressions passed as arguments to the user-defined window
/// function.
input_exprs: &'a [Arc<dyn PhysicalExpr>],
/// The corresponding data types of expressions passed as arguments
/// to the user-defined window function.
input_types: &'a [DataType],
}

impl<'a> ExpressionArgs<'a> {
/// Create an instance of [`ExpressionArgs`].
///
/// # Arguments
///
/// * `input_exprs` - The expressions passed as arguments
/// to the user-defined window function.
/// * `input_types` - The data types corresponding to the
/// arguments to the user-defined window function.
///
pub fn new(
input_exprs: &'a [Arc<dyn PhysicalExpr>],
input_types: &'a [DataType],
) -> Self {
Self {
input_exprs,
input_types,
}
}

/// Returns the expressions passed as arguments to the user-defined
/// window function.
pub fn input_exprs(&self) -> &'a [Arc<dyn PhysicalExpr>] {
self.input_exprs
}

/// Returns the [`DataType`]s corresponding to the input expressions
/// to the user-defined window function.
pub fn input_types(&self) -> &'a [DataType] {
self.input_types
}
}
1 change: 1 addition & 0 deletions datafusion/functions-window-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@
//! Common user-defined window functionality for [DataFusion]
//!
//! [DataFusion]: <https://crates.io/crates/datafusion>
pub mod expr;
pub mod field;
pub mod partition;
1 change: 1 addition & 0 deletions datafusion/functions-window/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ path = "src/lib.rs"
datafusion-common = { workspace = true }
datafusion-expr = { workspace = true }
datafusion-functions-window-common = { workspace = true }
datafusion-physical-expr = { workspace = true }
datafusion-physical-expr-common = { workspace = true }
log = { workspace = true }
paste = "1.0.15"
Expand Down
Loading
Loading