comphead commented on code in PR #26664:
URL: https://github.com/apache/datafusion/pull/26664#discussion_r4366002680
##########
datafusion/functions-window/src/nth_value.rs:
##########
@@ -464,18 +475,55 @@ impl PartitionEvaluator for NthValueEvaluator {
} else {
// FIRST_VALUE, LAST_VALUE, NTH_VALUE window functions take a
single column, values will have size 1.
let arr = &values[0];
- let n_range = range.end - range.start;
- if n_range == 0 {
- // We produce None if the window is empty.
- return ScalarValue::try_from(arr.data_type());
- }
- match self.valid_index(arr, range) {
- Some(index) => ScalarValue::try_from_array(arr, index),
- None => ScalarValue::try_from(arr.data_type()),
+ match (self.valid_index(arr, range), &self.last_valid) {
+ (Some(index), _) => ScalarValue::try_from_array(arr, index),
+ (None, Some(last_valid)) => Ok(last_valid.clone()),
Review Comment:
Thanks, added as `window_ignore_nulls_matrix.slt`, which sweeps `batch_size`
over 1, 2, 100 and 8192. At `batch_size = 2` each call evaluates two frames, so
the query reaches the `take`/`zip` path in `evaluate_ranges` but not this
scalar arm. `batch_size = 1` covers the arm. Each path fails its run if empty
frames return NULL.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
