SQL: More straightforward handling of join planning. #9648
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,18 +22,21 @@ | |
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.Iterables; | ||
| import org.apache.calcite.plan.RelOptCluster; | ||
| import org.apache.calcite.plan.RelOptCost; | ||
| import org.apache.calcite.plan.RelOptPlanner; | ||
| import org.apache.calcite.plan.RelOptRule; | ||
| import org.apache.calcite.plan.RelTraitSet; | ||
| import org.apache.calcite.plan.volcano.RelSubset; | ||
| import org.apache.calcite.rel.RelNode; | ||
| import org.apache.calcite.rel.RelWriter; | ||
| import org.apache.calcite.rel.core.Join; | ||
| import org.apache.calcite.rel.core.JoinRelType; | ||
| import org.apache.calcite.rel.metadata.RelMetadataQuery; | ||
| import org.apache.calcite.rel.type.RelDataType; | ||
| import org.apache.calcite.rex.RexNode; | ||
| import org.apache.calcite.sql.SqlKind; | ||
| import org.apache.druid.java.util.common.IAE; | ||
| import org.apache.druid.java.util.common.Pair; | ||
| import org.apache.druid.java.util.common.StringUtils; | ||
|
|
@@ -66,32 +69,10 @@ public class DruidJoinQueryRel extends DruidRel<DruidJoinQueryRel> | |
| private RelNode left; | ||
| private RelNode right; | ||
|
|
||
| private DruidJoinQueryRel( | ||
| RelOptCluster cluster, | ||
| RelTraitSet traitSet, | ||
| Join joinRel, | ||
| PartialDruidQuery partialQuery, | ||
| QueryMaker queryMaker | ||
| ) | ||
|
|
@@ -100,8 +81,6 @@ private DruidJoinQueryRel( | |
| this.joinRel = joinRel; | ||
| this.left = joinRel.getLeft(); | ||
| this.right = joinRel.getRight(); | ||
| this.partialQuery = partialQuery; | ||
| } | ||
|
|
||
|
|
@@ -110,18 +89,15 @@ private DruidJoinQueryRel( | |
| */ | ||
| public static DruidJoinQueryRel create( | ||
| final Join joinRel, | ||
| final QueryMaker queryMaker | ||
| ) | ||
| { | ||
| return new DruidJoinQueryRel( | ||
| joinRel.getCluster(), | ||
| joinRel.getTraitSet(), | ||
| joinRel, | ||
| PartialDruidQuery.create(joinRel), | ||
| queryMaker | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -149,19 +125,11 @@ public DruidJoinQueryRel withPartialQuery(final PartialDruidQuery newQueryBuilde | |
| getCluster(), | ||
| getTraitSet().plusAll(newQueryBuilder.getRelTraits()), | ||
| joinRel, | ||
| newQueryBuilder, | ||
| getQueryMaker() | ||
| ); | ||
| } | ||
|
|
||
| @Override | ||
| public DruidQuery toDruidQuery(final boolean finalizeAggregations) | ||
| { | ||
|
|
@@ -176,18 +144,14 @@ public DruidQuery toDruidQuery(final boolean finalizeAggregations) | |
| final DataSource rightDataSource; | ||
|
|
||
| if (computeLeftRequiresSubquery(leftDruidRel)) { | ||
| leftDataSource = new QueryDataSource(leftQuery.getQuery()); | ||
| } else { | ||
| leftDataSource = leftQuery.getDataSource(); | ||
| } | ||
|
|
||
| if (computeRightRequiresSubquery(rightDruidRel)) { | ||
| rightDataSource = new QueryDataSource(rightQuery.getQuery()); | ||
| } else { | ||
| rightDataSource = rightQuery.getDataSource(); | ||
| } | ||
|
|
||
|
|
@@ -250,8 +214,6 @@ public DruidJoinQueryRel asDruidConvention() | |
| .map(input -> RelOptRule.convert(input, DruidConvention.instance())) | ||
| .collect(Collectors.toList()) | ||
| ), | ||
| partialQuery, | ||
| getQueryMaker() | ||
| ); | ||
|
|
@@ -290,8 +252,6 @@ public RelNode copy(final RelTraitSet traitSet, final List<RelNode> inputs) | |
| getCluster(), | ||
| traitSet, | ||
| joinRel.copy(joinRel.getTraitSet(), inputs), | ||
| getPartialDruidQuery(), | ||
| getQueryMaker() | ||
| ); | ||
|
|
@@ -319,12 +279,9 @@ public RelWriter explainTerms(RelWriter pw) | |
| throw new RuntimeException(e); | ||
| } | ||
|
|
||
| return joinRel.explainTerms(pw) | ||
| .item("query", queryString) | ||
| .item("signature", druidQuery.getOutputRowSignature()); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -336,10 +293,23 @@ protected RelDataType deriveRowType() | |
| @Override | ||
| public RelOptCost computeSelfCost(final RelOptPlanner planner, final RelMetadataQuery mq) | ||
| { | ||
| double cost; | ||
|
|
||
| if (computeLeftRequiresSubquery(getSomeDruidChild(left))) { | ||
| cost = CostEstimates.COST_JOIN_SUBQUERY; | ||
| } else { | ||
| cost = partialQuery.estimateCost(); | ||
| } | ||
|
|
||
| if (computeRightRequiresSubquery(getSomeDruidChild(right))) { | ||
| cost += CostEstimates.COST_JOIN_SUBQUERY; | ||
| } | ||
|
|
||
| if (joinRel.getCondition().isA(SqlKind.LITERAL) && !joinRel.getCondition().isAlwaysFalse()) { | ||
| cost += CostEstimates.COST_JOIN_CROSS; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. should we add a branch to set the cost to 0 if the joinCondition is a literal and is always false? since a false join condition means nothing will match therefore you don't need to do work for either the left or the right hand side?
Contributor
Author
There was a problem hiding this comment. Currently, the native query join handling isn't smart enough to totally eliminate a join that has a false condition. It will still evaluate the left and right hand sides if they are subqueries. And it will still walk through every row on the left hand side. So I think it is fair to keep considering these costs in the cost estimator. |
||
|
|
||
| return planner.getCostFactory().makeCost(cost, 0, 0); | ||
| } | ||
|
|
||
| private static JoinType toDruidJoinType(JoinRelType calciteJoinType) | ||
|
|
@@ -395,4 +365,14 @@ private static Pair<String, RowSignature> computeJoinRowSignature( | |
|
|
||
| return Pair.of(rightPrefix, signatureBuilder.build()); | ||
| } | ||
|
|
||
| private static DruidRel<?> getSomeDruidChild(final RelNode child) | ||
| { | ||
| if (child instanceof DruidRel) { | ||
| return (DruidRel<?>) child; | ||
| } else { | ||
| final RelSubset subset = (RelSubset) child; | ||
|
Contributor
There was a problem hiding this comment. how do we know this will be a RelSubset? I couldn't trace that path down here
Contributor
Author
There was a problem hiding this comment. My understanding of Calcite's planner tells me that the children will either be single rels or will be a subset of equivalent rels. So DruidRel and RelSubset are the two cases that can happen. I hope I understand the planner correctly — I can't point to any specific code that guarantees what I am saying is true.
Contributor
There was a problem hiding this comment. when I wrote this, I was wondering whether bindable parameters would change the node here somehow. The calcite query tests are pretty comprehensive around different types of JOINs and nested queries, that I feel pretty confident to agree with your understanding. Any chance you can test this with parameterized sql.
Contributor
Author
There was a problem hiding this comment. I will look at adding some tests for this in the same followup as #9648 (comment).
Contributor
There was a problem hiding this comment. "subset of equivalent rels" -> Does this imply that the cost of each RelNode will be equal? I saw that computeLeftRequiresSubquery can returns different depending on which child we pick from the list of RelList. This then result in a very very different cost since a child that results in requiring subquery will have very high cost and a child that doesnt will have a much lower cost. More specifically, I saw two RelNode in the list. One RelNode has a filter = null and the other has "filter":{"type":"selector","dimension":"v","value":"xa","extractionFn":null} |
||
| return (DruidRel<?>) Iterables.getFirst(subset.getRels(), null); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity where did these numbers come from? Experiments I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly I made them up, but then verified through experiments that they achieve the plans that we want to achieve.