You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2015/09/03 00:15:56 UTC
[08/50] incubator-calcite git commit: [CALCITE-799] Incorrect result
for "HAVING count(*) > 1"
[CALCITE-799] Incorrect result for "HAVING count(*) > 1"
FilterAggregateTransposeRule was mixing up group keys, and as a result would sometimes push down a filter on an aggregate function when it should not have.
Project: http://git-wip-us.apache.org/repos/asf/incubator-calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/92f32e8d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-calcite/tree/92f32e8d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-calcite/diff/92f32e8d
Branch: refs/heads/branch-release
Commit: 92f32e8d5b0301e91bc31b3b95ae6049824b9ecf
Parents: c57d807
Author: Julian Hyde <jh...@apache.org>
Authored: Mon Jul 13 12:57:46 2015 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Mon Jul 13 12:57:46 2015 -0700
----------------------------------------------------------------------
.../rel/rules/FilterAggregateTransposeRule.java | 36 +++++++++++-------
.../apache/calcite/test/RelOptRulesTest.java | 8 ++++
.../org/apache/calcite/test/RelOptRulesTest.xml | 23 +++++++++++
core/src/test/resources/sql/agg.oq | 40 ++++++++++++++++++++
4 files changed, 93 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/92f32e8d/core/src/main/java/org/apache/calcite/rel/rules/FilterAggregateTransposeRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/FilterAggregateTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/FilterAggregateTransposeRule.java
index d3de47f..720e33d 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/FilterAggregateTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/FilterAggregateTransposeRule.java
@@ -88,7 +88,6 @@ public class FilterAggregateTransposeRule extends RelOptRule {
final List<RexNode> conditions =
RelOptUtil.conjunctions(filterRel.getCondition());
- final ImmutableBitSet groupKeys = aggRel.getGroupSet();
final RexBuilder rexBuilder = filterRel.getCluster().getRexBuilder();
final List<RelDataTypeField> origFields =
aggRel.getRowType().getFieldList();
@@ -98,19 +97,7 @@ public class FilterAggregateTransposeRule extends RelOptRule {
for (RexNode condition : conditions) {
ImmutableBitSet rCols = RelOptUtil.InputFinder.bits(condition);
- boolean push = groupKeys.contains(rCols);
- if (push && aggRel.indicator) {
- // If grouping sets are used, the filter can be pushed if
- // the columns referenced in the predicate are present in
- // all the grouping sets.
- for (ImmutableBitSet groupingSet: aggRel.getGroupSets()) {
- if (!groupingSet.contains(rCols)) {
- push = false;
- break;
- }
- }
- }
- if (push) {
+ if (canPush(aggRel, rCols)) {
pushedConditions.add(
condition.accept(
new RelOptUtil.RexInputConverter(rexBuilder, origFields,
@@ -131,6 +118,27 @@ public class FilterAggregateTransposeRule extends RelOptRule {
rel = builder.push(rel).filter(remainingConditions).build();
call.transformTo(rel);
}
+
+ private boolean canPush(Aggregate aggregate, ImmutableBitSet rCols) {
+ // If the filter references columns not in the group key, we cannot push
+ final ImmutableBitSet groupKeys =
+ ImmutableBitSet.range(0, aggregate.getGroupSet().cardinality());
+ if (!groupKeys.contains(rCols)) {
+ return false;
+ }
+
+ if (aggregate.indicator) {
+ // If grouping sets are used, the filter can be pushed if
+ // the columns referenced in the predicate are present in
+ // all the grouping sets.
+ for (ImmutableBitSet groupingSet: aggregate.getGroupSets()) {
+ if (!groupingSet.contains(rCols)) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
}
// End FilterAggregateTransposeRule.java
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/92f32e8d/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 8a62e89..3739af5 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -250,6 +250,14 @@ public class RelOptRulesTest extends RelOptTestBase {
}
/** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-799">[CALCITE-799],
+ * Incorrect result for {@code HAVING count(*) > 1}</a>. */
+ @Test public void testPushFilterPastAggThree() {
+ checkPlanning(FilterAggregateTransposeRule.INSTANCE,
+ "select deptno from emp group by deptno having count(*) > 1");
+ }
+
+ /** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-448">[CALCITE-448],
* FilterIntoJoinRule creates filters containing invalid RexInputRef</a>. */
@Test public void testPushFilterPastProject() {
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/92f32e8d/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 1baaac4..eb9ed66 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -3753,4 +3753,27 @@ LogicalAggregate(group=[{}], EXPR$0=[COUNT()])
]]>
</Resource>
</TestCase>
+ <TestCase name="testPushFilterPastAggThree">
+ <Resource name="sql">
+ <![CDATA[select deptno from emp group by deptno having count(*) > 1]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalProject(DEPTNO=[$0])
+ LogicalFilter(condition=[>($1, 1)])
+ LogicalAggregate(group=[{0}], agg#0=[COUNT()])
+ LogicalProject(DEPTNO=[$7])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalProject(DEPTNO=[$0])
+ LogicalFilter(condition=[>($1, 1)])
+ LogicalAggregate(group=[{0}], agg#0=[COUNT()])
+ LogicalProject(DEPTNO=[$7])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
</Root>
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/92f32e8d/core/src/test/resources/sql/agg.oq
----------------------------------------------------------------------
diff --git a/core/src/test/resources/sql/agg.oq b/core/src/test/resources/sql/agg.oq
index d81fa9e..aeecc04 100644
--- a/core/src/test/resources/sql/agg.oq
+++ b/core/src/test/resources/sql/agg.oq
@@ -800,4 +800,44 @@ order by 1, 2;
!ok
+# [CALCITE-799] Incorrect result for "HAVING count(*) > 1"
+select d.deptno, min(e.empid) as empid
+from (values (100, 'Bill', 1),
+ (200, 'Eric', 1),
+ (150, 'Sebastian', 3)) as e(empid, name, deptno)
+join (values (1, 'LeaderShip'),
+ (2, 'TestGroup'),
+ (3, 'Development')) as d(deptno, name)
+on e.deptno = d.deptno
+group by d.deptno
+having count(*) > 1;
++--------+-------+
+| DEPTNO | EMPID |
++--------+-------+
+| 1 | 100 |
++--------+-------+
+(1 row)
+
+!ok
+
+# Same, using USING (combining [CALCITE-799] and [CALCITE-801])
+select d.deptno, min(e.empid) as empid
+from (values (100, 'Bill', 1),
+ (200, 'Eric', 1),
+ (150, 'Sebastian', 3)) as e(empid, name, deptno)
+join (values (1, 'LeaderShip'),
+ (2, 'TestGroup'),
+ (3, 'Development')) as d(deptno, name)
+using (deptno)
+group by d.deptno
+having count(*) > 1;
++--------+-------+
+| DEPTNO | EMPID |
++--------+-------+
+| 1 | 100 |
++--------+-------+
+(1 row)
+
+!ok
+
# End agg.oq