You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2023/09/20 02:42:57 UTC
[GitHub] [pinot] Jackie-Jiang opened a new pull request, #11630: Optimize group-by and join for single key scenario
Jackie-Jiang opened a new pull request, #11630:
URL: https://github.com/apache/pinot/pull/11630
- For single join key and group key scenario, no need to wrap the values into the `Key` object. Directly use the value as the key in the map.
- Optimize the case of no join key
- In group-by executor, store merged results in `ArrayList`, group key map in `Object2IntOpenHashMap`
- In join operator, store matched right rows in `BitSet`
- Use `RANDOM_DISTRIBUTED` when there is no distribution key for `HASH_DISTRIBUTED`
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1331215732
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/colocated/GreedyShuffleRewriteVisitor.java:
##########
@@ -190,23 +191,24 @@ public Set<ColocationKey> visitMailboxReceive(MailboxReceiveNode node, GreedyShu
// distribution.
((MailboxSendNode) node.getSender()).setDistributionType(RelDistribution.Type.SINGLETON);
return oldColocationKeys;
- } else if (selector == null) {
+ } else if (distributionKeys == null) {
return new HashSet<>();
}
// This means we can't skip shuffle and there's a partitioning enforced by receiver.
int numPartitions = new HashSet<>(
_dispatchablePlanMetadataMap.get(node.getPlanFragmentId()).getWorkerIdToServerInstanceMap().values()).size();
- List<ColocationKey> colocationKeys = ((FieldSelectionKeySelector) selector).getColumnIndices().stream()
- .map(x -> new ColocationKey(x, numPartitions, selector.hashAlgorithm())).collect(Collectors.toList());
+ List<ColocationKey> colocationKeys =
+ distributionKeys.stream().map(x -> new ColocationKey(x, numPartitions, KeySelector.DEFAULT_HASH_ALGORITHM))
Review Comment:
shall we make this algorithm configurable?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1332130606
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/JoinNode.java:
##########
@@ -94,24 +92,24 @@ public <T, C> T visit(PlanNodeVisitor<T, C> visitor, C context) {
public static class JoinKeys {
@ProtoProperties
- private KeySelector<Object[], Object[]> _leftJoinKeySelector;
+ private List<Integer> _leftKeys;
@ProtoProperties
- private KeySelector<Object[], Object[]> _rightJoinKeySelector;
+ private List<Integer> _rightKeys;
Review Comment:
superb refactoring. this was a legacy property we added for pluggability that we never used. we should remove these and use factory methods!
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1331949272
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/colocated/GreedyShuffleRewriteVisitor.java:
##########
@@ -190,23 +191,24 @@ public Set<ColocationKey> visitMailboxReceive(MailboxReceiveNode node, GreedyShu
// distribution.
((MailboxSendNode) node.getSender()).setDistributionType(RelDistribution.Type.SINGLETON);
return oldColocationKeys;
- } else if (selector == null) {
+ } else if (distributionKeys == null) {
return new HashSet<>();
}
// This means we can't skip shuffle and there's a partitioning enforced by receiver.
int numPartitions = new HashSet<>(
_dispatchablePlanMetadataMap.get(node.getPlanFragmentId()).getWorkerIdToServerInstanceMap().values()).size();
- List<ColocationKey> colocationKeys = ((FieldSelectionKeySelector) selector).getColumnIndices().stream()
- .map(x -> new ColocationKey(x, numPartitions, selector.hashAlgorithm())).collect(Collectors.toList());
+ List<ColocationKey> colocationKeys =
+ distributionKeys.stream().map(x -> new ColocationKey(x, numPartitions, KeySelector.DEFAULT_HASH_ALGORITHM))
Review Comment:
This optimizer is triggered by a hint, and we are not using it. I believe the long term plan is to remove it. cc @walterddr
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on pull request #11630: Optimize group-by and join for single key scenario
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #11630:
URL: https://github.com/apache/pinot/pull/11630#issuecomment-1727053483
Please fix the CI.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #11630: Optimize group-by and join for single key scenario
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11630:
URL: https://github.com/apache/pinot/pull/11630#issuecomment-1726824983
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11630](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f4b16c6) into [master](https://app.codecov.io/gh/apache/pinot/commit/47eff886cd0a130cbff9566175f7ec36ea7792e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (47eff88) will **decrease** coverage by `48.64%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11630 +/- ##
=============================================
- Coverage 63.08% 14.45% -48.64%
+ Complexity 1110 201 -909
=============================================
Files 2330 2334 +4
Lines 125145 125184 +39
Branches 19195 19207 +12
=============================================
- Hits 78952 18098 -60854
- Misses 40558 105559 +65001
+ Partials 5635 1527 -4108
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-17](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-20](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <0.00%> (-48.49%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <0.00%> (-48.64%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <0.00%> (-48.63%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <0.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files Changed](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../apache/pinot/core/util/DataBlockExtractUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0RhdGFCbG9ja0V4dHJhY3RVdGlscy5qYXZh) | `0.00% <0.00%> (-14.29%)` | :arrow_down: |
| [...he/pinot/query/planner/logical/PlanFragmenter.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1BsYW5GcmFnbWVudGVyLmphdmE=) | `0.00% <0.00%> (-89.37%)` | :arrow_down: |
| [.../query/planner/logical/RelToPlanNodeConverter.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JlbFRvUGxhbk5vZGVDb252ZXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-86.92%)` | :arrow_down: |
| [...t/query/planner/logical/ShuffleRewriteVisitor.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1NodWZmbGVSZXdyaXRlVmlzaXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...t/query/planner/partitioning/EmptyKeySelector.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wYXJ0aXRpb25pbmcvRW1wdHlLZXlTZWxlY3Rvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../pinot/query/planner/partitioning/KeySelector.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wYXJ0aXRpb25pbmcvS2V5U2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...query/planner/partitioning/KeySelectorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wYXJ0aXRpb25pbmcvS2V5U2VsZWN0b3JGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...y/planner/partitioning/MultiColumnKeySelector.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wYXJ0aXRpb25pbmcvTXVsdGlDb2x1bW5LZXlTZWxlY3Rvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../planner/partitioning/SingleColumnKeySelector.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9wYXJ0aXRpb25pbmcvU2luZ2xlQ29sdW1uS2V5U2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...hysical/colocated/GreedyShuffleRewriteVisitor.java](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9waHlzaWNhbC9jb2xvY2F0ZWQvR3JlZWR5U2h1ZmZsZVJld3JpdGVWaXNpdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [11 more](https://app.codecov.io/gh/apache/pinot/pull/11630?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [1491 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11630/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #11630: Optimize group-by and join for single key scenario
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11630:
URL: https://github.com/apache/pinot/pull/11630
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1332127248
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/colocated/GreedyShuffleRewriteVisitor.java:
##########
@@ -190,23 +191,24 @@ public Set<ColocationKey> visitMailboxReceive(MailboxReceiveNode node, GreedyShu
// distribution.
((MailboxSendNode) node.getSender()).setDistributionType(RelDistribution.Type.SINGLETON);
return oldColocationKeys;
- } else if (selector == null) {
+ } else if (distributionKeys == null) {
return new HashSet<>();
}
// This means we can't skip shuffle and there's a partitioning enforced by receiver.
int numPartitions = new HashSet<>(
_dispatchablePlanMetadataMap.get(node.getPlanFragmentId()).getWorkerIdToServerInstanceMap().values()).size();
- List<ColocationKey> colocationKeys = ((FieldSelectionKeySelector) selector).getColumnIndices().stream()
- .map(x -> new ColocationKey(x, numPartitions, selector.hashAlgorithm())).collect(Collectors.toList());
+ List<ColocationKey> colocationKeys =
+ distributionKeys.stream().map(x -> new ColocationKey(x, numPartitions, KeySelector.DEFAULT_HASH_ALGORITHM))
Review Comment:
i think for now we can assume that. the problem is we dont know how users will actually be using. having this configurable can be done in the future when needed.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1332128451
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/partitioning/KeySelectorFactory.java:
##########
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.query.planner.partitioning;
+
+import java.util.List;
+
+
+public class KeySelectorFactory {
+ private KeySelectorFactory() {
+ }
+
+ public static KeySelector<?> getKeySelector(List<Integer> keyIds) {
Review Comment:
@xiangfu0 we can add configurable algorithms by adding other factory methods IMO
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #11630: Optimize group-by and join for single key scenario
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11630:
URL: https://github.com/apache/pinot/pull/11630#discussion_r1332130606
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/JoinNode.java:
##########
@@ -94,24 +92,24 @@ public <T, C> T visit(PlanNodeVisitor<T, C> visitor, C context) {
public static class JoinKeys {
@ProtoProperties
- private KeySelector<Object[], Object[]> _leftJoinKeySelector;
+ private List<Integer> _leftKeys;
@ProtoProperties
- private KeySelector<Object[], Object[]> _rightJoinKeySelector;
+ private List<Integer> _rightKeys;
Review Comment:
superb refactoring. this was a legacy property we added for pluggability that we never used. we should've removed these and use factory methods even before the 1.0 release :-P
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org