comphead commented on code in PR #9221:
URL: https://github.com/apache/arrow-datafusion/pull/9221#discussion_r1494913964
##########
datafusion/physical-expr/src/window/lead_lag.rs:
##########
@@ -204,17 +215,34 @@ impl PartitionEvaluator for WindowShiftEvaluator {
values: &[ArrayRef],
range: &Range<usize>,
) -> Result<ScalarValue> {
+ // TODO: try to get rid of i64 usize conversion
+ // TODO: do not recalc default value every call
+ // TODO: lead mode
let array = &values[0];
let dtype = array.data_type();
+ let len = array.len() as i64;
// LAG mode
- let idx = if self.shift_offset > 0 {
+ let mut idx = if self.is_causal() {
range.end as i64 - self.shift_offset - 1
} else {
// LEAD mode
range.start as i64 - self.shift_offset
};
- if idx < 0 || idx as usize >= array.len() {
+ // Support LAG only for now, as LEAD requires some refactoring first
Review Comment:
for LEAD function we likely need to refactor the evaluator and how it works.
The problem is for LEAD we have to adjust values that have already been
emitted by evaluator which is not doable afaik. @mustafasrepo I would love to
get your input how we can solve such challenge. One of solution is to emit not
the single value like now, but the entire array so it gives more control
--
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]
