yashmayya commented on code in PR #14264:
URL: https://github.com/apache/pinot/pull/14264#discussion_r1820124388
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/window/value/FirstValueWindowFunction.java:
##########
@@ -78,12 +86,73 @@ private List<Object> processRowsWindow(List<Object[]> rows)
{
lowerBound++;
upperBound = Math.min(upperBound + 1, numRows - 1);
}
+
+ return result;
+ }
+
+ private List<Object> processRowsWindowIgnoreNulls(List<Object[]> rows) {
Review Comment:
> From high level, for UNBOUNDED PROCEEDING, CURRENT ROW, UNBOUNDED
FOLLOWING, the behavior of ROWS and RANGES should be the same
The behavior will only be the same for `UNBOUNDED PRECEDING TO UNBOUNDED
FOLLOWING` right? With `CURRENT ROW` as either lower or upper bound or both,
the behavior and logic are both significantly different due to the need to
consider peer groups (rows before and after with the same order key) along with
current row with `RANGE` type window frames.
> We can replace the for loop to nCopy, and also short-circuit all if
checks. The total cost of the query is just finding the first/last non-null
value, so the save could potentially be relatively significant.
This makes sense for true unbounded case though (`UNBOUNDED PRECEDING TO
UNBOUNDED FOLLOWING`, `CURRENT ROW` is not really unbounded), I've raised a
small follow-up PR - https://github.com/apache/pinot/pull/14324.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
