From c69361b92fbdc78c5e37d9d625ed6a4cbde7514e Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 8 Dec 2020 12:05:14 +0530 Subject: [PATCH 1/4] Fix post-aggregator computation --- .../groupby/strategy/GroupByStrategyV2.java | 23 +++---------------- .../druid/sql/calcite/CalciteQueryTest.java | 18 +++++++-------- 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java b/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java index 5e7c8b2d6bf6..8a4673d58c08 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java @@ -408,27 +408,10 @@ public Sequence processSubtotalsSpec( // Dimension spec including dimension name and output name final List subTotalDimensionSpec = new ArrayList<>(dimsInSubtotalSpec.size()); final List dimensions = query.getDimensions(); - final List newDimensions = new ArrayList<>(); - for (int i = 0; i < dimensions.size(); i++) { - DimensionSpec dimensionSpec = dimensions.get(i); + for (DimensionSpec dimensionSpec : dimensions) { if (dimsInSubtotalSpec.contains(dimensionSpec.getOutputName())) { - newDimensions.add( - new DefaultDimensionSpec( - dimensionSpec.getOutputName(), - dimensionSpec.getOutputName(), - dimensionSpec.getOutputType() - ) - ); subTotalDimensionSpec.add(dimensionSpec); - } else { - // Insert dummy dimension so all subtotals queries have ResultRows with the same shape. - // Use a field name that does not appear in the main query result, to assure the result will be null. - String dimName = "_" + i; - while (query.getResultRowSignature().indexOf(dimName) >= 0) { - dimName = "_" + dimName; - } - newDimensions.add(DefaultDimensionSpec.of(dimName)); } } @@ -442,8 +425,8 @@ public Sequence processSubtotalsSpec( } GroupByQuery subtotalQuery = baseSubtotalQuery - .withLimitSpec(subtotalQueryLimitSpec) - .withDimensionSpecs(newDimensions); + .withLimitSpec(subtotalQueryLimitSpec); + //.withDimensionSpecs(newDimensions); final GroupByRowProcessor.ResultSupplier resultSupplierOneFinal = resultSupplierOne; if (Utils.isPrefix(subtotalSpec, queryDimNames)) { 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 0b94b4ca2730..09a5d02e78f3 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 @@ -12159,23 +12159,23 @@ public void testGroupingAggregatorWithPostAggregator() throws Exception List resultList; if (NullHandling.sqlCompatible()) { resultList = ImmutableList.of( - new Object[]{NULL_STRING, 2L, 0L, "INDIVIDUAL"}, - new Object[]{"", 1L, 0L, "INDIVIDUAL"}, - new Object[]{"a", 2L, 0L, "INDIVIDUAL"}, - new Object[]{"abc", 1L, 0L, "INDIVIDUAL"}, + new Object[]{NULL_STRING, 2L, 0L, NULL_STRING}, + new Object[]{"", 1L, 0L, ""}, + new Object[]{"a", 2L, 0L, "a"}, + new Object[]{"abc", 1L, 0L, "abc"}, new Object[]{NULL_STRING, 6L, 1L, "ALL"} ); } else { resultList = ImmutableList.of( - new Object[]{"", 3L, 0L, "INDIVIDUAL"}, - new Object[]{"a", 2L, 0L, "INDIVIDUAL"}, - new Object[]{"abc", 1L, 0L, "INDIVIDUAL"}, + new Object[]{"", 3L, 0L, ""}, + new Object[]{"a", 2L, 0L, "a"}, + new Object[]{"abc", 1L, 0L, "abc"}, new Object[]{NULL_STRING, 6L, 1L, "ALL"} ); } testQuery( "SELECT dim2, SUM(cnt), GROUPING(dim2), \n" - + "CASE WHEN GROUPING(dim2) = 1 THEN 'ALL' ELSE 'INDIVIDUAL' END\n" + + "CASE WHEN GROUPING(dim2) = 1 THEN 'ALL' ELSE dim2 END\n" + "FROM druid.foo\n" + "GROUP BY GROUPING SETS ( (dim2), () )", ImmutableList.of( @@ -12200,7 +12200,7 @@ public void testGroupingAggregatorWithPostAggregator() throws Exception ) .setPostAggregatorSpecs(Collections.singletonList(new ExpressionPostAggregator( "p0", - "case_searched((\"a1\" == 1),'ALL','INDIVIDUAL')", + "case_searched((\"a1\" == 1),'ALL',\"d0\")", null, ExprMacroTable.nil() ))) From a7847a16944e2360956c40882c9209e6ec5252b6 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 8 Dec 2020 16:51:10 +0530 Subject: [PATCH 2/4] remove commented code --- .../apache/druid/query/groupby/strategy/GroupByStrategyV2.java | 1 - 1 file changed, 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java b/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java index 8a4673d58c08..af452eb39c62 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/strategy/GroupByStrategyV2.java @@ -426,7 +426,6 @@ public Sequence processSubtotalsSpec( GroupByQuery subtotalQuery = baseSubtotalQuery .withLimitSpec(subtotalQueryLimitSpec); - //.withDimensionSpecs(newDimensions); final GroupByRowProcessor.ResultSupplier resultSupplierOneFinal = resultSupplierOne; if (Utils.isPrefix(subtotalSpec, queryDimNames)) { From a21e287cc745caa957437e8bfee5b0ceba9ed025 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 15 Dec 2020 01:23:12 +0530 Subject: [PATCH 3/4] Fix numeric null handling --- .../query/groupby/epinephelinae/RowBasedGrouperHelper.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java index 9a87cb59b7a2..c17c0d672c49 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java @@ -712,13 +712,13 @@ public InputRawSupplierColumnSelectorStrategy makeColumnSelectorStrategy( return new StringInputRawSupplierColumnSelectorStrategy(); case LONG: return (InputRawSupplierColumnSelectorStrategy) - columnSelector -> columnSelector::getLong; + columnSelector -> () -> columnSelector.isNull() ? null : columnSelector.getLong(); case FLOAT: return (InputRawSupplierColumnSelectorStrategy) - columnSelector -> columnSelector::getFloat; + columnSelector -> () -> columnSelector.isNull() ? null : columnSelector.getFloat(); case DOUBLE: return (InputRawSupplierColumnSelectorStrategy) - columnSelector -> columnSelector::getDouble; + columnSelector -> () -> columnSelector.isNull() ? null : columnSelector.getDouble(); default: throw new IAE("Cannot create query type helper from invalid type [%s]", type); } From dbd6526ebef123b947d00207a697ddd5b927ddbc Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 15 Dec 2020 13:43:20 +0530 Subject: [PATCH 4/4] Add test when subquery returns null long --- .../query/groupby/GroupByQueryRunnerTest.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java index 751e58ab93a6..df1251b44b4d 100644 --- a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryRunnerTest.java @@ -93,6 +93,7 @@ import org.apache.druid.query.extraction.JavaScriptExtractionFn; import org.apache.druid.query.extraction.MapLookupExtractor; import org.apache.druid.query.extraction.RegexDimExtractionFn; +import org.apache.druid.query.extraction.SearchQuerySpecDimExtractionFn; import org.apache.druid.query.extraction.StringFormatExtractionFn; import org.apache.druid.query.extraction.StrlenExtractionFn; import org.apache.druid.query.extraction.TimeFormatExtractionFn; @@ -9563,6 +9564,92 @@ public void testGroupByNestedWithInnerQueryNumerics() TestHelper.assertExpectedObjects(expectedResults, results, "numerics"); } + @Test + public void testGroupByNestedWithInnerQueryOutputNullNumerics() + { + cannotVectorize(); + + if (config.getDefaultStrategy().equals(GroupByStrategySelector.STRATEGY_V1)) { + expectedException.expect(UnsupportedOperationException.class); + expectedException.expectMessage("GroupBy v1 only supports dimensions with an outputType of STRING."); + } + + // Following extractionFn will generate null value for one kind of quality + ExtractionFn extractionFn = new SearchQuerySpecDimExtractionFn(new ContainsSearchQuerySpec("1200", false)); + GroupByQuery subquery = makeQueryBuilder() + .setDataSource(QueryRunnerTestHelper.DATA_SOURCE) + .setQuerySegmentSpec(QueryRunnerTestHelper.FIRST_TO_THIRD) + .setDimensions( + new DefaultDimensionSpec("quality", "alias"), + new ExtractionDimensionSpec("qualityLong", "ql_alias", ValueType.LONG, extractionFn), + new ExtractionDimensionSpec("qualityFloat", "qf_alias", ValueType.FLOAT, extractionFn), + new ExtractionDimensionSpec("qualityDouble", "qd_alias", ValueType.DOUBLE, extractionFn) + ) + .setDimFilter( + new InDimFilter( + "quality", + Arrays.asList("entertainment", "business"), + null + ) + ).setAggregatorSpecs(QueryRunnerTestHelper.ROWS_COUNT, new LongSumAggregatorFactory("idx", "index")) + .setGranularity(QueryRunnerTestHelper.DAY_GRAN) + .build(); + + GroupByQuery outerQuery = makeQueryBuilder() + .setDataSource(subquery) + .setQuerySegmentSpec(QueryRunnerTestHelper.FIRST_TO_THIRD) + .setDimensions( + new DefaultDimensionSpec("ql_alias", "quallong", ValueType.LONG), + new DefaultDimensionSpec("qf_alias", "qualfloat", ValueType.FLOAT), + new DefaultDimensionSpec("qd_alias", "qualdouble", ValueType.DOUBLE) + ) + .setAggregatorSpecs( + new LongSumAggregatorFactory("ql_alias_sum", "ql_alias"), + new DoubleSumAggregatorFactory("qf_alias_sum", "qf_alias"), + new DoubleSumAggregatorFactory("qd_alias_sum", "qd_alias") + ) + .setGranularity(QueryRunnerTestHelper.ALL_GRAN) + .build(); + + List expectedResults = Arrays.asList( + makeRow( + outerQuery, + "2011-04-01", + "quallong", + NullHandling.defaultLongValue(), + "qualfloat", + NullHandling.defaultFloatValue(), + "qualdouble", + NullHandling.defaultDoubleValue(), + "ql_alias_sum", + NullHandling.defaultLongValue(), + "qf_alias_sum", + NullHandling.defaultFloatValue(), + "qd_alias_sum", + NullHandling.defaultDoubleValue() + ), + makeRow( + outerQuery, + "2011-04-01", + "quallong", + 1200L, + "qualfloat", + 12000.0, + "qualdouble", + 12000.0, + "ql_alias_sum", + 2400L, + "qf_alias_sum", + 24000.0, + "qd_alias_sum", + 24000.0 + ) + ); + + Iterable results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, outerQuery); + TestHelper.assertExpectedObjects(expectedResults, results, "numerics"); + } + @Test public void testGroupByNestedWithInnerQueryNumericsWithLongTime() {