walterddr commented on code in PR #9870:
URL: https://github.com/apache/pinot/pull/9870#discussion_r1035529816
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/ShuffleRewriteVisitor.java:
##########
@@ -189,4 +172,27 @@ private static boolean canSkipShuffle(Set<Integer>
partitionKeys, KeySelector<Ob
}
return false;
}
+
+ private static Set<Integer>
deriveNewPartitionKeysFromRexExpressions(List<RexExpression> rexExpressionList,
+ Set<Integer> oldPartitionKeys) {
+ Map<Integer, Integer> partitionKeyMap = new HashMap<>();
+ for (int i = 0; i < rexExpressionList.size(); i++) {
+ RexExpression rex = rexExpressionList.get(i);
+ if (rex instanceof RexExpression.InputRef) {
+ // put the old-index to new-index mapping
+ // TODO: it doesn't handle duplicate references. e.g. if the same old
partition key is referred twice. it will
+ // only keep the second one. (see JOIN handling on left/right as
another example)
+ partitionKeyMap.put(((RexExpression.InputRef) rex).getIndex(), i);
Review Comment:
technically this is not a "bug" b/c the partitionkey generated this rewrite
is in fact "CORRECT".
both the join situation and the project with multiple select are considered
as "equivalent" partition and affects the "canSkipShuffle" conditions. these
also include:
1. transformed version instead of just inputRef, such as ` 1 - partitionKey1`
2. mixed of transformed with all partitionKeys included and guaranteed to be
unique, such as `makeListUDF(partitionKey1, partitionKey2)`
3. ...
There can be many more, we can iteratively add support for these semantics
--
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]
