You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "dang-stripe (via GitHub)" <gi...@apache.org> on 2026/04/10 04:22:27 UTC
[PR] [multistage] add lookup join support to physical optimizer [pinot]
dang-stripe opened a new pull request, #18158:
URL: https://github.com/apache/pinot/pull/18158
Addresses: https://github.com/apache/pinot/issues/17961
## Problem
Using lookup joins in the physical optimizer via query hint `joinOptions(join_strategy='lookup')` would fail with `"Right input must be leaf operator"`. The optimizer inserted a `BROADCAST_EXCHANGE` on the dimension table side, splitting it into a separate fragment, but `LookupJoinOperator` needs the dimension table as a `LeafOperator` in the same fragment because it expects to have access to a `DimensionTableDataManager`.
## Solution
This adds lookup join support to the V2 physical optimizer by:
* Adds a `LOOKUP_LOCAL_EXCHANGE` which acts as a pseudo-exchange so the fragment is classified as a leaf stage and the dim table is visible in EXPLAIN plans
* Adding a `LookupJoinRule` to isolate every lookup join in its own plan fragment with exchanges on all sides (IDENTITY_EXCHANGE above the join and for non-dim table, LOOKUP_LOCAL_EXCHANGE for dim table)
```mermaid
graph TD
subgraph before ["Before: Dim table split into separate fragment"]
A0[Downstream operators] --> A1["PhysicalJoin<br/>join_strategy=lookup"]
A1 --> B1[IDENTITY_EXCHANGE]
A1 --> C1["IDENTITY_EXCHANGE<br/>❌ splits dim into separate fragment"]
B1 --> D1[FactTableScan]
C1 --> E1[DimTableScan]
style C1 fill:#f88,stroke:#c00
style E1 fill:#f88,stroke:#c00
end
subgraph after ["After: LookupJoinRule isolates the lookup join"]
A2[Downstream operators] --> IX_ABOVE["IDENTITY_EXCHANGE<br/>(above — isolation)"]
IX_ABOVE --> J2["PhysicalJoin<br/>join_strategy=lookup"]
J2 --> IX_LEFT["IDENTITY_EXCHANGE<br/>(left — leaf boundary)"]
J2 --> LLE["LOOKUP_LOCAL_EXCHANGE<br/>(right — pseudo, no split)"]
IX_LEFT --> FS2[FactTableScan]
LLE --> DS2[DimTableScan]
style LLE fill:#8f8,stroke:#0a0
style IX_ABOVE fill:#8cf,stroke:#06c
style IX_LEFT fill:#8cf,stroke:#06c
end
before ~~~ after
```
Another alternative was to match MSE v1 behavior by skipping exchange on right side, but I didn't go down this path since omitting exchanges could break assumptions in other rules (every stage boundary has a PhysicalExchange) and we wouldn't get visibility in explain plans.
## Testing
We've deployed this on our production clusters and have compared production query results (multiple chained joins using dim tables for date spines and fx rates) with and without the physical optimizer. We haven't seen issues with correctness.
cc @ankitsultana @shauryachats
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4291374294
Docs PR opened: https://github.com/pinot-contrib/pinot-docs/pull/748
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4220807657
thanks @dang-stripe. will take a look at this over the weekend.
I am curious: have you seen any workloads where physical optimizer performs better than the other optimizer?
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on code in PR #18158:
URL: https://github.com/apache/pinot/pull/18158#discussion_r3081273410
##########
pinot-query-planner/src/test/resources/queries/PhysicalOptimizerPlans.json:
##########
@@ -1126,5 +1023,109 @@
]
}
]
+ },
+ "physical_opt_lookup_join": {
+ "queries": [
+ {
+ "description": "Simple lookup join — dim table uses LOOKUP_LOCAL_EXCHANGE, join is isolated by IDENTITY_EXCHANGE above and on the left",
+ "sql": "SET usePhysicalOptimizer=true; EXPLAIN PLAN FOR SELECT /*+ joinOptions(join_strategy='lookup') */ a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1",
+ "output": [
+ "Execution Plan",
+ "\nPhysicalExchange(exchangeStrategy=[SINGLETON_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0], col2=[$2])",
+ "\n PhysicalExchange(exchangeStrategy=[IDENTITY_EXCHANGE])",
+ "\n PhysicalJoin(condition=[=($0, $1)], joinType=[inner])",
+ "\n PhysicalExchange(exchangeStrategy=[IDENTITY_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0])",
+ "\n PhysicalTableScan(table=[[default, a]])",
+ "\n PhysicalExchange(exchangeStrategy=[LOOKUP_LOCAL_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0], col2=[$1])",
+ "\n PhysicalTableScan(table=[[default, b]])",
+ "\n"
+ ]
+ },
+ {
+ "description": "Lookup join with aggregation — lookup join feeds into GROUP BY with leaf/final aggregate split",
Review Comment:
yep added two tests cases below to cover the patterns we're doing in production
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4226149944
> have you seen any workloads where physical optimizer performs better than the other optimizer?
@ankitsultana thanks for looking! yep we're seeing around 50% reduction in p50/p75 broker latency for one of our workloads. this workload involves rather complex queries doing up to 5-6 joins + window operations. we both enabled physical optimizer for the queries and colocated segments for each partition to a single server, so we're seeing a large reduction in shuffle exchanges.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #18158:
URL: https://github.com/apache/pinot/pull/18158#discussion_r3069078358
##########
pinot-query-planner/src/test/resources/queries/PhysicalOptimizerPlans.json:
##########
@@ -1126,5 +1023,109 @@
]
}
]
+ },
+ "physical_opt_lookup_join": {
+ "queries": [
+ {
+ "description": "Simple lookup join — dim table uses LOOKUP_LOCAL_EXCHANGE, join is isolated by IDENTITY_EXCHANGE above and on the left",
+ "sql": "SET usePhysicalOptimizer=true; EXPLAIN PLAN FOR SELECT /*+ joinOptions(join_strategy='lookup') */ a.col1, b.col2 FROM a JOIN b ON a.col1 = b.col1",
+ "output": [
+ "Execution Plan",
+ "\nPhysicalExchange(exchangeStrategy=[SINGLETON_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0], col2=[$2])",
+ "\n PhysicalExchange(exchangeStrategy=[IDENTITY_EXCHANGE])",
+ "\n PhysicalJoin(condition=[=($0, $1)], joinType=[inner])",
+ "\n PhysicalExchange(exchangeStrategy=[IDENTITY_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0])",
+ "\n PhysicalTableScan(table=[[default, a]])",
+ "\n PhysicalExchange(exchangeStrategy=[LOOKUP_LOCAL_EXCHANGE])",
+ "\n PhysicalProject(col1=[$0], col2=[$1])",
+ "\n PhysicalTableScan(table=[[default, b]])",
+ "\n"
+ ]
+ },
+ {
+ "description": "Lookup join with aggregation — lookup join feeds into GROUP BY with leaf/final aggregate split",
Review Comment:
is it possible to add a model query that you folks are using in your production? want to make sure that regressions can be caught by unit tests itself
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4247328795
@ankitsultana got it. i'm curious, what's the high level approach to doing it properly?
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #18158:
URL: https://github.com/apache/pinot/pull/18158#discussion_r3069046381
##########
pinot-query-planner/src/test/resources/queries/PhysicalOptimizerPlans.json:
##########
@@ -1126,5 +1023,109 @@
]
}
]
+ },
+ "physical_opt_lookup_join": {
Review Comment:
Does this work if we are doing lookups on multiple tables? Maybe add a test for that too
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/v2/opt/rules/LookupJoinRule.java:
##########
@@ -0,0 +1,189 @@
+/**
+ * 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.physical.v2.opt.rules;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.calcite.rel.RelDistribution;
+import org.apache.calcite.rel.core.Join;
+import org.apache.pinot.calcite.rel.hint.PinotHintOptions;
+import org.apache.pinot.calcite.rel.traits.PinotExecStrategyTrait;
+import org.apache.pinot.query.context.PhysicalPlannerContext;
+import org.apache.pinot.query.planner.physical.v2.ExchangeStrategy;
+import org.apache.pinot.query.planner.physical.v2.PRelNode;
+import org.apache.pinot.query.planner.physical.v2.PinotDataDistribution;
+import org.apache.pinot.query.planner.physical.v2.nodes.PhysicalExchange;
+import org.apache.pinot.query.planner.physical.v2.opt.PRelNodeTransformer;
+
+
+/**
+ * Post-pass rule that isolates every lookup join in its own plan fragment.
+ *
+ * <p>Runs after {@link WorkerExchangeAssignmentRule}, which assigns workers and exchanges generically (with zero
+ * lookup join awareness). This rule then post-processes lookup joins to ensure:</p>
+ * <ol>
+ * <li><b>Right side</b>: Exchange converted to {@link ExchangeStrategy#LOOKUP_LOCAL_EXCHANGE} — a pseudo-exchange
+ * that does not split fragments. Later, during plan fragment assignment,
+ * {@code PlanFragmentAndMailboxAssignment.processLookupLocalExchange} registers the dim table with fake
+ * segments so the fragment is classified as a leaf stage for {@code DimensionTableDataManager} access.</li>
+ * <li><b>Left side</b>: Exchange exists (inserts {@link ExchangeStrategy#IDENTITY_EXCHANGE} if missing, e.g. when
+ * the left input is an intermediate node like a hash join with the same workers).</li>
+ * <li><b>Above</b>: Lookup join wrapped in {@link ExchangeStrategy#IDENTITY_EXCHANGE} so downstream nodes
+ * (other joins, sorts) are not absorbed into the lookup join's leaf fragment.</li>
+ * </ol>
+ *
+ * <p>This achieves the same outcome as V1, where {@code WorkerManager.assignWorkersToNonRootFragment} detects
+ * lookup joins and assigns the same workers as the fact table while registering fake empty segments for the
+ * dim table.</p>
+ */
+public class LookupJoinRule implements PRelNodeTransformer {
+ private final PhysicalPlannerContext _context;
+
+ public LookupJoinRule(PhysicalPlannerContext context) {
+ _context = context;
+ }
+
+ @Override
+ public PRelNode execute(PRelNode rootNode) {
+ // TODO: Support lookup joins in Lite mode. Currently fails with "Right input must be leaf
+ // operator" because the broker lacks DimensionTableDataManager.
+ if (_context.isUseLiteMode()) {
+ return rootNode;
+ }
+ return transform(rootNode, null);
+ }
Review Comment:
We have to approach lookup joins a bit differently to support it in a generic way that's compatible with the other optimizations. But for now this looks good to me.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4264419029
> @ankitsultana got it. i'm curious, what's the high level approach to doing it properly?
merged the PR for now to unblock and will raise a proper fix hopefully soon.
The proper way depends on the exact scope of features we want to support. I'll share a write-up soon on this.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4246290702
@dang-stripe : should be good to merge since this only impacts lookup joins. will wait for tests to pass.
I'll raise another PR hopefully soon to change the approach though. There's a different way we need to go about this to support lookup joins more broadly. Will share a design doc too. It also ties in with sub-plan based execution so it will enable a lot more optimizations in the future.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4245881438
@ankitsultana thanks for the review! addressed feedback. let me know if there's anything else i should address.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18158:
URL: https://github.com/apache/pinot/pull/18158#issuecomment-4220828805
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18158?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `85.57692%` with `15 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.42%. Comparing base ([`b5f9bce`](https://app.codecov.io/gh/apache/pinot/commit/b5f9bcecb5d39ecb2d6c73c6c03acd0100e7244e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e2e5c9e`](https://app.codecov.io/gh/apache/pinot/commit/e2e5c9ec2e233322e6a8bd03bf8879ad87838453?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 78 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18158?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../planner/physical/v2/opt/rules/LookupJoinRule.java](https://app.codecov.io/gh/apache/pinot/pull/18158?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Fphysical%2Fv2%2Fopt%2Frules%2FLookupJoinRule.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9waHlzaWNhbC92Mi9vcHQvcnVsZXMvTG9va3VwSm9pblJ1bGUuamF2YQ==) | 81.03% | [5 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../physical/v2/PlanFragmentAndMailboxAssignment.java](https://app.codecov.io/gh/apache/pinot/pull/18158?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fplanner%2Fphysical%2Fv2%2FPlanFragmentAndMailboxAssignment.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9waHlzaWNhbC92Mi9QbGFuRnJhZ21lbnRBbmRNYWlsYm94QXNzaWdubWVudC5qYXZh) | 90.24% | [1 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (b5f9bce) and HEAD (e2e5c9e). Click for more details.
>
> <details><summary>HEAD has 32 uploads less than BASE</summary>
>
>| Flag | BASE (b5f9bce) | HEAD (e2e5c9e) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|3|1|
>|temurin|9|1|
>|java-11|4|0|
>|unittests2|1|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18158 +/- ##
============================================
- Coverage 63.31% 55.42% -7.89%
+ Complexity 1540 826 -714
============================================
Files 3200 2519 -681
Lines 194221 144788 -49433
Branches 29929 23265 -6664
============================================
- Hits 122972 80255 -42717
+ Misses 61583 57630 -3953
+ Partials 9666 6903 -2763
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18158/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/18158/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/18158/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/18158/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/18158/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/18158/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/18158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.42% <85.57%> (-7.87%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.42% <85.57%> (-7.89%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.42% <85.57%> (-7.89%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.42% <85.57%> (-0.07%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18158/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/18158?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).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on code in PR #18158:
URL: https://github.com/apache/pinot/pull/18158#discussion_r3062036182
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/v2/opt/rules/LookupJoinRule.java:
##########
@@ -0,0 +1,189 @@
+/**
+ * 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.physical.v2.opt.rules;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.calcite.rel.RelDistribution;
+import org.apache.calcite.rel.core.Join;
+import org.apache.pinot.calcite.rel.hint.PinotHintOptions;
+import org.apache.pinot.calcite.rel.traits.PinotExecStrategyTrait;
+import org.apache.pinot.query.context.PhysicalPlannerContext;
+import org.apache.pinot.query.planner.physical.v2.ExchangeStrategy;
+import org.apache.pinot.query.planner.physical.v2.PRelNode;
+import org.apache.pinot.query.planner.physical.v2.PinotDataDistribution;
+import org.apache.pinot.query.planner.physical.v2.nodes.PhysicalExchange;
+import org.apache.pinot.query.planner.physical.v2.opt.PRelNodeTransformer;
+
+
+/**
+ * Post-pass rule that isolates every lookup join in its own plan fragment.
+ *
+ * <p>Runs after {@link WorkerExchangeAssignmentRule}, which assigns workers and exchanges generically (with zero
+ * lookup join awareness). This rule then post-processes lookup joins to ensure:</p>
+ * <ol>
+ * <li><b>Right side</b>: Exchange converted to {@link ExchangeStrategy#LOOKUP_LOCAL_EXCHANGE} — a pseudo-exchange
+ * that does not split fragments. Later, during plan fragment assignment,
+ * {@code PlanFragmentAndMailboxAssignment.processLookupLocalExchange} registers the dim table with fake
+ * segments so the fragment is classified as a leaf stage for {@code DimensionTableDataManager} access.</li>
+ * <li><b>Left side</b>: Exchange exists (inserts {@link ExchangeStrategy#IDENTITY_EXCHANGE} if missing, e.g. when
+ * the left input is an intermediate node like a hash join with the same workers).</li>
+ * <li><b>Above</b>: Lookup join wrapped in {@link ExchangeStrategy#IDENTITY_EXCHANGE} so downstream nodes
+ * (other joins, sorts) are not absorbed into the lookup join's leaf fragment.</li>
+ * </ol>
+ *
+ * <p>This achieves the same outcome as V1, where {@code WorkerManager.assignWorkersToNonRootFragment} detects
+ * lookup joins and assigns the same workers as the fact table while registering fake empty segments for the
+ * dim table.</p>
+ */
+public class LookupJoinRule implements PRelNodeTransformer {
+ private final PhysicalPlannerContext _context;
+
+ public LookupJoinRule(PhysicalPlannerContext context) {
+ _context = context;
+ }
+
+ @Override
+ public PRelNode execute(PRelNode rootNode) {
+ // TODO: Support lookup joins in Lite mode. Currently fails with "Right input must be leaf
+ // operator" because the broker lacks DimensionTableDataManager.
+ if (_context.isUseLiteMode()) {
+ return rootNode;
+ }
+ return transform(rootNode, null);
+ }
Review Comment:
i've skipped lite mode since it adds quite a bit of scope. the broker gets assigned as worker for the join fragment so we'd need reassign it to a server and handle routing.
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana merged PR #18158:
URL: https://github.com/apache/pinot/pull/18158
--
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] [multistage] add lookup join support to physical optimizer [pinot]
Posted by "dang-stripe (via GitHub)" <gi...@apache.org>.
dang-stripe commented on code in PR #18158:
URL: https://github.com/apache/pinot/pull/18158#discussion_r3081269955
##########
pinot-query-planner/src/test/resources/queries/PhysicalOptimizerPlans.json:
##########
@@ -1126,5 +1023,109 @@
]
}
]
+ },
+ "physical_opt_lookup_join": {
Review Comment:
yep it does. added a test for it.
--
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