jcsherin commented on code in PR #13201:
URL: https://github.com/apache/datafusion/pull/13201#discussion_r1836112390
##########
datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:
##########
@@ -1500,129 +1494,6 @@ mod tests {
Ok(source)
}
- // Tests NTH_VALUE(negative index) with memoize feature.
- // To be able to trigger memoize feature for NTH_VALUE we need to
- // - feed BoundedWindowAggExec with batch stream data.
- // - Window frame should contain UNBOUNDED PRECEDING.
- // It hard to ensure these conditions are met, from the sql query.
- #[tokio::test]
- async fn test_window_nth_value_bounded_memoize() -> Result<()> {
Review Comment:
~~I reused `create_udwf_window_expr` by changing the visibility to current
module to build the physical expressions required for this test.~~
~~You may apply this patch as-is and the test will pass.~~
In
[5c4e0f6](https://github.com/apache/datafusion/pull/13201/commits/5c4e0f6ec67945b0e6f70662b5fd75513f77af65)
the visibility of `create_udwf_window_expr` is changed to `pub` so that it can
be reused in physical plan roundtrip test.
Updated patch:
```patch
diff --git a/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs
b/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs
index 0a898bd85..a031253e4 100644
--- a/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs
+++ b/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs
@@ -1163,7 +1163,9 @@ mod tests {
use crate::expressions::PhysicalSortExpr;
use crate::projection::ProjectionExec;
use crate::streaming::{PartitionStream, StreamingTableExec};
- use crate::windows::{create_window_expr, BoundedWindowAggExec,
InputOrderMode};
+ use crate::windows::{
+ create_udwf_window_expr, create_window_expr, BoundedWindowAggExec,
InputOrderMode,
+ };
use crate::{execute_stream, get_plan_string, ExecutionPlan};
use arrow_array::builder::{Int64Builder, UInt64Builder};
@@ -1180,13 +1182,14 @@ mod tests {
WindowFrame, WindowFrameBound, WindowFrameUnits,
WindowFunctionDefinition,
};
use datafusion_functions_aggregate::count::count_udaf;
- use datafusion_functions_window::nth_value::first_value_udwf;
use datafusion_functions_window::nth_value::last_value_udwf;
use datafusion_functions_window::nth_value::nth_value_udwf;
- use datafusion_physical_expr::expressions::{col, lit, Column};
+ use datafusion_physical_expr::expressions::{col, Column, Literal};
use datafusion_physical_expr::{LexOrdering, PhysicalExpr};
use crate::memory::MemoryExec;
+ use crate::common::collect;
+ use datafusion_physical_expr::window::BuiltInWindowExpr;
use futures::future::Shared;
use futures::{pin_mut, ready, FutureExt, Stream, StreamExt};
use itertools::Itertools;
@@ -1524,36 +1527,35 @@ mod tests {
)
.map(|e| Arc::new(e) as Arc<dyn ExecutionPlan>)?;
let col_a = col("a", &schema)?;
- let nth_value_func1 = NthValue::nth(
- "nth_value(-1)",
- Arc::clone(&col_a),
- DataType::Int32,
- 1,
+ let nth_value_func1 = create_udwf_window_expr(
+ &nth_value_udwf(),
+ &[Arc::clone(&col_a),
Arc::new(Literal::new(ScalarValue::Int32(Some(1))))],
+ &schema,
+ "nth_value(-1)".to_string(),
false,
)?
.reverse_expr()
.unwrap();
- let nth_value_func2 = NthValue::nth(
- "nth_value(-2)",
- Arc::clone(&col_a),
- DataType::Int32,
- 2,
+ let nth_value_func2 = create_udwf_window_expr(
+ &nth_value_udwf(),
+ &[Arc::clone(&col_a),
Arc::new(Literal::new(ScalarValue::Int32(Some(2))))],
+ &schema,
+ "nth_value(-2)".to_string(),
false,
- )?
- .reverse_expr()
- .unwrap();
- let last_value_func = Arc::new(NthValue::last(
- "last",
- Arc::clone(&col_a),
- DataType::Int32,
+ )?.reverse_expr().unwrap();
+ let last_value_func = create_udwf_window_expr(
+ &last_value_udwf(),
+ &[Arc::clone(&col_a)],
+ &schema,
+ "last".to_string(),
false,
- )) as _;
+ )?;
let window_exprs = vec![
// LAST_VALUE(a)
Arc::new(BuiltInWindowExpr::new(
last_value_func,
&[],
- &LexOrdering::default(),
+ &LexOrdering::default(),
Arc::new(WindowFrame::new_bounds(
WindowFrameUnits::Rows,
WindowFrameBound::Preceding(ScalarValue::UInt64(None)),
```
--
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
