This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 3113e7b3505 Fix grouping aggregator when one of the dimension is a
simple extraction (#15421)
3113e7b3505 is described below
commit 3113e7b3505a77d33f70b769b0e8b09c1c86cc5b
Author: Abhishek Agarwal <[email protected]>
AuthorDate: Fri Nov 24 13:15:07 2023 +0530
Fix grouping aggregator when one of the dimension is a simple extraction
(#15421)
This PR fixes an issue where the grouping aggregator wrongly assumes that a
key dimension is a virtual column and assigns a wrong name to it. This results
in a mismatch between the dimensions that grouping aggregator sees and the
dimension names that rows are aggregated on. And finally, grouping aggregator
generates wrong result.
---
.../aggregation/builtin/GroupingSqlAggregator.java | 7 +-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 91 ++++++++++++++++++++++
2 files changed, 96 insertions(+), 2 deletions(-)
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/GroupingSqlAggregator.java
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/GroupingSqlAggregator.java
index ec829df11d7..7c123dab927 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/GroupingSqlAggregator.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/GroupingSqlAggregator.java
@@ -116,10 +116,13 @@ public class GroupingSqlAggregator implements
SqlAggregator
return expression.getDirectColumn();
}
- String virtualColumn =
virtualColumnRegistry.getOrCreateVirtualColumnForExpression(
+ if (expression.isSimpleExtraction()) {
+ return expression.getSimpleExtraction().getColumn();
+ }
+
+ return virtualColumnRegistry.getOrCreateVirtualColumnForExpression(
expression,
node.getType()
);
- return virtualColumn;
}
}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 6e996e46337..eb232c6957a 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -2580,6 +2580,97 @@ public class CalciteQueryTest extends
BaseCalciteQueryTest
);
}
+ @Test
+ public void testExactCountDistinctLookup()
+ {
+ msqIncompatible();
+ final String sqlQuery = "SELECT CAST(LOOKUP(dim1, 'lookyloo') AS VARCHAR),
"
+ + "COUNT(DISTINCT foo.dim2), "
+ + "SUM(foo.cnt) FROM druid.foo "
+ + "GROUP BY 1";
+
+ // ExtractionDimensionSpec cannot be vectorized
+ cannotVectorize();
+
+ requireMergeBuffers(3);
+
+ testQuery(
+ PLANNER_CONFIG_NO_HLL.withOverrides(
+ ImmutableMap.of(
+ PlannerConfig.CTX_KEY_USE_GROUPING_SET_FOR_EXACT_DISTINCT,
+ "true"
+ )
+ ),
+ sqlQuery,
+ CalciteTests.REGULAR_USER_AUTH_RESULT,
+ ImmutableList.of(
+ GroupByQuery.builder()
+ .setDataSource(
+ new QueryDataSource(
+ GroupByQuery.builder()
+
.setDataSource(CalciteTests.DATASOURCE1)
+
.setInterval(querySegmentSpec(Filtration.eternity()))
+ .setGranularity(Granularities.ALL)
+ .setDimensions(dimensions(
+ new ExtractionDimensionSpec(
+ "dim1",
+ "d0",
+ ColumnType.STRING,
+ new
RegisteredLookupExtractionFn(
+ null,
+ "lookyloo",
+ false,
+ null,
+ null,
+ true
+ )
+ ),
+ new
DefaultDimensionSpec("dim2", "d1", ColumnType.STRING)
+ ))
+ .setAggregatorSpecs(
+ aggregators(
+ new
LongSumAggregatorFactory("a0", "cnt"),
+ new
GroupingAggregatorFactory(
+ "a1",
+ Arrays.asList("dim1",
"dim2")
+ )
+ )
+ )
+ .setSubtotalsSpec(
+ ImmutableList.of(
+ ImmutableList.of("d0",
"d1"),
+ ImmutableList.of("d0")
+ )
+ )
+ .build()
+ )
+ )
+ .setInterval(querySegmentSpec(Filtration.eternity()))
+ .setGranularity(Granularities.ALL)
+ .setDimensions(new DefaultDimensionSpec("d0", "_d0",
ColumnType.STRING))
+ .setAggregatorSpecs(aggregators(
+ new FilteredAggregatorFactory(
+ new CountAggregatorFactory("_a0"),
+ and(
+ notNull("d1"),
+ equality("a1", 0L, ColumnType.LONG)
+ )
+ ),
+ new FilteredAggregatorFactory(
+ new LongMinAggregatorFactory("_a1", "a0"),
+ equality("a1", 1L, ColumnType.LONG)
+ )
+ ))
+ .setContext(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{NullHandling.defaultStringValue(),
NullHandling.replaceWithDefault() ? 2L : 3L, 5L},
+ new Object[]{"xabc", 0L, 1L}
+ )
+ );
+ }
+
@Test
public void testHavingOnFloatSum()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
