jcsherin commented on code in PR #13201:
URL: https://github.com/apache/datafusion/pull/13201#discussion_r1829235397
##########
datafusion/functions-window/src/nth_value.rs:
##########
@@ -15,143 +15,254 @@
// specific language governing permissions and limitations
// under the License.
-//! Defines physical expressions for `FIRST_VALUE`, `LAST_VALUE`, and
`NTH_VALUE`
-//! functions that can be evaluated at run time during query execution.
+//! `nth_value` window function implementation
+
+use crate::utils::{get_scalar_value_from_args, get_signed_integer};
use std::any::Any;
use std::cmp::Ordering;
+use std::fmt::Debug;
use std::ops::Range;
-use std::sync::Arc;
-
-use crate::window::window_expr::{NthValueKind, NthValueState};
-use crate::window::BuiltInWindowFunctionExpr;
-use crate::PhysicalExpr;
+use std::sync::OnceLock;
-use arrow::array::{Array, ArrayRef};
-use arrow::datatypes::{DataType, Field};
-use datafusion_common::Result;
-use datafusion_common::ScalarValue;
+use datafusion_common::arrow::array::ArrayRef;
+use datafusion_common::arrow::datatypes::{DataType, Field};
+use datafusion_common::{exec_err, Result, ScalarValue};
+use datafusion_expr::window_doc_sections::DOC_SECTION_ANALYTICAL;
use datafusion_expr::window_state::WindowAggState;
-use datafusion_expr::PartitionEvaluator;
+use datafusion_expr::{
+ Documentation, PartitionEvaluator, ReversedUDWF, Signature, TypeSignature,
+ Volatility, WindowUDFImpl,
+};
+use datafusion_functions_window_common::field;
+use datafusion_functions_window_common::partition::PartitionEvaluatorArgs;
+use field::WindowUDFFieldArgs;
+
+define_udwf_and_expr!(
+ First,
+ first_value,
+ "returns the first value in the window frame",
+ NthValue::first
+);
+define_udwf_and_expr!(
+ Last,
+ last_value,
+ "returns the last value in the window frame",
+ NthValue::last
+);
+define_udwf_and_expr!(
+ NthValue,
+ nth_value,
+ "returns the nth value in the window frame",
+ NthValue::nth
+);
Review Comment:
The macro generated expression API does not match what we need.
This is the removed expression API for `nth_value`. Also there doesn't seem
to be separate APIs for `first_value` and `last_value`. So you do not need to
create expression APIs for them.
```
/// Create an expression to represent the `nth_value` window function
pub fn nth_value(arg: Expr, n: i64) -> Expr {
Expr::WindowFunction(WindowFunction::new(
BuiltInWindowFunction::NthValue,
vec![arg, n.lit()],
))
}
```
This is the macro generated expression API:
```rust
#[doc = " Create a [`WindowFunction`](datafusion_expr::Expr::WindowFunction)
expression for"]
#[doc = " `NthValue` user-defined window function."]
#[doc = ""]
#[doc = " returns the nth value in the window frame"]
pub fn nth_value() -> datafusion_expr::Expr {
nth_value_udwf().call(vec![])
}
```
The `define_udwf_and_expr` does not support generating expression functions
which accepts arguments to code complexity low.
You can take a look at the code in `lead`/`lag` to see how to apply it here
as well:
https://github.com/apache/datafusion/blob/9e636d8fe920340409e527da36cff3d2c25aef5f/datafusion/functions-window/src/lead_lag.rs#L40-L93
--
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
