Repository: calcite Updated Branches: refs/heads/master cf5ff72b7 -> ecc1623fb
[CALCITE-1714] Do not push query with group by on druid metrics (Slim Bouguerra) Close apache/calcite#409 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/0209b16c Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/0209b16c Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/0209b16c Branch: refs/heads/master Commit: 0209b16c0adca468c3da06ef036e3d06432ea17b Parents: cf5ff72 Author: Slim Bouguerra <[email protected]> Authored: Mon Mar 27 11:05:42 2017 +0100 Committer: Jesus Camacho Rodriguez <[email protected]> Committed: Mon Mar 27 19:53:35 2017 +0100 ---------------------------------------------------------------------- .../calcite/adapter/druid/DruidQuery.java | 3 +- .../calcite/adapter/druid/DruidRules.java | 34 +++++++++ .../org/apache/calcite/test/DruidAdapterIT.java | 76 ++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/0209b16c/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java ---------------------------------------------------------------------- diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java index 061c52c..9378e47 100644 --- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java +++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java @@ -509,7 +509,7 @@ public class DruidQuery extends AbstractRelNode implements BindableRel { final String s = fieldNames.get(groupKey); final RexNode project = projects.get(groupKey); if (project instanceof RexInputRef) { - // Reference, it could be to the timestamp column or any other dimension + // Reference could be to the timestamp or druid dimension but no druid metric final RexInputRef ref = (RexInputRef) project; final String origin = druidTable.getRowType(getCluster().getTypeFactory()) .getFieldList().get(ref.getIndex()).getName(); @@ -563,7 +563,6 @@ public class DruidQuery extends AbstractRelNode implements BindableRel { } fieldNames = builder.build(); - ImmutableList<JsonCollation> collations = null; boolean sortsMetric = false; if (collationIndexes != null) { http://git-wip-us.apache.org/repos/asf/calcite/blob/0209b16c/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java ---------------------------------------------------------------------- diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java index 2054c11..a417698 100644 --- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java +++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidRules.java @@ -364,6 +364,9 @@ public class DruidRules { for (AggregateCall aggCall : aggregate.getAggCallList()) { builder.addAll(aggCall.getArgList()); } + if (checkAggregateOnMetric(aggregate.getGroupSet(), aggregate, query)) { + return false; + } return !checkTimestampRefOnQuery(builder.build(), query.getTopNode(), query); } } @@ -398,6 +401,10 @@ public class DruidRules { return; } + if (checkAggregateOnMetric(aggregate.getGroupSet(), project, query)) { + return; + } + final RelNode newProject = project.copy(project.getTraitSet(), ImmutableList.of(Util.last(query.rels))); final DruidQuery projectDruidQuery = DruidQuery.extendQuery(query, newProject); @@ -600,6 +607,33 @@ public class DruidRules { return false; } + /** Checks whether any of the references leads to a metric column. */ + private static boolean checkAggregateOnMetric(ImmutableBitSet set, RelNode topProject, + DruidQuery query) { + if (topProject instanceof Project) { + ImmutableBitSet.Builder newSet = ImmutableBitSet.builder(); + final Project project = (Project) topProject; + for (int index : set) { + RexNode node = project.getProjects().get(index); + if (node instanceof RexInputRef) { + newSet.set(((RexInputRef) node).getIndex()); + } else if (node instanceof RexCall) { + RexCall call = (RexCall) node; + newSet.set(((RexInputRef) call.getOperands().get(0)).getIndex()); + } + } + set = newSet.build(); + } + for (int index : set) { + if (query.druidTable.metricFieldNames + .contains(query.getTopNode().getRowType().getFieldNames().get(index))) { + return true; + } + } + + return false; + } + /** * Rule to push a {@link org.apache.calcite.rel.core.Project} * past a {@link org.apache.calcite.rel.core.Filter} http://git-wip-us.apache.org/repos/asf/calcite/blob/0209b16c/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java ---------------------------------------------------------------------- diff --git a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java index 4ee32a5..312ff63 100644 --- a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java +++ b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java @@ -411,6 +411,60 @@ public class DruidAdapterIT { .explainContains(explain); } + @Test public void testGroupbyMetric() { + final String sql = "select \"store_sales\" ,\"product_id\" from \"foodmart\" " + + "where \"product_id\" = 1020" + "group by \"store_sales\" ,\"product_id\" "; + final String plan = "PLAN=EnumerableInterpreter\n BindableAggregate(group=[{0, 1}])\n" + + " DruidQuery(table=[[foodmart, foodmart]], " + + "intervals=[[1900-01-09T00:00:00.000/2992-01-10T00:00:00.000]], filter=[=($1, 1020)]," + + " projects=[[$90, $1]])\n"; + sql(sql).explainContains(plan). + queryContains( + druidChecker("{\"queryType\":\"select\",\"dataSource\":\"foodmart\"," + + "\"descending\":false," + + "\"intervals\":[\"1900-01-09T00:00:00.000/2992-01-10T00:00:00.000" + + "\"],\"filter\":{\"type\":\"selector\",\"dimension\":\"product_id\"," + + "\"value\":\"1020\"},\"dimensions\":[\"product_id\"]," + + "\"metrics\":[\"store_sales\"],\"granularity\":\"all\"," + + "\"pagingSpec\":{\"threshold\":16384,\"fromNext\":true}," + + "\"context\":{\"druid.query.fetch\":false}}") + ) + .returnsUnordered("store_sales=0.5099999904632568; product_id=1020", + "store_sales=1.0199999809265137; product_id=1020", + "store_sales=1.5299999713897705; product_id=1020", + "store_sales=2.0399999618530273; product_id=1020", + "store_sales=2.549999952316284; product_id=1020" + ); + } + + @Test public void testPushSimpleGroupBy() { + final String sql = "select \"product_id\" from \"foodmart\" where " + + "\"product_id\" = 1020 group by \"product_id\""; + final String druidQuery = "{\"queryType\":\"groupBy\",\"dataSource\":\"foodmart\"," + + "\"granularity\":\"all\",\"dimensions\":[\"product_id\"]," + + "\"limitSpec\":{\"type\":\"default\"},\"filter\":{\"type\":\"selector\"," + + "\"dimension\":\"product_id\",\"value\":\"1020\"}," + + "\"aggregations\":[{\"type\":\"longSum\",\"name\":\"dummy_agg\"," + + "\"fieldName\":\"dummy_agg\"}]," + + "\"intervals\":[\"1900-01-09T00:00:00.000/2992-01-10T00:00:00.000\"]}"; + sql(sql).queryContains(druidChecker(druidQuery)).returnsUnordered("product_id=1020"); + } + + @Test public void testComplexPushGroupBy() { + final String innerQuery = "select \"product_id\" as \"id\" from \"foodmart\" where " + + "\"product_id\" = 1020"; + final String sql = "select \"id\" from (" + innerQuery + ") group by \"id\""; + + sql(sql).returnsUnordered("id=1020") + .queryContains( + druidChecker("{\"queryType\":\"groupBy" + + "\",\"dataSource\":\"foodmart\",\"granularity\":\"all\"," + + "\"dimensions\":[\"product_id\"],\"limitSpec\":{\"type\":\"default\"}," + + "\"filter\":{\"type\":\"selector\",\"dimension\":\"product_id\"," + + "\"value\":\"1020\"},\"aggregations\":[{\"type\":\"longSum\"," + + "\"name\":\"dummy_agg\",\"fieldName\":\"dummy_agg\"}]," + + "\"intervals\":[\"1900-01-09T00:00:00.000/2992-01-10T00:00:00.000\"]}")); + } /** Test case for * <a href="https://issues.apache.org/jira/browse/CALCITE-1281";>[CALCITE-1281] * Druid adapter wrongly returns all numeric values as int or float</a>. */ @@ -928,6 +982,20 @@ public class DruidAdapterIT { .queryContains(druidChecker("'queryType':'groupBy'")); } + @Test public void testGroupByTimeAndOneMetricNotProjected() { + final String sql = + "select count(*) as \"c\", floor(\"timestamp\" to MONTH) as \"month\", floor" + + "(\"store_sales\") as sales\n" + + "from \"foodmart\"\n" + + "group by floor(\"timestamp\" to MONTH), \"state_province\", floor" + + "(\"store_sales\")\n" + + "order by \"c\" desc limit 3"; + sql(sql).returnsOrdered("c=494; month=1997-11-01 00:00:00; SALES=5.0", + "c=475; month=1997-12-01 00:00:00; SALES=5.0", + "c=468; month=1997-03-01 00:00:00; SALES=5.0" + ).queryContains(druidChecker("'queryType':'select'")); + } + @Test public void testGroupByTimeAndOneColumnNotProjected() { final String sql = "select count(*) as \"c\",\n" + " floor(\"timestamp\" to MONTH) as \"month\"\n" @@ -1420,6 +1488,14 @@ public class DruidAdapterIT { sql(sql, WIKI).queryContains(druidChecker(druidQuery)); } + @Test public void testGroupByMetricAndExtractTime() { + final String sql = "SELECT count(*), floor(\"timestamp\" to DAY), \"store_sales\" " + + "FROM \"foodmart\"\n" + + "GROUP BY \"store_sales\", floor(\"timestamp\" to DAY)\n ORDER BY \"store_sales\" DESC\n" + + "LIMIT 10\n"; + sql(sql).queryContains(druidChecker("{\"queryType\":\"select\"")); + } + } // End DruidAdapterIT.java
