somandal commented on code in PR #10286:
URL: https://github.com/apache/pinot/pull/10286#discussion_r1117478600


##########
pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotWindowExchangeNodeInsertRule.java:
##########
@@ -160,6 +162,19 @@ private boolean isPartitionByOnlyQuery(Window.Group 
windowGroup) {
       Set<Integer> partitionByKeyList = new 
HashSet<>(windowGroup.keys.toList());
       Set<Integer> orderByKeyList = new 
HashSet<>(windowGroup.orderKeys.getKeys());
       isPartitionByOnly = partitionByKeyList.equals(orderByKeyList);
+      if (isPartitionByOnly) {

Review Comment:
   yup, i tried these in postgres, but since postgres does an explicit sort for 
even the PARTITION BY keys it did make a difference in the ordering.
   
   Since we've decided not to care about ordering on PARTITION BY key, the 
above 3 queries will have the same aggregation value. The NULLS LAST vs. NULLS 
FIRST will only modify the order of the NULL rows, but this can be handled 
within the operator and still treat this type of query as a PARTITION BY only 
query.
   
   I guess based on my own response above, adding this check probably doesn't 
make sense 😅 



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

Reply via email to