You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2018/08/28 17:59:34 UTC

[GitHub] fjy closed pull request #6250: SQL: Fix post-aggregator naming logic for sort-project.

fjy closed pull request #6250: SQL: Fix post-aggregator naming logic for sort-project.
URL: https://github.com/apache/incubator-druid/pull/6250
 
 
   

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/sql/src/main/java/io/druid/sql/calcite/rel/DruidQuery.java b/sql/src/main/java/io/druid/sql/calcite/rel/DruidQuery.java
index e6d5ba5e767..a65e2c1819c 100644
--- a/sql/src/main/java/io/druid/sql/calcite/rel/DruidQuery.java
+++ b/sql/src/main/java/io/druid/sql/calcite/rel/DruidQuery.java
@@ -89,7 +89,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.OptionalInt;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
 
@@ -298,7 +297,7 @@ private static Grouping computeGrouping(
           plannerContext,
           aggregateRowSignature,
           aggregateProject,
-          0
+          "p"
       );
       projectRowOrderAndPostAggregations.postAggregations.forEach(
           postAggregator -> aggregations.add(Aggregation.create(postAggregator))
@@ -337,17 +336,11 @@ private SortProject computeSortProject(
     if (sortProject == null) {
       return null;
     } else {
-      final List<PostAggregator> postAggregators = grouping.getPostAggregators();
-      final OptionalInt maybeMaxCounter = postAggregators
-          .stream()
-          .mapToInt(postAggregator -> Integer.parseInt(postAggregator.getName().substring(1)))
-          .max();
-
       final ProjectRowOrderAndPostAggregations projectRowOrderAndPostAggregations = computePostAggregations(
           plannerContext,
           sortingInputRowSignature,
           sortProject,
-          maybeMaxCounter.orElse(-1) + 1 // 0 if max doesn't exist
+          "s"
       );
 
       return new SortProject(
@@ -374,12 +367,17 @@ private static ProjectRowOrderAndPostAggregations computePostAggregations(
       PlannerContext plannerContext,
       RowSignature inputRowSignature,
       Project project,
-      int outputNameCounter
+      String basePrefix
   )
   {
     final List<String> rowOrder = new ArrayList<>();
     final List<PostAggregator> aggregations = new ArrayList<>();
+    final String outputNamePrefix = Calcites.findUnusedPrefix(
+        basePrefix,
+        new TreeSet<>(inputRowSignature.getRowOrder())
+    );
 
+    int outputNameCounter = 0;
     for (final RexNode postAggregatorRexNode : project.getChildExps()) {
       // Attempt to convert to PostAggregator.
       final DruidExpression postAggregatorExpression = Expressions.toDruidExpression(
@@ -397,7 +395,7 @@ private static ProjectRowOrderAndPostAggregations computePostAggregations(
         // (There might be a SQL-level type cast that we don't care about)
         rowOrder.add(postAggregatorExpression.getDirectColumn());
       } else {
-        final String postAggregatorName = "p" + outputNameCounter++;
+        final String postAggregatorName = outputNamePrefix + outputNameCounter++;
         final PostAggregator postAggregator = new ExpressionPostAggregator(
             postAggregatorName,
             postAggregatorExpression.getExpression(),
diff --git a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
index d8b01567ba6..f948b0c9c29 100644
--- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java
@@ -4389,6 +4389,69 @@ public void testExactCountDistinctUsingSubquery() throws Exception
     );
   }
 
+  @Test
+  public void testMinMaxAvgDailyCountWithLimit() throws Exception
+  {
+    testQuery(
+        "SELECT * FROM ("
+        + "  SELECT max(cnt), min(cnt), avg(cnt), TIME_EXTRACT(max(t), 'EPOCH') last_time, count(1) num_days FROM (\n"
+        + "      SELECT TIME_FLOOR(__time, 'P1D') AS t, count(1) cnt\n"
+        + "      FROM \"foo\"\n"
+        + "      GROUP BY 1\n"
+        + "  )"
+        + ") LIMIT 1\n",
+        ImmutableList.of(
+            GroupByQuery.builder()
+                        .setDataSource(
+                            new QueryDataSource(
+                                GroupByQuery.builder()
+                                            .setDataSource(CalciteTests.DATASOURCE1)
+                                            .setInterval(QSS(Filtration.eternity()))
+                                            .setGranularity(Granularities.ALL)
+                                            .setVirtualColumns(
+                                                EXPRESSION_VIRTUAL_COLUMN(
+                                                    "d0:v",
+                                                    "timestamp_floor(\"__time\",'P1D',null,'UTC')",
+                                                    ValueType.LONG
+                                                )
+                                            )
+                                            .setDimensions(DIMS(new DefaultDimensionSpec("d0:v", "d0", ValueType.LONG)))
+                                            .setAggregatorSpecs(AGGS(new CountAggregatorFactory("a0")))
+                                            .setContext(QUERY_CONTEXT_DEFAULT)
+                                            .build()
+                            )
+                        )
+                        .setInterval(QSS(Filtration.eternity()))
+                        .setGranularity(Granularities.ALL)
+                        .setAggregatorSpecs(AGGS(
+                            new LongMaxAggregatorFactory("_a0", "a0"),
+                            new LongMinAggregatorFactory("_a1", "a0"),
+                            new LongSumAggregatorFactory("_a2:sum", "a0"),
+                            new CountAggregatorFactory("_a2:count"),
+                            new LongMaxAggregatorFactory("_a3", "d0"),
+                            new CountAggregatorFactory("_a4")
+                        ))
+                        .setPostAggregatorSpecs(
+                            ImmutableList.of(
+                                new ArithmeticPostAggregator(
+                                    "_a2",
+                                    "quotient",
+                                    ImmutableList.of(
+                                        new FieldAccessPostAggregator(null, "_a2:sum"),
+                                        new FieldAccessPostAggregator(null, "_a2:count")
+                                    )
+                                ),
+                                EXPRESSION_POST_AGG("s0", "timestamp_extract(\"_a3\",'EPOCH','UTC')")
+                            )
+                        )
+                        .setLimit(1)
+                        .setContext(QUERY_CONTEXT_DEFAULT)
+                        .build()
+        ),
+        ImmutableList.of(new Object[]{1L, 1L, 1L, 978480000L, 6L})
+    );
+  }
+
   @Test
   public void testAvgDailyCountDistinct() throws Exception
   {
@@ -6971,7 +7034,7 @@ public void testProjectAfterSort2() throws Exception
                             AGGS(new CountAggregatorFactory("a0"), new DoubleSumAggregatorFactory("a1", "m2"))
                         )
                         .setPostAggregatorSpecs(Collections.singletonList(EXPRESSION_POST_AGG(
-                            "p0",
+                            "s0",
                             "(\"a1\" / \"a0\")"
                         )))
                         .setLimitSpec(


 

----------------------------------------------------------------
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

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org