You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2018/07/31 20:00:17 UTC
[GitHub] ilooner closed pull request #1372: DRILL-6589: Push transitive
closure predicates past aggregates/projects
ilooner closed pull request #1372: DRILL-6589: Push transitive closure predicates past aggregates/projects
URL: https://github.com/apache/drill/pull/1372
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/PlannerPhase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java
index e5a3746a42f..2d02011dc80 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java
@@ -559,6 +559,7 @@ static RuleSet getJoinTransitiveClosureRules() {
RuleInstance.DRILL_JOIN_PUSH_TRANSITIVE_PREDICATES_RULE,
DrillFilterJoinRules.DRILL_FILTER_INTO_JOIN,
RuleInstance.REMOVE_IS_NOT_DISTINCT_FROM_RULE,
+ DrillFilterAggregateTransposeRule.DRILL_LOGICAL_INSTANCE,
RuleInstance.DRILL_FILTER_MERGE_RULE
).build());
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillAggregateRelBase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillAggregateRelBase.java
index 18103c44c58..cd1f4fa46f1 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillAggregateRelBase.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillAggregateRelBase.java
@@ -17,24 +17,23 @@
*/
package org.apache.drill.exec.planner.common;
-import java.util.List;
-
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.AggregateCall;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.util.ImmutableBitSet;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.expr.holders.IntHolder;
import org.apache.drill.exec.planner.cost.DrillCostBase;
import org.apache.drill.exec.planner.cost.DrillCostBase.DrillCostFactory;
-import org.apache.calcite.plan.RelOptPlanner;
-import org.apache.calcite.rel.core.Aggregate;
-import org.apache.calcite.rel.core.AggregateCall;
-import org.apache.calcite.rel.InvalidRelException;
-import org.apache.calcite.rel.RelNode;
-import org.apache.calcite.util.ImmutableBitSet;
-import org.apache.calcite.plan.RelOptCluster;
-import org.apache.calcite.plan.RelOptCost;
-import org.apache.calcite.plan.RelTraitSet;
import org.apache.drill.exec.planner.physical.PrelUtil;
+import java.util.List;
+
/**
* Base class for logical and physical Aggregations implemented in Drill
@@ -42,11 +41,10 @@
public abstract class DrillAggregateRelBase extends Aggregate implements DrillRelNode {
public DrillAggregateRelBase(RelOptCluster cluster, RelTraitSet traits, RelNode child, boolean indicator,
- ImmutableBitSet groupSet, List<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) throws InvalidRelException {
+ ImmutableBitSet groupSet, List<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) {
super(cluster, traits, child, indicator, groupSet, groupSets, aggCalls);
}
-
/**
* Estimate cost of hash agg. Called by DrillAggregateRel.computeSelfCost() and HashAggPrel.computeSelfCost()
*/
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java
index 55cd7bfc22a..5a7421b6679 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java
@@ -17,11 +17,15 @@
*/
package org.apache.drill.exec.planner.logical;
-import java.util.List;
-
+import com.google.common.collect.Lists;
import org.apache.calcite.linq4j.Ord;
+import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelOptCost;
import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.AggregateCall;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.type.SqlTypeName;
@@ -36,14 +40,8 @@
import org.apache.drill.common.logical.data.LogicalOperator;
import org.apache.drill.exec.planner.common.DrillAggregateRelBase;
import org.apache.drill.exec.planner.torel.ConversionContext;
-import org.apache.calcite.rel.core.AggregateCall;
-import org.apache.calcite.rel.core.Aggregate;
-import org.apache.calcite.rel.InvalidRelException;
-import org.apache.calcite.rel.RelNode;
-import org.apache.calcite.plan.RelOptCluster;
-import org.apache.calcite.plan.RelTraitSet;
-import com.google.common.collect.Lists;
+import java.util.List;
/**
* Aggregation implemented in Drill.
@@ -51,17 +49,13 @@
public class DrillAggregateRel extends DrillAggregateRelBase implements DrillRel {
/** Creates a DrillAggregateRel. */
public DrillAggregateRel(RelOptCluster cluster, RelTraitSet traits, RelNode child, boolean indicator, ImmutableBitSet groupSet,
- List<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) throws InvalidRelException {
+ List<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) {
super(cluster, traits, child, indicator, groupSet, groupSets, aggCalls);
}
@Override
public Aggregate copy(RelTraitSet traitSet, RelNode input, boolean indicator, ImmutableBitSet groupSet, List<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) {
- try {
- return new DrillAggregateRel(getCluster(), traitSet, input, indicator, groupSet, groupSets, aggCalls);
- } catch (InvalidRelException e) {
- throw new AssertionError(e);
- }
+ return new DrillAggregateRel(getCluster(), traitSet, input, indicator, groupSet, groupSets, aggCalls);
}
@Override
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRule.java
index 2b998b2902c..75f806f37a4 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRule.java
@@ -17,15 +17,12 @@
*/
package org.apache.drill.exec.planner.logical;
-import org.apache.calcite.rel.InvalidRelException;
-import org.apache.calcite.rel.RelNode;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.logical.LogicalAggregate;
-import org.apache.calcite.util.trace.CalciteTrace;
-import org.slf4j.Logger;
/**
* Rule that converts an {@link LogicalAggregate} to a {@link DrillAggregateRel}, implemented by a Drill "segment" operation
@@ -33,7 +30,6 @@
*/
public class DrillAggregateRule extends RelOptRule {
public static final RelOptRule INSTANCE = new DrillAggregateRule();
- protected static final Logger tracer = CalciteTrace.getPlannerTracer();
private DrillAggregateRule() {
super(
@@ -55,11 +51,7 @@ public void onMatch(RelOptRuleCall call) {
final RelTraitSet traits = aggregate.getTraitSet().plus(DrillRel.DRILL_LOGICAL);
final RelNode convertedInput = convert(input, input.getTraitSet().plus(DrillRel.DRILL_LOGICAL).simplify());
- try {
- call.transformTo(new DrillAggregateRel(aggregate.getCluster(), traits, convertedInput, aggregate.indicator,
- aggregate.getGroupSet(), aggregate.getGroupSets(), aggregate.getAggCallList()));
- } catch (InvalidRelException e) {
- tracer.warn(e.toString());
- }
+ call.transformTo(new DrillAggregateRel(aggregate.getCluster(), traits, convertedInput, aggregate.indicator,
+ aggregate.getGroupSet(), aggregate.getGroupSets(), aggregate.getAggCallList()));
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillFilterAggregateTransposeRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillFilterAggregateTransposeRule.java
index 9f2088d9ba5..6bb409ee742 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillFilterAggregateTransposeRule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillFilterAggregateTransposeRule.java
@@ -24,18 +24,22 @@
import org.apache.calcite.rel.core.Filter;
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.rules.FilterAggregateTransposeRule;
+import org.apache.calcite.tools.RelBuilderFactory;
import org.apache.drill.exec.planner.DrillRelBuilder;
public class DrillFilterAggregateTransposeRule extends FilterAggregateTransposeRule{
// Since Calcite's default FilterAggregateTransposeRule would match Filter on top of Aggregate, it potentially will match Rels with mixed CONVENTION trait.
- // Here override match method, such that the rule matchs with Rel in the same CONVENTION.
+ // Here override match method, such that the rule matches with Rel in the same CONVENTION.
- public static final FilterAggregateTransposeRule INSTANCE = new DrillFilterAggregateTransposeRule();
+ public static final FilterAggregateTransposeRule INSTANCE = new DrillFilterAggregateTransposeRule(
+ DrillRelBuilder.proto(Contexts.of(RelFactories.DEFAULT_FILTER_FACTORY)));
- private DrillFilterAggregateTransposeRule() {
- super(Filter.class, DrillRelBuilder.proto(Contexts.of(RelFactories.DEFAULT_FILTER_FACTORY)),
- Aggregate.class);
+ public static final FilterAggregateTransposeRule DRILL_LOGICAL_INSTANCE = new DrillFilterAggregateTransposeRule(
+ DrillRelBuilder.proto(DrillRelFactories.DRILL_LOGICAL_FILTER_FACTORY, DrillRelFactories.DRILL_LOGICAL_AGGREGATE_FACTORY));
+
+ private DrillFilterAggregateTransposeRule(RelBuilderFactory relBuilderFactory) {
+ super(Filter.class, relBuilderFactory, Aggregate.class);
}
@Override
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java
index 55b295848b2..ac814fc4acb 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceAggregatesRule.java
@@ -17,36 +17,19 @@
*/
package org.apache.drill.exec.planner.logical;
-import java.math.BigDecimal;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.calcite.plan.Convention;
import org.apache.calcite.plan.RelOptCluster;
-import org.apache.calcite.rel.InvalidRelException;
-import org.apache.calcite.rel.core.Aggregate;
-import org.apache.calcite.rel.core.Window;
-import org.apache.calcite.rel.logical.LogicalAggregate;
-import org.apache.calcite.sql.SqlKind;
-import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlOperatorBinding;
-import org.apache.calcite.sql.fun.SqlCountAggFunction;
-import org.apache.calcite.sql.type.SqlReturnTypeInference;
-import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.util.trace.CalciteTrace;
-import org.apache.drill.exec.planner.physical.PlannerSettings;
-import org.apache.drill.exec.planner.sql.DrillCalciteSqlAggFunctionWrapper;
-import org.apache.drill.exec.planner.sql.DrillSqlOperator;
-import org.apache.calcite.rel.core.AggregateCall;
-import org.apache.calcite.rel.RelNode;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.core.Window;
+import org.apache.calcite.rel.logical.LogicalAggregate;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
@@ -54,18 +37,31 @@
import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.fun.SqlAvgAggFunction;
+import org.apache.calcite.sql.fun.SqlCountAggFunction;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.fun.SqlSumAggFunction;
import org.apache.calcite.sql.fun.SqlSumEmptyIsZeroAggFunction;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
+import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.util.CompositeList;
import org.apache.calcite.util.ImmutableIntList;
import org.apache.calcite.util.Util;
-
-import com.google.common.collect.ImmutableList;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.planner.sql.DrillCalciteSqlAggFunctionWrapper;
+import org.apache.drill.exec.planner.sql.DrillSqlOperator;
import org.apache.drill.exec.planner.sql.TypeInferenceUtils;
import org.apache.drill.exec.planner.sql.parser.DrillCalciteWrapperUtility;
-import org.slf4j.Logger;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
/**
* Rule to reduce aggregates to simpler forms. Currently only AVG(x) to
@@ -693,7 +689,6 @@ private RelDataType getFieldType(RelNode relNode, int i) {
}
private static class DrillConvertSumToSumZero extends RelOptRule {
- protected static final Logger tracer = CalciteTrace.getPlannerTracer();
public DrillConvertSumToSumZero(RelOptRuleOperand operand) {
super(operand, DrillRelFactories.LOGICAL_BUILDER, null);
@@ -744,18 +739,14 @@ public void onMatch(RelOptRuleCall call) {
}
}
- try {
- call.transformTo(new DrillAggregateRel(
- oldAggRel.getCluster(),
- oldAggRel.getTraitSet(),
- oldAggRel.getInput(),
- oldAggRel.indicator,
- oldAggRel.getGroupSet(),
- oldAggRel.getGroupSets(),
- newAggregateCalls));
- } catch (InvalidRelException e) {
- tracer.warn(e.toString());
- }
+ call.transformTo(new DrillAggregateRel(
+ oldAggRel.getCluster(),
+ oldAggRel.getTraitSet(),
+ oldAggRel.getInput(),
+ oldAggRel.indicator,
+ oldAggRel.getGroupSet(),
+ oldAggRel.getGroupSets(),
+ newAggregateCalls));
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRelFactories.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRelFactories.java
index 59e4d06fc99..fcf93fa6c67 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRelFactories.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRelFactories.java
@@ -17,9 +17,11 @@
*/
package org.apache.drill.exec.planner.logical;
+import com.google.common.collect.ImmutableList;
import org.apache.calcite.plan.Contexts;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.AggregateCall;
import org.apache.calcite.rel.core.CorrelationId;
import org.apache.calcite.rel.core.JoinRelType;
import org.apache.calcite.rel.core.RelFactories;
@@ -27,6 +29,7 @@
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
import org.apache.drill.exec.planner.DrillRelBuilder;
import java.util.List;
@@ -48,7 +51,7 @@
*/
public class DrillRelFactories {
-
+ private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillRelFactories.class);
public static final RelFactories.ProjectFactory DRILL_LOGICAL_PROJECT_FACTORY =
new DrillProjectFactoryImpl();
@@ -57,6 +60,7 @@
public static final RelFactories.JoinFactory DRILL_LOGICAL_JOIN_FACTORY = new DrillJoinFactoryImpl();
+ public static final RelFactories.AggregateFactory DRILL_LOGICAL_AGGREGATE_FACTORY = new DrillAggregateFactoryImpl();
/**
* A {@link RelBuilderFactory} that creates a {@link DrillRelBuilder} that will
* create logical relational expressions for everything.
@@ -76,7 +80,7 @@
/**
* Implementation of {@link RelFactories.ProjectFactory} that returns a vanilla
- * {@link org.apache.calcite.rel.logical.LogicalProject}.
+ * {@link DrillProjectRel}.
*/
private static class DrillProjectFactoryImpl implements RelFactories.ProjectFactory {
@Override
@@ -92,7 +96,7 @@ public RelNode createProject(RelNode child,
/**
* Implementation of {@link RelFactories.FilterFactory} that
- * returns a vanilla {@link LogicalFilter}.
+ * returns a vanilla {@link DrillFilterRel}.
*/
private static class DrillFilterFactoryImpl implements RelFactories.FilterFactory {
@Override
@@ -103,7 +107,7 @@ public RelNode createFilter(RelNode child, RexNode condition) {
/**
* Implementation of {@link RelFactories.JoinFactory} that returns a vanilla
- * {@link org.apache.calcite.rel.logical.LogicalJoin}.
+ * {@link DrillJoinRel}.
*/
private static class DrillJoinFactoryImpl implements RelFactories.JoinFactory {
@@ -122,4 +126,16 @@ public RelNode createJoin(RelNode left, RelNode right,
}
}
+ /**
+ * Implementation of {@link RelFactories.AggregateFactory} that returns a vanilla
+ * {@link DrillAggregateRel}.
+ */
+ private static class DrillAggregateFactoryImpl implements RelFactories.AggregateFactory {
+
+ @Override
+ public RelNode createAggregate(RelNode input, boolean indicator, ImmutableBitSet groupSet,
+ ImmutableList<ImmutableBitSet> groupSets, List<AggregateCall> aggCalls) {
+ return new DrillAggregateRel(input.getCluster(), input.getTraitSet(), input, indicator, groupSet, groupSets, aggCalls);
+ }
+ }
}
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetFilterPushdownWithTransitivePredicates.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetFilterPushdownWithTransitivePredicates.java
index 1f8f0d90a78..312abf27071 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetFilterPushdownWithTransitivePredicates.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetFilterPushdownWithTransitivePredicates.java
@@ -269,5 +269,19 @@ public void testForWithStatementAndDynamicStar() throws Exception {
final String[] expectedPlan = {"first.*numRowGroups=2", "second.*numRowGroups=1"};
testPlanMatchingPatterns(query, expectedPlan);
}
+
+ @Test
+ public void testForTransitiveFilterPushPastAgg() throws Exception {
+ String query = String.format("SELECT t1.`year` FROM %s t1 WHERE t1.`month` = 7 AND t1.`period` = 2 AND t1.`month` IN " +
+ "(SELECT t2.`month` FROM %s t2)", FIRST_TABLE_NAME, SECOND_TABLE_NAME);
+
+ // Validate the plan
+ int actualRowCount = testSql(query);
+ int expectedRowCount = 2;
+ assertEquals("Expected and actual row count should match", expectedRowCount, actualRowCount);
+
+ final String[] expectedPlan = {"first.*numRowGroups=1", "second.*numRowGroups=1"};
+ testPlanMatchingPatterns(query, expectedPlan);
+ }
}
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services