[ https://issues.apache.org/jira/browse/DRILL-6476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16511344#comment-16511344 ]
ASF GitHub Bot commented on DRILL-6476: --------------------------------------- ilooner closed pull request #1308: DRILL-6476: Generate explain plan which shows relation between Latera… URL: https://github.com/apache/drill/pull/1308 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PhysicalPlanCreator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PhysicalPlanCreator.java index 6a94662e78..220add66fb 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PhysicalPlanCreator.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PhysicalPlanCreator.java @@ -52,11 +52,6 @@ public QueryContext getContext() { return context; } -// public int getOperatorId(Prel prel){ -// OpId id = opIdMap.get(prel); -// return id.getAsSingleInt(); -// } - public PhysicalOperator addMetadata(Prel originalPrel, PhysicalOperator op){ op.setOperatorId(opIdMap.get(originalPrel).getAsSingleInt()); op.setCost(originalPrel.estimateRowCount(originalPrel.getCluster().getMetadataQuery())); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnnestPrel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnnestPrel.java index cd598eb989..a22beea0e3 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnnestPrel.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/UnnestPrel.java @@ -75,4 +75,7 @@ public boolean needsFinalColumnReordering() { return true; } + public Class<?> getParentClass() { + return CorrelatePrel.class; + } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/explain/NumberingRelWriter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/explain/NumberingRelWriter.java index 045dba9d48..38b97b67d6 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/explain/NumberingRelWriter.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/explain/NumberingRelWriter.java @@ -19,6 +19,7 @@ import java.io.PrintWriter; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -31,8 +32,10 @@ import org.apache.calcite.runtime.FlatLists; import org.apache.calcite.sql.SqlExplainLevel; import org.apache.calcite.util.Pair; +import org.apache.drill.exec.planner.physical.CorrelatePrel; import org.apache.drill.exec.planner.physical.HashJoinPrel; import org.apache.drill.exec.planner.physical.Prel; +import org.apache.drill.exec.planner.physical.UnnestPrel; import org.apache.drill.exec.planner.physical.explain.PrelSequencer.OpId; import com.google.common.collect.ImmutableList; @@ -47,6 +50,7 @@ private final SqlExplainLevel detailLevel; protected final Spacer spacer = new Spacer(); private final List<Pair<String, Object>> values = new ArrayList<>(); + private final Map<String, Prel> sourceOperatorRegistry; private final Map<Prel, OpId> ids; //~ Constructors ----------------------------------------------------------- @@ -55,6 +59,7 @@ public NumberingRelWriter(Map<Prel, OpId> ids, PrintWriter pw, SqlExplainLevel d this.pw = pw; this.ids = ids; this.detailLevel = detailLevel; + this.sourceOperatorRegistry = new HashMap<>(); } //~ Methods ---------------------------------------------------------------- @@ -62,16 +67,10 @@ public NumberingRelWriter(Map<Prel, OpId> ids, PrintWriter pw, SqlExplainLevel d protected void explain_( RelNode rel, List<Pair<String, Object>> values) { - List<RelNode> inputs = rel.getInputs(); - if (rel instanceof HashJoinPrel && ((HashJoinPrel) rel).isSwapped()) { - HashJoinPrel joinPrel = (HashJoinPrel) rel; - inputs = FlatLists.of(joinPrel.getRight(), joinPrel.getLeft()); - } - RelMetadataQuery mq = RelMetadataQuery.instance(); if (!mq.isVisibleInExplain(rel, detailLevel)) { // render children in place of this, at same level - explainInputs(inputs); + explainInputs(rel); return; } @@ -95,6 +94,7 @@ protected void explain_( s.append(rel.getRelTypeName().replace("Prel", "")); if (detailLevel != SqlExplainLevel.NO_ATTRIBUTES) { int j = 0; + s.append(getDependentSrcOp(rel)); for (Pair<String, Object> value : values) { if (value.right instanceof RelNode) { continue; @@ -125,14 +125,61 @@ protected void explain_( } pw.println(s); spacer.add(2); - explainInputs(inputs); + explainInputs(rel); spacer.subtract(2); } - private void explainInputs(List<RelNode> inputs) { - for (RelNode input : inputs) { - input.explain(this); + private String getDependentSrcOp(RelNode rel) { + if (rel instanceof UnnestPrel) { + return this.getDependentSrcOp((UnnestPrel) rel); + } + return ""; + } + + private String getDependentSrcOp(UnnestPrel unnest) { + Prel parent = this.getRegisteredPrel(unnest.getParentClass()); + if (parent != null && parent instanceof CorrelatePrel) { + OpId id = ids.get(parent); + return String.format(" [srcOp=%02d-%02d] ", id.fragmentId, id.opId); } + return ""; + } + + public void register(Prel toRegister) { + this.sourceOperatorRegistry.put(toRegister.getClass().getSimpleName(), toRegister); + } + + public Prel getRegisteredPrel(Class<?> classname) { + return this.sourceOperatorRegistry.get(classname.getSimpleName()); + } + + public void unRegister(Prel unregister) { + this.sourceOperatorRegistry.remove(unregister.getClass().getSimpleName()); + } + + + private void explainInputs(RelNode rel) { + if (rel instanceof CorrelatePrel) { + this.explainInputs((CorrelatePrel) rel); + } else { + List<RelNode> inputs = rel.getInputs(); + if (rel instanceof HashJoinPrel && ((HashJoinPrel) rel).isSwapped()) { + HashJoinPrel joinPrel = (HashJoinPrel) rel; + inputs = FlatLists.of(joinPrel.getRight(), joinPrel.getLeft()); + } + for (RelNode input : inputs) { + input.explain(this); + } + } + } + + //Correlate is handled differently because explain plan + //needs to show relation between Lateral and Unnest operators. + private void explainInputs(CorrelatePrel correlate) { + correlate.getInput(0).explain(this); + this.register(correlate); + correlate.getInput(1).explain(this); + this.unRegister(correlate); } public final void explain(RelNode rel, List<Pair<String, Object>> valueList) { diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java index 00ab971e15..d027e77afd 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java @@ -435,4 +435,25 @@ private String getRightChildOfLateral(String explain) throws Exception { String CorrelateUnnest = matcher.group(0); return CorrelateUnnest.substring(CorrelateUnnest.lastIndexOf("Scan")); } + + + //The following test is for testing the explain plan contains relation between lateral and corresponding unnest. + @Test + public void testLateralAndUnnestExplainPlan() throws Exception { + String Sql = "select c.* from cp.`lateraljoin/nested-customer.json` c, unnest(c.orders) Orders(ord)"; + ClusterFixtureBuilder builder = ClusterFixture.builder(dirTestWatcher) + .setOptionDefault(ExecConstants.ENABLE_UNNEST_LATERAL_KEY, true) + .setOptionDefault(ExecConstants.SLICE_TARGET, 1); + + try (ClusterFixture cluster = builder.build(); + ClientFixture client = cluster.clientFixture()) { + String explain = client.queryBuilder().sql(Sql).explainText(); + String srcOp = explain.substring(explain.indexOf("srcOp")); + assertTrue(srcOp != null && srcOp.length() > 0); + String correlateFragmentPattern = srcOp.substring(srcOp.indexOf("=")+1, srcOp.indexOf("]")); + assertTrue(correlateFragmentPattern != null && correlateFragmentPattern.length() > 0); + Matcher matcher = Pattern.compile(correlateFragmentPattern + ".*Correlate", Pattern.MULTILINE | Pattern.DOTALL).matcher(explain); + assertTrue(matcher.find()); + } + } } ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Generate explain plan which shows relation between Lateral and the > corresponding Unnest. > ---------------------------------------------------------------------------------------- > > Key: DRILL-6476 > URL: https://issues.apache.org/jira/browse/DRILL-6476 > Project: Apache Drill > Issue Type: Bug > Components: Query Planning & Optimization > Affects Versions: 1.14.0 > Reporter: Hanumath Rao Maduri > Assignee: Hanumath Rao Maduri > Priority: Major > Labels: ready-to-commit > Fix For: 1.14.0 > > > Currently, explain plan doesn't show that which lateral and unnest node's > are related. This information is good to have so that the visual plan can use > it and show the relation visually. > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)