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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
