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 2025/01/11 20:54:40 UTC
[PR] Support BROADCAST join strategy [pinot]
Jackie-Jiang opened a new pull request, #14797:
URL: https://github.com/apache/pinot/pull/14797
Add support for `BROADCAST` join strategy, where left table remains on the local server, and right table is broadcasted to the left table to perform the join.
E.g.
```
SELECT /*+ joinOptions(join_strategy = 'broadcast') */ a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1 WHERE a.col2 = 'foo' AND b.col2 = 'bar'
```
Related to #14518
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591791730
I think I had given some feedback on the naming thing long time ago when Multi Stage engine was being developed. I can't seem to find the thread now.
Ideally (this is how more feature complete engines typically do), there is a clear distinction between an EXCHANGE type and the implementation of physical operator.
In this case:
Exchange types can be
- BROADCAST
- HASH_PARTITION (like Spark shuffle)
- ROUND_ROBIN
- SEND_TO_SINGLE
Exchange itself is an implemented as a pair operator:
- Sender operator as the root operator in the sender stage (downstream)
- Receiver operator as the leaf operator in the receiver stage (upstream)
Regardless of the exchange, there is some processing done in the receiver stage after the exchange (between 2 stages). In this case, it will be JOIN
- Logical Operation - JOIN
- Exchange type - BROADCAST
- Phsyical operation (depending on planner / optimizer) - Hash, Sort-Merge, NLJ etc
With that being said, BROADCAST is NOT A JOIN Strategy. It is an exchange type (e.g an exchange between two Major Fragments in Presto / Trino).
So, we should try to build this clear distinction both in code and design.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915912246
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/InStageStatsTreeBuilder.java:
##########
@@ -138,11 +138,11 @@ public ObjectNode visitFilter(FilterNode node, Void context) {
@Override
public ObjectNode visitJoin(JoinNode node, Void context) {
- if (node.getJoinStrategy() == JoinNode.JoinStrategy.HASH) {
- return recursiveCase(node, MultiStageOperator.Type.HASH_JOIN);
- } else {
- assert node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP;
+ if (node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP) {
return recursiveCase(node, MultiStageOperator.Type.LOOKUP_JOIN);
+ } else {
+ // TODO: Consider renaming this operator type. It handles multiple join strategies.
Review Comment:
Sounds good. Removed this TODO, and we can revisit this when adding the next join operator
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2594005921
You are right @siddharthteotia. The distribution is a property of the exchange. I was discussing that with @bziobrowski yesterday, and he rightfully mentioned that exchange types also affect aggregates.
> I'm thinking changing the join_strategy hint to exchange_type hint (still under joinOptions). For this particular PR's purpose, I can add local_broadcast to represent left local, right broadcast. @gortiz @siddharthteotia wdyt?
+1 to that.
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1926093200
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java:
##########
@@ -52,22 +54,132 @@ public void onMatch(RelOptRuleCall call) {
RelNode left = PinotRuleUtils.unboxRel(join.getInput(0));
RelNode right = PinotRuleUtils.unboxRel(join.getInput(1));
JoinInfo joinInfo = join.analyzeCondition();
+ Map<String, String> joinHintOptions = PinotHintOptions.JoinHintOptions.getJoinHintOptions(join);
+ PinotHintOptions.DistributionType leftDistributionType;
+ PinotHintOptions.DistributionType rightDistributionType;
+ if (joinHintOptions != null) {
+ leftDistributionType = PinotHintOptions.JoinHintOptions.getLeftDistributionType(joinHintOptions);
+ rightDistributionType = PinotHintOptions.JoinHintOptions.getRightDistributionType(joinHintOptions);
+ } else {
+ leftDistributionType = null;
+ rightDistributionType = null;
+ }
RelNode newLeft;
RelNode newRight;
if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(join)) {
- // Lookup join - add local exchange on the left side
- newLeft = PinotLogicalExchange.create(left, RelDistributions.SINGLETON);
+ // Lookup join
Review Comment:
Refactored and extracted the common code.
I added a TODO for the validation for hash join because I haven't thought through which combination is invalid. User might decide to use a certain distribution based on their data layout, and I don't want to fail them prematurely.
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1926060053
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
@siddharthteotia We are following this, but the actual problem is that: left and right exchange should be associated with JOIN instead of table. One example would be: there is no guarantee that the JOIN is applied to 2 tables. It could also be applied to the result of a query, or another JOIN etc.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915363570
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/InStageStatsTreeBuilder.java:
##########
@@ -138,11 +138,11 @@ public ObjectNode visitFilter(FilterNode node, Void context) {
@Override
public ObjectNode visitJoin(JoinNode node, Void context) {
- if (node.getJoinStrategy() == JoinNode.JoinStrategy.HASH) {
- return recursiveCase(node, MultiStageOperator.Type.HASH_JOIN);
- } else {
- assert node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP;
+ if (node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP) {
return recursiveCase(node, MultiStageOperator.Type.LOOKUP_JOIN);
+ } else {
+ // TODO: Consider renaming this operator type. It handles multiple join strategies.
Review Comment:
I think we are using names here in a strange way. This is a hash operator because it implements the join using a hash map. The other is a lookup join operator because it implements it using lookup logic.
In parallel, we have join strategies. One of the strategies creates logical partitions at query time based on the values of the columns being joined. The way these partitions are decided is based on hash code, so it is called hash strategy. In the documentation I used [Query time partition join strategy](https://docs.pinot.apache.org/users/user-guide-query/multi-stage-query/join-strategies/query-time-partition-join-strategy) because I didn't want to focus too much on the fact that is being using hashes.
Imagine a scenario where we add sorted joins. The type of the join should be _sort_ and the strategy used for the distribution of its inputs may be hash.
TL;DR: I think we need to distinguish between join algorithm (lookup, hash, sorted, nested look) and distribution strategies (hash/partitioned, local, randon, broadcast, etc)
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591909132
I'm thinking changing the `join_strategy` hint to `exchange_type` hint (still under `joinOptions`). For this particular PR's purpose, I can add `local_broadcast` to represent left local, right broadcast. @gortiz @siddharthteotia wdyt?
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915363570
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/InStageStatsTreeBuilder.java:
##########
@@ -138,11 +138,11 @@ public ObjectNode visitFilter(FilterNode node, Void context) {
@Override
public ObjectNode visitJoin(JoinNode node, Void context) {
- if (node.getJoinStrategy() == JoinNode.JoinStrategy.HASH) {
- return recursiveCase(node, MultiStageOperator.Type.HASH_JOIN);
- } else {
- assert node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP;
+ if (node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP) {
return recursiveCase(node, MultiStageOperator.Type.LOOKUP_JOIN);
+ } else {
+ // TODO: Consider renaming this operator type. It handles multiple join strategies.
Review Comment:
I think we are using names here in a strange way. This is a hash operator because it implements the join using a hash map. The other is a lookup join operator because it implements it using lookup logic.
In parallel, we have join strategies. One of the strategies creates logical partitions at query time based on the values of the columns being joined. The way these partitions are decided is based on hash code, so it is called hash strategy. In the documentation I used [Query time partition join strategy](https://docs.pinot.apache.org/users/user-guide-query/multi-stage-query/join-strategies/query-time-partition-join-strategy) because I didn't want to focus too much on the fact that is being using hashes.
Imagine a scenario where we add sorted joins. The type of the join should be _sort_ and the strategy used for the distribution of its inputs may be hash.
TL;DR: I think we need to distinguish between join algorithm (lookup, hash, sorted, nested look) and distribution strategies (hash/partitioned, local, randon, broadcast, etc). The algorithm will probably change the operator class being used while the distribution strategy will change the exchange of the children of the joins
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915386396
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/InStageStatsTreeBuilder.java:
##########
@@ -138,11 +138,11 @@ public ObjectNode visitFilter(FilterNode node, Void context) {
@Override
public ObjectNode visitJoin(JoinNode node, Void context) {
- if (node.getJoinStrategy() == JoinNode.JoinStrategy.HASH) {
- return recursiveCase(node, MultiStageOperator.Type.HASH_JOIN);
- } else {
- assert node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP;
+ if (node.getJoinStrategy() == JoinNode.JoinStrategy.LOOKUP) {
return recursiveCase(node, MultiStageOperator.Type.LOOKUP_JOIN);
+ } else {
+ // TODO: Consider renaming this operator type. It handles multiple join strategies.
Review Comment:
I agree that we should separate the type of join and the type of distribution/shuffle. The latter is not unique to joins and could also be used in e.g. aggregations.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591632107
> I'll take a look, but I think we need to rethink the names of join strategies. A strategy should not be defined uniquely by what it does on one of the sides of the join. This case, for example, is clear: The broadcast strategy will be applied on the right-hand side, but what will happen with the left one? We already have a strategy where the right-hand side is broadcasted, but the left is randomly shuffled.
I feel `BROADCAST` usually means fixing one side, and broadcasting the other side. Several query engines only support this strategy. We don't have an explicit join strategy for randomly shuffling left side and broadcasting right side, so if we want to add one, we can think of a new name for this less commonly used one.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591798729
May be for this PR it is fine if we are trying to get something going for a large fact to small dimension table JOIN. My recommendation would be to start thinking about revamping / refactoring all of this. It would make future additions more flexible, decouple physical operator implementation from exchange types (which should always be the case) and implement exchange as operators as well which will make it easier to optimize a plan with the desired exchange type.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591867040
> we can combine different exchange type with join algorithm. Exchange type is also useful for other operations such as aggregate.
Exactly. It will be much more flexible to build an optimal plan with the decoupling where we can choose exchange type based on data characteristics and/or physical operator algorithm.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2585452297
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14797?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`996e7a5`)](https://app.codecov.io/gh/apache/pinot/commit/996e7a515f786ee6cb7f0adeeb63977c7969c226?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1570 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (996e7a5). Click for more details.
>
> <details><summary>HEAD has 48 uploads less than BASE</summary>
>
>| Flag | BASE (59551e4) | HEAD (996e7a5) |
>|------|------|------|
>|integration|7|2|
>|integration2|3|2|
>|temurin|12|2|
>|java-21|7|2|
>|skip-bytebuffers-true|3|1|
>|skip-bytebuffers-false|7|1|
>|unittests|5|0|
>|unittests1|2|0|
>|java-11|5|0|
>|unittests2|3|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #14797 +/- ##
=============================================
- Coverage 61.75% 0.00% -61.76%
=============================================
Files 2436 3 -2433
Lines 133233 6 -133227
Branches 20636 0 -20636
=============================================
- Hits 82274 0 -82274
+ Misses 44911 6 -44905
+ Partials 6048 0 -6048
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/14797/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/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.63%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.75%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-27.73%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-61.76%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/14797/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/14797/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/14797?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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
Re: [PR] Support exchange type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1919686882
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
Hmm, I'm not able to make it work because this is not really a `TABLE_SCAN` option, or an option applied to any specific `RelNode`. The left and right side of a `JOIN` could be any `RelNode`, and it could be another chained `JOIN`. Do you see a way to extract this hint?
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1924775050
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
Are we not using this?
https://calcite.apache.org/javadocAggregate/org/apache/calcite/rel/hint/package-summary.html
Calcite has good support for hint extraction and propagation.
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1924774471
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java:
##########
@@ -52,22 +54,132 @@ public void onMatch(RelOptRuleCall call) {
RelNode left = PinotRuleUtils.unboxRel(join.getInput(0));
RelNode right = PinotRuleUtils.unboxRel(join.getInput(1));
JoinInfo joinInfo = join.analyzeCondition();
+ Map<String, String> joinHintOptions = PinotHintOptions.JoinHintOptions.getJoinHintOptions(join);
+ PinotHintOptions.DistributionType leftDistributionType;
+ PinotHintOptions.DistributionType rightDistributionType;
+ if (joinHintOptions != null) {
+ leftDistributionType = PinotHintOptions.JoinHintOptions.getLeftDistributionType(joinHintOptions);
+ rightDistributionType = PinotHintOptions.JoinHintOptions.getRightDistributionType(joinHintOptions);
+ } else {
+ leftDistributionType = null;
+ rightDistributionType = null;
+ }
RelNode newLeft;
RelNode newRight;
if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(join)) {
- // Lookup join - add local exchange on the left side
- newLeft = PinotLogicalExchange.create(left, RelDistributions.SINGLETON);
+ // Lookup join
Review Comment:
(nit) this function is somewhat less readable. we should consider refactoring
we should also add validation. for example, a query can't have a exchange hint for left side as BROADCAST and right side as HASH for the same JOIN op.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915393382
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -121,13 +124,21 @@ private void assignWorkersToNonRootFragment(PlanFragment fragment, DispatchableP
}
}
- private boolean isLocalExchange(List<PlanFragment> children) {
- if (children.size() != 1) {
- return false;
+ /**
+ * Returns the index of the child fragment that has a local exchange (SINGLETON distribution), or {@code null} if none
+ * exists.
+ */
+ @Nullable
+ private Integer findLocalExchange(List<PlanFragment> children) {
+ int numChildren = children.size();
+ for (int i = 0; i < numChildren; i++) {
+ PlanNode childPlanNode = children.get(i).getFragmentRoot();
+ if (childPlanNode instanceof MailboxSendNode
+ && ((MailboxSendNode) childPlanNode).getDistributionType() == RelDistribution.Type.SINGLETON) {
+ return i;
+ }
}
- PlanNode childPlanNode = children.get(0).getFragmentRoot();
- return childPlanNode instanceof MailboxSendNode
- && ((MailboxSendNode) childPlanNode).getDistributionType() == RelDistribution.Type.SINGLETON;
Review Comment:
nit: why not return -1 and avoid allocating object?
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2590767062
We need to add documentation at https://docs.pinot.apache.org/users/user-guide-query/multi-stage-query/join-strategies. Feel free to use my diagrams at https://app.excalidraw.com/s/6rIIm06x9LN/amPNwZicV0. I don't know how to share excalidraw diagrams with edit permissions without giving the write permission to the whole internet.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2590678897
I'll take a look, but I think we need to rethink the names of join strategies. A strategy should not be defined uniquely by what it does on one of the sides of the join. This case, for example, is clear: The broadcast strategy will be applied on the right-hand side, but what will happen with the left one? We already have a strategy where the right-hand side is broadcasted, but the left is randomly shuffled.
--
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
Re: [PR] Support exchange type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1919472915
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
Good suggestion. Let me try it and see if I can make it work
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #14797:
URL: https://github.com/apache/pinot/pull/14797
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1927784369
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
It can be associated to the `TableScan` node, but the abstraction is not clear to me. We are deciding the exchange distribution for `Join`, but not `TableScan`, and also the child of `Join` might not be table scan
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2591853675
@siddharthteotia I like the idea of de-coupling exchange and join algorithm. Currently they are coupled under join strategy, where we support `hash`, `lookup`, `dynamic_broadcast` before this PR. We can start thinking how to organize the hint so that we can combine different exchange type with join algorithm. Exchange type is also useful for other operations such as aggregate.
We can address this as a separate effort. cc @ankitsultana
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2590681267
cc @bziobrowski @yashmayya @albertobastos
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2594015264
Sounds good.
> I'm thinking changing the join_strategy hint to exchange_type hint (still under joinOptions)
I am ok with this for now. It's fine to provide exchange type as hint as long as the exchange_type is not a property of solely for JoinOptions. It should be independent and JoinOptions or any query type should be able to leverage it especially if user know what they are doing and are trying to dictate the exchange-type via hints.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2593395149
@siddharthteotia I am right now working on refactoring the optimizer where many of the optimizations like coloration, skipping of partial aggregates, etc. will become automatic. Will add you to the Slack channel.
--
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
Re: [PR] Support exchange type hint to allow broadcast join [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #14797:
URL: https://github.com/apache/pinot/pull/14797#issuecomment-2594665174
Updated the PR to decouple exchange type from join strategy. Allow customizing exchange type for both left and right side.
--
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
Re: [PR] Support exchange type hint to allow broadcast join [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1919074014
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
Shouldn't be better to create a new hint for exchanges instead of assigning it to the join?
The way I'm suggesting the query would be something like:
```sql
SELECT
{tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val
FROM {tbl1} /*+ exchangeOption(type = 'local') */
JOIN {tbl2} /*+ exchangeOption(type = 'broadcast') */
ON {tbl1}.num = {tbl2}.num;
```
This could also be used for example in aggregates. For example, we could write something like:
```sql
SELECT
{tbl1}.num, count(*)
from {tbl1} /*+ exchangeOption(type = 'local') */
GROUP BY {tbl1}.num;
```
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915344916
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java:
##########
@@ -54,10 +54,15 @@ public void onMatch(RelOptRuleCall call) {
JoinInfo joinInfo = join.analyzeCondition();
RelNode newLeft;
RelNode newRight;
- if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(join)) {
+ String joinStrategyHint = PinotHintOptions.JoinHintOptions.getJoinStrategyHint(join);
+ if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(joinStrategyHint)) {
Review Comment:
nit: what about having a `PinotHintOptions.JoinHintOptions.getJoinStrategy` that returns an enum we can use in a switch here? We expect to add at least 3 new extra strategies (listed in https://github.com/apache/pinot/issues/14518, including random + broadcast right now cannot be specified), so the switch syntax may be helpful.
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915907959
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -121,13 +124,21 @@ private void assignWorkersToNonRootFragment(PlanFragment fragment, DispatchableP
}
}
- private boolean isLocalExchange(List<PlanFragment> children) {
- if (children.size() != 1) {
- return false;
+ /**
+ * Returns the index of the child fragment that has a local exchange (SINGLETON distribution), or {@code null} if none
+ * exists.
+ */
+ @Nullable
+ private Integer findLocalExchange(List<PlanFragment> children) {
+ int numChildren = children.size();
+ for (int i = 0; i < numChildren; i++) {
+ PlanNode childPlanNode = children.get(i).getFragmentRoot();
+ if (childPlanNode instanceof MailboxSendNode
+ && ((MailboxSendNode) childPlanNode).getDistributionType() == RelDistribution.Type.SINGLETON) {
+ return i;
+ }
}
- PlanNode childPlanNode = children.get(0).getFragmentRoot();
- return childPlanNode instanceof MailboxSendNode
- && ((MailboxSendNode) childPlanNode).getDistributionType() == RelDistribution.Type.SINGLETON;
Review Comment:
Sure
--
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
Re: [PR] Support BROADCAST join strategy [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1915926539
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java:
##########
@@ -54,10 +54,15 @@ public void onMatch(RelOptRuleCall call) {
JoinInfo joinInfo = join.analyzeCondition();
RelNode newLeft;
RelNode newRight;
- if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(join)) {
+ String joinStrategyHint = PinotHintOptions.JoinHintOptions.getJoinStrategyHint(join);
+ if (PinotHintOptions.JoinHintOptions.useLookupJoinStrategy(joinStrategyHint)) {
Review Comment:
Done
--
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
Re: [PR] Support distribution type hint to allow broadcast join [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14797:
URL: https://github.com/apache/pinot/pull/14797#discussion_r1926502164
##########
pinot-query-runtime/src/test/resources/queries/QueryHints.json:
##########
@@ -125,6 +125,14 @@
"description": "Colocated JOIN with partition column and group by non-partitioned column with stage parallelism",
"sql": "SET stageParallelism=2; SELECT {tbl1}.name, SUM({tbl2}.num) FROM {tbl1} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ JOIN {tbl2} /*+ tableOptions(partition_function='hashcode', partition_key='num', partition_size='4') */ ON {tbl1}.num = {tbl2}.num GROUP BY {tbl1}.name"
},
+ {
+ "description": "Broadcast JOIN without partition hint",
+ "sql": "SELECT /*+ joinOptions(left_exchange_type = 'local', right_exchange_type = 'broadcast') */ {tbl1}.num, {tbl1}.name, {tbl2}.num, {tbl2}.val FROM {tbl1} JOIN {tbl2} ON {tbl1}.num = {tbl2}.num"
Review Comment:
And something like:
```sql
select /*+ NO_HASH_JOIN, RESOURCE(mem='128mb', parallelism='24') */
from
(select * from emp limit 10) as emp /*+ INDEX(idx1, idx2) */
join
dept /*+ PROPERTIES(k1='v1', k2='v2') */
on emp.deptno=dept.deptno
```
doesn't work?
--
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