From 75f3066c55708db642554d39de94ab506f2be087 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Sun, 16 Jun 2024 22:04:23 +0530 Subject: [PATCH 1/3] correct window output col name --- .../sql/VarianceSqlAggregatorTest.java | 27 +++++++++++++++++++ .../druid/sql/calcite/rel/Windowing.java | 4 ++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java index 7c63d63aab8d..50ab9eaece90 100644 --- a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java +++ b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java @@ -20,6 +20,7 @@ package org.apache.druid.query.aggregation.variance.sql; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.inject.Injector; import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.InputRow; @@ -32,6 +33,7 @@ import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.Druids; +import org.apache.druid.query.QueryContexts; import org.apache.druid.query.QueryRunnerFactoryConglomerate; import org.apache.druid.query.aggregation.CountAggregatorFactory; import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory; @@ -61,6 +63,7 @@ import org.apache.druid.sql.calcite.SqlTestFrameworkConfig; import org.apache.druid.sql.calcite.TempDirProducer; import org.apache.druid.sql.calcite.filtration.Filtration; +import org.apache.druid.sql.calcite.planner.PlannerContext; import org.apache.druid.sql.calcite.util.CalciteTests; import org.apache.druid.sql.calcite.util.SqlTestFramework.StandardComponentSupplier; import org.apache.druid.sql.calcite.util.TestDataBuilder; @@ -698,4 +701,28 @@ public void testVarianceAggAsInput() expectedResults ); } + + @Test + public void testOverWindow() + { + testBuilder() + .sql( + "select dim4, dim5, mod(m1, 3), var_pop(mod(m1, 3)) over (partition by dim4 order by dim5) c\n" + + "from numfoo\n" + + "group by dim4, dim5, mod(m1, 3)") + .queryContext(ImmutableMap.of( + PlannerContext.CTX_ENABLE_WINDOW_FNS, true, + QueryContexts.ENABLE_DEBUG, true, + QueryContexts.WINDOWING_STRICT_VALIDATION, false + )) + .expectedResults(ImmutableList.of( + new Object[]{"a", "aa", 1.0D, 0.0D}, + new Object[]{"a", "ab", 2.0D, 0.25D}, + new Object[]{"a", "ba", 0.0D, 0.6666666666666666D}, + new Object[]{"b", "aa", 2.0D, 0.0D}, + new Object[]{"b", "ab", 0.0D, 1.0D}, + new Object[]{"b", "ad", 1.0D, 0.6666666666666666D} + )) + .run(); + } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java index 4f0f0eda21b9..0e06a74569fc 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java @@ -172,7 +172,9 @@ public static Windowing fromCalciteStuff( throw new CannotBuildQueryException(window, aggregateCall); } - aggregations.add(Iterables.getOnlyElement(aggregation.getAggregatorFactories())); + AggregatorFactory aggregatorFactory = Iterables.getOnlyElement(aggregation.getAggregatorFactories()); + windowOutputColumns.set(windowOutputColumns.size() - 1, aggregatorFactory.getName()); + aggregations.add(aggregatorFactory); } else { processors.add( maker.make( From a5ebefda3fb944c64b8a0e404ef91ec1534f0f19 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Thu, 20 Jun 2024 22:46:40 +0530 Subject: [PATCH 2/3] correct agg names --- ...CompressedBigDecimalSqlAggregatorBase.java | 2 +- ...ressedBigDecimalSqlAggregatorTestBase.java | 26 +++++------ .../TDigestGenerateSketchSqlAggregator.java | 5 +- .../sql/TDigestSketchSqlAggregatorTest.java | 16 +++---- .../sql/DoublesSketchObjectSqlAggregator.java | 5 +- .../sql/DoublesSketchSqlAggregatorTest.java | 46 +++++++++---------- .../bloom/sql/BloomFilterSqlAggregator.java | 3 +- .../sql/BloomFilterSqlAggregatorTest.java | 24 +++++----- .../sql/BaseVarianceSqlAggregator.java | 13 ++++-- .../sql/VarianceSqlAggregatorTest.java | 32 ++++++------- .../druid/sql/calcite/rel/Windowing.java | 4 +- 11 files changed, 88 insertions(+), 88 deletions(-) diff --git a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java index 33c010b58cc8..16ee758243a3 100644 --- a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java +++ b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java @@ -130,7 +130,7 @@ public Aggregation toDruidAggregation( // create the factory AggregatorFactory aggregatorFactory = factoryCreator.create( - StringUtils.format("%s:agg", name), + name, sumColumnName, size, scale, diff --git a/extensions-contrib/compressed-bigdecimal/src/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorTestBase.java b/extensions-contrib/compressed-bigdecimal/src/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorTestBase.java index 671aa9aa084d..5227700bb7cc 100644 --- a/extensions-contrib/compressed-bigdecimal/src/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorTestBase.java +++ b/extensions-contrib/compressed-bigdecimal/src/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorTestBase.java @@ -165,9 +165,9 @@ protected void testCompressedBigDecimalAggWithNumberParseHelper( .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators( - factoryCreator.create("a0:agg", "m1", 9, 9, false), - factoryCreator.create("a1:agg", "m2", 9, 9, false), - factoryCreator.create("a2:agg", "dim1", 9, 9, false) + factoryCreator.create("a0", "m1", 9, 9, false), + factoryCreator.create("a1", "m2", 9, 9, false), + factoryCreator.create("a2", "dim1", 9, 9, false) ) .context(QUERY_CONTEXT_DEFAULT) @@ -190,7 +190,7 @@ protected void testCompressedBigDecimalAggWithStrictNumberParseHelper( .dataSource(CalciteTests.DATASOURCE1) .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) - .aggregators(factoryCreator.create("a0:agg", "dim1", 9, 9, true)) + .aggregators(factoryCreator.create("a0", "dim1", 9, 9, true)) .context(QUERY_CONTEXT_DEFAULT) .build() ), @@ -218,9 +218,9 @@ public void testCompressedBigDecimalAggDefaultNumberParseAndCustomSizeAndScaleHe .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators( - factoryCreator.create("a0:agg", "m1", 9, 3, false), - factoryCreator.create("a1:agg", "m2", 9, 3, false), - factoryCreator.create("a2:agg", "dim1", 9, 3, false) + factoryCreator.create("a0", "m1", 9, 3, false), + factoryCreator.create("a1", "m2", 9, 3, false), + factoryCreator.create("a2", "dim1", 9, 3, false) ) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -249,9 +249,9 @@ public void testCompressedBigDecimalAggDefaultScaleHelper( .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators( - factoryCreator.create("a0:agg", "m1", 9, 9, false), - factoryCreator.create("a1:agg", "m2", 9, 9, false), - factoryCreator.create("a2:agg", "dim1", 9, 9, false) + factoryCreator.create("a0", "m1", 9, 9, false), + factoryCreator.create("a1", "m2", 9, 9, false), + factoryCreator.create("a2", "dim1", 9, 9, false) ) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -275,9 +275,9 @@ public void testCompressedBigDecimalAggDefaultSizeAndScaleHelper( .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators( - factoryCreator.create("a0:agg", "m1", 6, 9, false), - factoryCreator.create("a1:agg", "m2", 6, 9, false), - factoryCreator.create("a2:agg", "dim1", 6, 9, false) + factoryCreator.create("a0", "m1", 6, 9, false), + factoryCreator.create("a1", "m2", 6, 9, false), + factoryCreator.create("a2", "dim1", 6, 9, false) ) .context(QUERY_CONTEXT_DEFAULT) .build() diff --git a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java index 1777ce5c5449..aa4edb2c4be4 100644 --- a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java +++ b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java @@ -79,7 +79,6 @@ public Aggregation toDruidAggregation( } final AggregatorFactory aggregatorFactory; - final String aggName = StringUtils.format("%s:agg", name); Integer compression = TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION; if (aggregateCall.getArgList().size() > 1) { @@ -116,7 +115,7 @@ public Aggregation toDruidAggregation( // No existing match found. Create a new one. if (input.isDirectColumnAccess()) { aggregatorFactory = new TDigestSketchAggregatorFactory( - aggName, + name, input.getDirectColumn(), compression ); @@ -125,7 +124,7 @@ public Aggregation toDruidAggregation( input, ColumnType.FLOAT ); - aggregatorFactory = new TDigestSketchAggregatorFactory(aggName, virtualColumnName, compression); + aggregatorFactory = new TDigestSketchAggregatorFactory(name, virtualColumnName, compression); } return Aggregation.create(aggregatorFactory); diff --git a/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java b/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java index c4913667cb06..2a53ef0369b6 100644 --- a/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java +++ b/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java @@ -135,7 +135,7 @@ public void testComputingSketchOnNumericValues() .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators(ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "m1", 200) + new TDigestSketchAggregatorFactory("a0", "m1", 200) )) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -205,7 +205,7 @@ public void testComputingSketchOnNumericValuesWithCastedCompressionParameter() .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators(ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "m1", 200) + new TDigestSketchAggregatorFactory("a0", "m1", 200) )) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -242,7 +242,7 @@ public void testComputingSketchOnCastedString() ) ) .aggregators(ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "v0", 200) + new TDigestSketchAggregatorFactory("a0", "v0", 200) )) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -275,7 +275,7 @@ public void testDefaultCompressionForTDigestGenerateSketchAgg() .intervals(new MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity()))) .granularity(Granularities.ALL) .aggregators(ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION) + new TDigestSketchAggregatorFactory("a0", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION) )) .context(QUERY_CONTEXT_DEFAULT) .build() @@ -357,7 +357,7 @@ public void testGeneratingSketchAndComputingQuantileOnFly() .setDimensions(new DefaultDimensionSpec("dim1", "d0")) .setAggregatorSpecs( ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "m1", 200) + new TDigestSketchAggregatorFactory("a0", "m1", 200) ) ) .setContext(QUERY_CONTEXT_DEFAULT) @@ -368,7 +368,7 @@ public void testGeneratingSketchAndComputingQuantileOnFly() .setGranularity(Granularities.ALL) .setAggregatorSpecs( ImmutableList.of( - new TDigestSketchAggregatorFactory("_a0:agg", "a0:agg", 100) + new TDigestSketchAggregatorFactory("_a0:agg", "a0", 100) ) ) .setPostAggregatorSpecs( @@ -534,7 +534,7 @@ public void testEmptyTimeseriesResults() .filters(numericEquality("dim2", 0L, ColumnType.LONG)) .granularity(Granularities.ALL) .aggregators(ImmutableList.of( - new TDigestSketchAggregatorFactory("a0:agg", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION), + new TDigestSketchAggregatorFactory("a0", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION), new TDigestSketchAggregatorFactory("a1:agg", "qsketch_m1", 100) )) .postAggregators( @@ -571,7 +571,7 @@ public void testGroupByAggregatorDefaultValues() .setAggregatorSpecs( aggregators( new FilteredAggregatorFactory( - new TDigestSketchAggregatorFactory("a0:agg", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION), + new TDigestSketchAggregatorFactory("a0", "m1", TDigestSketchAggregatorFactory.DEFAULT_COMPRESSION), equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java index 15b15b0dc21e..543aea49c1ab 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java @@ -89,7 +89,6 @@ public Aggregation toDruidAggregation( } final AggregatorFactory aggregatorFactory; - final String histogramName = StringUtils.format("%s:agg", name); final int k; if (aggregateCall.getArgList().size() >= 2) { @@ -108,7 +107,7 @@ public Aggregation toDruidAggregation( // No existing match found. Create a new one. if (input.isDirectColumnAccess()) { aggregatorFactory = new DoublesSketchAggregatorFactory( - histogramName, + name, input.getDirectColumn(), k, DoublesSketchApproxQuantileSqlAggregator.getMaxStreamLengthFromQueryContext(plannerContext.queryContext()), @@ -120,7 +119,7 @@ public Aggregation toDruidAggregation( ColumnType.FLOAT ); aggregatorFactory = new DoublesSketchAggregatorFactory( - histogramName, + name, virtualColumnName, k, DoublesSketchApproxQuantileSqlAggregator.getMaxStreamLengthFromQueryContext(plannerContext.queryContext()), diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java index bce6a306c800..9122e1ecc7e8 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java @@ -539,8 +539,8 @@ public void testDoublesSketchPostAggs() .aggregators(ImmutableList.of( new LongSumAggregatorFactory("a0", "cnt"), new DoublesSketchAggregatorFactory("a1:agg", "cnt", 128), - new DoublesSketchAggregatorFactory("a2:agg", "cnt", 128, null, false), - new DoublesSketchAggregatorFactory("a3:agg", "v0", 128, null, false) + new DoublesSketchAggregatorFactory("a2", "cnt", 128, null, false), + new DoublesSketchAggregatorFactory("a3", "v0", 128, null, false) )) .postAggregators( new DoublesSketchToQuantilePostAggregator( @@ -557,7 +557,7 @@ public void testDoublesSketchPostAggs() "p2", new FieldAccessPostAggregator( "p1", - "a2:agg" + "a2" ), 0.5f ), @@ -570,7 +570,7 @@ public void testDoublesSketchPostAggs() "p5", new FieldAccessPostAggregator( "p4", - "a3:agg" + "a3" ), 0.5f ), @@ -583,7 +583,7 @@ public void testDoublesSketchPostAggs() "p8", new FieldAccessPostAggregator( "p7", - "a2:agg" + "a2" ), 0.5f ), @@ -592,7 +592,7 @@ public void testDoublesSketchPostAggs() "p11", new FieldAccessPostAggregator( "p10", - "a2:agg" + "a2" ), new double[]{0.5d, 0.8d} ), @@ -600,7 +600,7 @@ public void testDoublesSketchPostAggs() "p13", new FieldAccessPostAggregator( "p12", - "a2:agg" + "a2" ), new double[]{0.5d, 0.8d} ), @@ -608,7 +608,7 @@ public void testDoublesSketchPostAggs() "p15", new FieldAccessPostAggregator( "p14", - "a2:agg" + "a2" ), new double[]{0.2d, 0.6d}, null @@ -617,7 +617,7 @@ public void testDoublesSketchPostAggs() "p17", new FieldAccessPostAggregator( "p16", - "a2:agg" + "a2" ), 3.0d ), @@ -625,7 +625,7 @@ public void testDoublesSketchPostAggs() "p19", new FieldAccessPostAggregator( "p18", - "a2:agg" + "a2" ), new double[]{0.2d, 0.6d} ), @@ -633,7 +633,7 @@ public void testDoublesSketchPostAggs() "p21", new FieldAccessPostAggregator( "p20", - "a2:agg" + "a2" ) ), expressionPostAgg( @@ -697,24 +697,24 @@ public void testDoublesSketchPostAggsPostSort() .granularity(Granularities.ALL) .aggregators( ImmutableList.of( - new DoublesSketchAggregatorFactory("a0:agg", "m1", 128, null, false) + new DoublesSketchAggregatorFactory("a0", "m1", 128, null, false) ) ) .postAggregators( ImmutableList.of( new DoublesSketchToQuantilePostAggregator( "p1", - new FieldAccessPostAggregator("p0", "a0:agg"), + new FieldAccessPostAggregator("p0", "a0"), 0.5 ), new DoublesSketchToQuantilePostAggregator( "s1", - new FieldAccessPostAggregator("s0", "a0:agg"), + new FieldAccessPostAggregator("s0", "a0"), 0.5 ), new DoublesSketchToQuantilePostAggregator( "s3", - new FieldAccessPostAggregator("s2", "a0:agg"), + new FieldAccessPostAggregator("s2", "a0"), 0.9800000190734863 ) ) @@ -750,8 +750,8 @@ public void testEmptyTimeseriesResults() .aggregators(ImmutableList.of( new DoublesSketchAggregatorFactory("a0:agg", "m1", null), new DoublesSketchAggregatorFactory("a1:agg", "qsketch_m1", null), - new DoublesSketchAggregatorFactory("a2:agg", "m1", null, null, false), - new DoublesSketchAggregatorFactory("a3:agg", "qsketch_m1", null, null, false) + new DoublesSketchAggregatorFactory("a2", "m1", null, null, false), + new DoublesSketchAggregatorFactory("a3", "qsketch_m1", null, null, false) )) .postAggregators( new DoublesSketchToQuantilePostAggregator("a0", makeFieldAccessPostAgg("a0:agg"), 0.01f), @@ -797,8 +797,8 @@ public void testEmptyTimeseriesResultsWithFinalizeSketches() .aggregators(ImmutableList.of( new DoublesSketchAggregatorFactory("a0:agg", "m1", null), new DoublesSketchAggregatorFactory("a1:agg", "qsketch_m1", null), - new DoublesSketchAggregatorFactory("a2:agg", "m1", null, null, true), - new DoublesSketchAggregatorFactory("a3:agg", "qsketch_m1", null, null, true) + new DoublesSketchAggregatorFactory("a2", "m1", null, null, true), + new DoublesSketchAggregatorFactory("a3", "qsketch_m1", null, null, true) )) .postAggregators( new DoublesSketchToQuantilePostAggregator("a0", makeFieldAccessPostAgg("a0:agg"), 0.01f), @@ -848,11 +848,11 @@ public void testGroupByAggregatorDefaultValues() equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new DoublesSketchAggregatorFactory("a2:agg", "m1", null, null, false), + new DoublesSketchAggregatorFactory("a2", "m1", null, null, false), equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new DoublesSketchAggregatorFactory("a3:agg", "qsketch_m1", null, null, false), + new DoublesSketchAggregatorFactory("a3", "qsketch_m1", null, null, false), equality("dim1", "nonexistent", ColumnType.STRING) ) ) @@ -919,11 +919,11 @@ public void testGroupByAggregatorDefaultValuesWithFinalizeSketches() equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new DoublesSketchAggregatorFactory("a2:agg", "m1", null, null, true), + new DoublesSketchAggregatorFactory("a2", "m1", null, null, true), equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new DoublesSketchAggregatorFactory("a3:agg", "qsketch_m1", null, null, true), + new DoublesSketchAggregatorFactory("a3", "qsketch_m1", null, null, true), equality("dim1", "nonexistent", ColumnType.STRING) ) ) diff --git a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregator.java b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregator.java index 5beb6c642326..209fe3500f40 100644 --- a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregator.java +++ b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregator.java @@ -81,7 +81,6 @@ public Aggregation toDruidAggregation( } final AggregatorFactory aggregatorFactory; - final String aggName = StringUtils.format("%s:agg", name); final RexNode maxNumEntriesOperand = inputAccessor.getField(aggregateCall.getArgList().get(1)); if (!maxNumEntriesOperand.isA(SqlKind.LITERAL)) { @@ -157,7 +156,7 @@ public Aggregation toDruidAggregation( } aggregatorFactory = new BloomFilterAggregatorFactory( - aggName, + name, spec, maxNumEntries ); diff --git a/extensions-core/druid-bloom-filter/src/test/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregatorTest.java b/extensions-core/druid-bloom-filter/src/test/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregatorTest.java index cfb4209cb3bf..7de8ef8280cc 100644 --- a/extensions-core/druid-bloom-filter/src/test/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregatorTest.java +++ b/extensions-core/druid-bloom-filter/src/test/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregatorTest.java @@ -148,7 +148,7 @@ public void testBloomFilterAgg() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("dim1", "a0:dim1"), TEST_NUM_ENTRIES ) @@ -207,12 +207,12 @@ public void testBloomFilterTwoAggs() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("dim1", "a0:dim1"), TEST_NUM_ENTRIES ), new BloomFilterAggregatorFactory( - "a1:agg", + "a1", new DefaultDimensionSpec("dim2", "a1:dim2"), TEST_NUM_ENTRIES ) @@ -258,7 +258,7 @@ public void testBloomFilterAggExtractionFn() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new ExtractionDimensionSpec( "dim1", "a0:dim1", @@ -308,7 +308,7 @@ public void testBloomFilterAggLong() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("l1", "a0:l1", ColumnType.LONG), TEST_NUM_ENTRIES ) @@ -361,7 +361,7 @@ public void testBloomFilterAggLongVirtualColumn() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("v0", "a0:v0"), TEST_NUM_ENTRIES ) @@ -415,7 +415,7 @@ public void testBloomFilterAggFloatVirtualColumn() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("v0", "a0:v0"), TEST_NUM_ENTRIES ) @@ -469,7 +469,7 @@ public void testBloomFilterAggDoubleVirtualColumn() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("v0", "a0:v0"), TEST_NUM_ENTRIES ) @@ -508,12 +508,12 @@ public void testEmptyTimeseriesResults() throws Exception .aggregators( ImmutableList.of( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("dim1", "a0:dim1"), TEST_NUM_ENTRIES ), new BloomFilterAggregatorFactory( - "a1:agg", + "a1", new DefaultDimensionSpec("l1", "a1:l1", ColumnType.LONG), TEST_NUM_ENTRIES ) @@ -559,7 +559,7 @@ public void testGroupByAggregatorDefaultValues() throws Exception aggregators( new FilteredAggregatorFactory( new BloomFilterAggregatorFactory( - "a0:agg", + "a0", new DefaultDimensionSpec("dim1", "a0:dim1"), TEST_NUM_ENTRIES ), @@ -567,7 +567,7 @@ public void testGroupByAggregatorDefaultValues() throws Exception ), new FilteredAggregatorFactory( new BloomFilterAggregatorFactory( - "a1:agg", + "a1", new DefaultDimensionSpec("l1", "a1:l1", ColumnType.LONG), TEST_NUM_ENTRIES ), diff --git a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/sql/BaseVarianceSqlAggregator.java b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/sql/BaseVarianceSqlAggregator.java index b2ed565d6276..7621fae4bf76 100644 --- a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/sql/BaseVarianceSqlAggregator.java +++ b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/sql/BaseVarianceSqlAggregator.java @@ -97,8 +97,15 @@ public Aggregation toDruidAggregation( final RelDataType dataType = inputOperand.getType(); final ColumnType inputType = Calcites.getColumnTypeForRelDataType(dataType); final DimensionSpec dimensionSpec; - final String aggName = StringUtils.format("%s:agg", name); final SqlAggFunction func = calciteFunction(); + boolean needsPostAggregator = false; + String aggName = name; + if (func.getName().equals(STDDEV_NAME) + || func.getName().equals(SqlKind.STDDEV_POP.name()) + || func.getName().equals(SqlKind.STDDEV_SAMP.name())) { + needsPostAggregator = true; + aggName = StringUtils.format("%s:agg", name); + } final String estimator; final String inputTypeName; PostAggregator postAggregator = null; @@ -136,9 +143,7 @@ public Aggregation toDruidAggregation( inputTypeName ); - if (func.getName().equals(STDDEV_NAME) - || func.getName().equals(SqlKind.STDDEV_POP.name()) - || func.getName().equals(SqlKind.STDDEV_SAMP.name())) { + if (needsPostAggregator) { postAggregator = new StandardDeviationPostAggregator( name, aggregatorFactory.getName(), diff --git a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java index 50ab9eaece90..f3e6406ecf38 100644 --- a/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java +++ b/extensions-core/stats/src/test/java/org/apache/druid/query/aggregation/variance/sql/VarianceSqlAggregatorTest.java @@ -202,9 +202,9 @@ public void testVarPop() .granularity(Granularities.ALL) .aggregators( ImmutableList.of( - new VarianceAggregatorFactory("a0:agg", "d1", "population", "double"), - new VarianceAggregatorFactory("a1:agg", "f1", "population", "float"), - new VarianceAggregatorFactory("a2:agg", "l1", "population", "long") + new VarianceAggregatorFactory("a0", "d1", "population", "double"), + new VarianceAggregatorFactory("a1", "f1", "population", "float"), + new VarianceAggregatorFactory("a2", "l1", "population", "long") ) ) .context(BaseCalciteQueryTest.QUERY_CONTEXT_DEFAULT) @@ -250,9 +250,9 @@ public void testVarSamp() .granularity(Granularities.ALL) .aggregators( ImmutableList.of( - new VarianceAggregatorFactory("a0:agg", "d1", "sample", "double"), - new VarianceAggregatorFactory("a1:agg", "f1", "sample", "float"), - new VarianceAggregatorFactory("a2:agg", "l1", "sample", "long") + new VarianceAggregatorFactory("a0", "d1", "sample", "double"), + new VarianceAggregatorFactory("a1", "f1", "sample", "float"), + new VarianceAggregatorFactory("a2", "l1", "sample", "long") ) ) .context(BaseCalciteQueryTest.QUERY_CONTEXT_DEFAULT) @@ -457,14 +457,14 @@ public void testVarianceOrderBy() .setGranularity(Granularities.ALL) .setDimensions(new DefaultDimensionSpec("dim2", "_d0")) .setAggregatorSpecs( - new VarianceAggregatorFactory("a0:agg", "f1", "sample", "float") + new VarianceAggregatorFactory("a0", "f1", "sample", "float") ) .setLimitSpec( DefaultLimitSpec .builder() .orderBy( new OrderByColumnSpec( - "a0:agg", + "a0", OrderByColumnSpec.Direction.DESCENDING, StringComparators.NUMERIC ) @@ -501,7 +501,7 @@ public void testVariancesOnCastedString() new VarianceAggregatorFactory("a0:agg", "v0", "population", "double"), new VarianceAggregatorFactory("a1:agg", "v0", "sample", "double"), new VarianceAggregatorFactory("a2:agg", "v0", "sample", "double"), - new VarianceAggregatorFactory("a3:agg", "v0", "sample", "double") + new VarianceAggregatorFactory("a3", "v0", "sample", "double") ) .postAggregators( new StandardDeviationPostAggregator("a0", "a0:agg", "population"), @@ -544,11 +544,11 @@ public void testEmptyTimeseriesResults() new VarianceAggregatorFactory("a0:agg", "d1", "population", "double"), new VarianceAggregatorFactory("a1:agg", "d1", "sample", "double"), new VarianceAggregatorFactory("a2:agg", "d1", "sample", "double"), - new VarianceAggregatorFactory("a3:agg", "d1", "sample", "double"), + new VarianceAggregatorFactory("a3", "d1", "sample", "double"), new VarianceAggregatorFactory("a4:agg", "l1", "population", "long"), new VarianceAggregatorFactory("a5:agg", "l1", "sample", "long"), new VarianceAggregatorFactory("a6:agg", "l1", "sample", "long"), - new VarianceAggregatorFactory("a7:agg", "l1", "sample", "long") + new VarianceAggregatorFactory("a7", "l1", "sample", "long") ) .postAggregators( @@ -609,7 +609,7 @@ public void testGroupByAggregatorDefaultValues() equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new VarianceAggregatorFactory("a3:agg", "d1", "sample", "double"), + new VarianceAggregatorFactory("a3", "d1", "sample", "double"), equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( @@ -625,7 +625,7 @@ public void testGroupByAggregatorDefaultValues() equality("dim1", "nonexistent", ColumnType.STRING) ), new FilteredAggregatorFactory( - new VarianceAggregatorFactory("a7:agg", "l1", "sample", "long"), + new VarianceAggregatorFactory("a7", "l1", "sample", "long"), equality("dim1", "nonexistent", ColumnType.STRING) ) ) @@ -681,9 +681,9 @@ public void testVarianceAggAsInput() .granularity(Granularities.ALL) .aggregators( ImmutableList.of( - new VarianceAggregatorFactory("a0:agg", "var1", "sample", "variance"), - new VarianceAggregatorFactory("a1:agg", "var1", "population", "variance"), - new VarianceAggregatorFactory("a2:agg", "var1", "sample", "variance"), + new VarianceAggregatorFactory("a0", "var1", "sample", "variance"), + new VarianceAggregatorFactory("a1", "var1", "population", "variance"), + new VarianceAggregatorFactory("a2", "var1", "sample", "variance"), new VarianceAggregatorFactory("a3:agg", "var1", "sample", "variance"), new VarianceAggregatorFactory("a4:agg", "var1", "population", "variance"), new VarianceAggregatorFactory("a5:agg", "var1", "sample", "variance") diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java index 0e06a74569fc..4f0f0eda21b9 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/Windowing.java @@ -172,9 +172,7 @@ public static Windowing fromCalciteStuff( throw new CannotBuildQueryException(window, aggregateCall); } - AggregatorFactory aggregatorFactory = Iterables.getOnlyElement(aggregation.getAggregatorFactories()); - windowOutputColumns.set(windowOutputColumns.size() - 1, aggregatorFactory.getName()); - aggregations.add(aggregatorFactory); + aggregations.add(Iterables.getOnlyElement(aggregation.getAggregatorFactories())); } else { processors.add( maker.make( From 7f996f849e1d66734d41cb5acdc03a4a9dcbfb0d Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Fri, 21 Jun 2024 06:26:47 +0530 Subject: [PATCH 3/3] checkstyle --- .../CompressedBigDecimalSqlAggregatorBase.java | 1 - .../tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java | 1 - .../quantiles/sql/DoublesSketchObjectSqlAggregator.java | 1 - 3 files changed, 3 deletions(-) diff --git a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java index 16ee758243a3..8ce60b1a5f37 100644 --- a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java +++ b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSqlAggregatorBase.java @@ -32,7 +32,6 @@ import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.util.Optionality; -import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.sql.calcite.aggregation.Aggregation; diff --git a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java index aa4edb2c4be4..5604622755a7 100644 --- a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java +++ b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java @@ -27,7 +27,6 @@ import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.util.Optionality; -import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.aggregation.tdigestsketch.TDigestSketchAggregatorFactory; import org.apache.druid.query.aggregation.tdigestsketch.TDigestSketchUtils; diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java index 543aea49c1ab..55973c093d6f 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java @@ -27,7 +27,6 @@ import org.apache.calcite.sql.SqlFunctionCategory; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.type.SqlTypeFamily; -import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.aggregation.datasketches.SketchQueryContext; import org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchAggregatorFactory;