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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
