Jackie-Jiang commented on code in PR #14264:
URL: https://github.com/apache/pinot/pull/14264#discussion_r1819891616


##########
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:
   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.
   
   From high level, for `UNBOUNDED PROCEEDING, CURRENT ROW, UNBOUNDED 
FOLLOWING`, the behavior of `ROWS` and `RANGES` should be the same. So we could 
potentially split the handling into 3 cases:
   - Without bounded proceeding/following
   - ROWS with bounded proceeding/following
   - RANGE with bounded proceeding/following - not supported
   
   We can discuss and address this in a separate PR



-- 
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]

Reply via email to