somandal commented on code in PR #10286:
URL: https://github.com/apache/pinot/pull/10286#discussion_r1107404202
##########
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:
I'm not entirely sure if this is actually required, but thought I'd look
into the correct semantics once I start the work on `ORDER BY` within `OVER()`.
This check will prevent marking queries like below as partition by only queries:
- OVER(PARTITION BY key1 ORDER BY key1 DESC)
- OVER(PARTITION BY key1 ORDER BY key1 NULLS FIRST)
The code will however allow queries of the type `OVER(PARTITION BY key1
ORDER BY key1)` to be marked as partition by only (since all aggregated values
will be the same across the whole partition just like OVER(PARTITION BY)).
Let me know if the preference is to just remove this extra check since
logically the actual aggregation values will still be the same for the full
partition irrespective of the direction and null direction.
--
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
