remove group-by v1

clintropolis · Aug 18, 2023 · 6e93044 · 6e93044
1 parent b97cc45
commit 6e93044
Show file tree

Hide file tree

Showing 115 changed files with 837 additions and 4,115 deletions.
diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/GroupByTypeInterfaceBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/GroupByTypeInterfaceBenchmark.java
@@ -52,13 +52,10 @@
 import org.apache.druid.query.dimension.DefaultDimensionSpec;
 import org.apache.druid.query.groupby.GroupByQuery;
 import org.apache.druid.query.groupby.GroupByQueryConfig;
-import org.apache.druid.query.groupby.GroupByQueryEngine;
 import org.apache.druid.query.groupby.GroupByQueryQueryToolChest;
 import org.apache.druid.query.groupby.GroupByQueryRunnerFactory;
+import org.apache.druid.query.groupby.GroupingEngine;
 import org.apache.druid.query.groupby.ResultRow;
-import org.apache.druid.query.groupby.strategy.GroupByStrategySelector;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV1;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV2;
 import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
 import org.apache.druid.query.spec.QuerySegmentSpec;
 import org.apache.druid.segment.IndexIO;
@@ -346,11 +343,6 @@ public void setup() throws IOException
     );
     final GroupByQueryConfig config = new GroupByQueryConfig()
     {
-      @Override
-      public String getDefaultStrategy()
-      {
-        return defaultStrategy;
-      }
 
       @Override
       public int getBufferGrouperInitialBuckets()
@@ -385,27 +377,19 @@ public String getFormatString()
     };
 
     final Supplier<GroupByQueryConfig> configSupplier = Suppliers.ofInstance(config);
-    final GroupByStrategySelector strategySelector = new GroupByStrategySelector(
+    final GroupingEngine groupingEngine = new GroupingEngine(
+        druidProcessingConfig,
         configSupplier,
-        new GroupByStrategyV1(
-            configSupplier,
-            new GroupByQueryEngine(configSupplier, bufferPool),
-            QueryBenchmarkUtil.NOOP_QUERYWATCHER
-        ),
-        new GroupByStrategyV2(
-            druidProcessingConfig,
-            configSupplier,
-            bufferPool,
-            mergePool,
-            TestHelper.makeJsonMapper(),
-            new ObjectMapper(new SmileFactory()),
-            QueryBenchmarkUtil.NOOP_QUERYWATCHER
-        )
+        bufferPool,
+        mergePool,
+        TestHelper.makeJsonMapper(),
+        new ObjectMapper(new SmileFactory()),
+        QueryBenchmarkUtil.NOOP_QUERYWATCHER
     );
 
     factory = new GroupByQueryRunnerFactory(
-        strategySelector,
-        new GroupByQueryQueryToolChest(strategySelector)
+        groupingEngine,
+        new GroupByQueryQueryToolChest(groupingEngine)
     );
   }
 

diff --git a/...marks/src/test/java/org/apache/druid/benchmark/query/CachingClusteredClientBenchmark.java b/...marks/src/test/java/org/apache/druid/benchmark/query/CachingClusteredClientBenchmark.java
@@ -77,14 +77,11 @@
 import org.apache.druid.query.expression.TestExprMacroTable;
 import org.apache.druid.query.groupby.GroupByQuery;
 import org.apache.druid.query.groupby.GroupByQueryConfig;
-import org.apache.druid.query.groupby.GroupByQueryEngine;
 import org.apache.druid.query.groupby.GroupByQueryQueryToolChest;
 import org.apache.druid.query.groupby.GroupByQueryRunnerFactory;
 import org.apache.druid.query.groupby.GroupByQueryRunnerTest;
+import org.apache.druid.query.groupby.GroupingEngine;
 import org.apache.druid.query.groupby.ResultRow;
-import org.apache.druid.query.groupby.strategy.GroupByStrategySelector;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV1;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV2;
 import org.apache.druid.query.planning.DataSourceAnalysis;
 import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
 import org.apache.druid.query.spec.QuerySegmentSpec;
@@ -287,11 +284,6 @@ public int getNumThreads()
                     GroupByQueryRunnerTest.DEFAULT_MAPPER,
                     new GroupByQueryConfig()
                     {
-                      @Override
-                      public String getDefaultStrategy()
-                      {
-                        return GroupByStrategySelector.STRATEGY_V2;
-                      }
                     },
                     processingConfig
                 )
@@ -364,25 +356,17 @@ private static GroupByQueryRunnerFactory makeGroupByQueryRunnerFactory(
         bufferSupplier,
         processingConfig.getNumMergeBuffers()
     );
-    final GroupByStrategySelector strategySelector = new GroupByStrategySelector(
+    final GroupingEngine groupingEngine = new GroupingEngine(
+        processingConfig,
         configSupplier,
-        new GroupByStrategyV1(
-            configSupplier,
-            new GroupByQueryEngine(configSupplier, bufferPool),
-            QueryRunnerTestHelper.NOOP_QUERYWATCHER
-        ),
-        new GroupByStrategyV2(
-            processingConfig,
-            configSupplier,
-            bufferPool,
-            mergeBufferPool,
-            mapper,
-            mapper,
-            QueryRunnerTestHelper.NOOP_QUERYWATCHER
-        )
+        bufferPool,
+        mergeBufferPool,
+        mapper,
+        mapper,
+        QueryRunnerTestHelper.NOOP_QUERYWATCHER
     );
-    final GroupByQueryQueryToolChest toolChest = new GroupByQueryQueryToolChest(strategySelector);
-    return new GroupByQueryRunnerFactory(strategySelector, toolChest);
+    final GroupByQueryQueryToolChest toolChest = new GroupByQueryQueryToolChest(groupingEngine);
+    return new GroupByQueryRunnerFactory(groupingEngine, toolChest);
   }
 
   @TearDown(Level.Trial)

diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/query/GroupByBenchmark.java
@@ -63,15 +63,12 @@
 import org.apache.druid.query.filter.BoundDimFilter;
 import org.apache.druid.query.groupby.GroupByQuery;
 import org.apache.druid.query.groupby.GroupByQueryConfig;
-import org.apache.druid.query.groupby.GroupByQueryEngine;
 import org.apache.druid.query.groupby.GroupByQueryQueryToolChest;
 import org.apache.druid.query.groupby.GroupByQueryRunnerFactory;
+import org.apache.druid.query.groupby.GroupingEngine;
 import org.apache.druid.query.groupby.ResultRow;
 import org.apache.druid.query.groupby.orderby.DefaultLimitSpec;
 import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
-import org.apache.druid.query.groupby.strategy.GroupByStrategySelector;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV1;
-import org.apache.druid.query.groupby.strategy.GroupByStrategyV2;
 import org.apache.druid.query.ordering.StringComparators;
 import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
 import org.apache.druid.query.spec.QuerySegmentSpec;
@@ -139,9 +136,6 @@ public class GroupByBenchmark
   @Param({"basic.A", "basic.nested"})
   private String schemaAndQuery;
 
-  @Param({"v1", "v2"})
-  private String defaultStrategy;
-
   @Param({"all", "day"})
   private String queryGranularity;
 
@@ -461,11 +455,6 @@ public void setup()
     );
     final GroupByQueryConfig config = new GroupByQueryConfig()
     {
-      @Override
-      public String getDefaultStrategy()
-      {
-        return defaultStrategy;
-      }
 
       @Override
       public int getBufferGrouperInitialBuckets()
@@ -500,27 +489,19 @@ public String getFormatString()
     };
 
     final Supplier<GroupByQueryConfig> configSupplier = Suppliers.ofInstance(config);
-    final GroupByStrategySelector strategySelector = new GroupByStrategySelector(
+    final GroupingEngine groupingEngine = new GroupingEngine(
+        druidProcessingConfig,
         configSupplier,
-        new GroupByStrategyV1(
-            configSupplier,
-            new GroupByQueryEngine(configSupplier, bufferPool),
-            QueryBenchmarkUtil.NOOP_QUERYWATCHER
-        ),
-        new GroupByStrategyV2(
-            druidProcessingConfig,
-            configSupplier,
-            bufferPool,
-            mergePool,
-            TestHelper.makeJsonMapper(),
-            new ObjectMapper(new SmileFactory()),
-            QueryBenchmarkUtil.NOOP_QUERYWATCHER
-        )
+        bufferPool,
+        mergePool,
+        TestHelper.makeJsonMapper(),
+        new ObjectMapper(new SmileFactory()),
+        QueryBenchmarkUtil.NOOP_QUERYWATCHER
     );
 
     factory = new GroupByQueryRunnerFactory(
-        strategySelector,
-        new GroupByQueryQueryToolChest(strategySelector)
+        groupingEngine,
+        new GroupByQueryQueryToolChest(groupingEngine)
     );
   }
 

diff --git a/...java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalAggregatorFactoryBase.java b/...java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalAggregatorFactoryBase.java
@@ -128,9 +128,6 @@ public Comparator<CompressedBigDecimal> getComparator()
   @Override
   public abstract AggregateCombiner<CompressedBigDecimal> makeAggregateCombiner();
 
-  @Override
-  public abstract List<AggregatorFactory> getRequiredColumns();
-
   @Override
   public abstract String toString();
 

diff --git a/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMaxAggregatorFactory.java b/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMaxAggregatorFactory.java
@@ -28,9 +28,6 @@
 import org.apache.druid.segment.ColumnSelectorFactory;
 import org.apache.druid.segment.ColumnValueSelector;
 
-import java.util.Collections;
-import java.util.List;
-
 public class CompressedBigDecimalMaxAggregatorFactory extends CompressedBigDecimalAggregatorFactoryBase
 {
   private static final byte CACHE_TYPE_ID = 0x37;
@@ -113,18 +110,6 @@ public AggregateCombiner<CompressedBigDecimal> makeAggregateCombiner()
     return new CompressedBigDecimalMaxAggregateCombiner();
   }
 
-  @Override
-  public List<AggregatorFactory> getRequiredColumns()
-  {
-    return Collections.singletonList(new CompressedBigDecimalMaxAggregatorFactory(
-        fieldName,
-        fieldName,
-        size,
-        scale,
-        strictNumberParsing
-    ));
-  }
-
   @Override
   public String toString()
   {

diff --git a/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMinAggregatorFactory.java b/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMinAggregatorFactory.java
@@ -29,8 +29,6 @@
 import org.apache.druid.segment.ColumnValueSelector;
 
 import javax.annotation.Nonnull;
-import java.util.Collections;
-import java.util.List;
 
 
 /**
@@ -118,18 +116,6 @@ public AggregateCombiner<CompressedBigDecimal> makeAggregateCombiner()
     return new CompressedBigDecimalMinAggregateCombiner();
   }
 
-  @Override
-  public List<AggregatorFactory> getRequiredColumns()
-  {
-    return Collections.singletonList(new CompressedBigDecimalMinAggregatorFactory(
-        name,
-        fieldName,
-        size,
-        scale,
-        strictNumberParsing
-    ));
-  }
-
   @Override
   public String toString()
   {

diff --git a/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSumAggregatorFactory.java b/.../java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSumAggregatorFactory.java
@@ -30,8 +30,6 @@
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import java.util.Collections;
-import java.util.List;
 
 public class CompressedBigDecimalSumAggregatorFactory extends CompressedBigDecimalAggregatorFactoryBase
 {
@@ -118,18 +116,6 @@ public AggregateCombiner<CompressedBigDecimal> makeAggregateCombiner()
     return new CompressedBigDecimalSumAggregateCombiner();
   }
 
-  @Override
-  public List<AggregatorFactory> getRequiredColumns()
-  {
-    return Collections.singletonList(new CompressedBigDecimalSumAggregatorFactory(
-        name,
-        fieldName,
-        size,
-        scale,
-        strictNumberParsing
-    ));
-  }
-
   @Override
   public String toString()
   {

diff --git a/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMaxFactoryTest.java b/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMaxFactoryTest.java
@@ -26,7 +26,6 @@
 
 import java.io.IOException;
 import java.math.BigDecimal;
-import java.util.Arrays;
 
 
 public class CompressedBigDecimalMaxFactoryTest extends CompressedBigDecimalFactoryTestBase
@@ -50,10 +49,6 @@ public void testCompressedBigDecimalMaxAggregatorFactory()
     Assert.assertEquals("5", aggregatorFactory.deserialize(new BigDecimal(5)).toString());
     Assert.assertEquals("5.0", aggregatorFactory.deserialize(5d).toString());
     Assert.assertEquals("5", aggregatorFactory.deserialize("5").toString());
-    Assert.assertEquals(
-        "[CompressedBigDecimalMaxAggregatorFactory{name='fieldName', type='COMPLEX<compressedBigDecimal>', fieldName='fieldName', requiredFields='[fieldName]', size='9', scale='0', strictNumberParsing='false'}]",
-        Arrays.toString(aggregatorFactory.getRequiredColumns().toArray())
-    );
     Assert.assertNull(aggregatorFactory.combine(null, null));
     Assert.assertEquals("4", aggregatorFactory.combine(new BigDecimal(4), null).toString());
     Assert.assertEquals("4", aggregatorFactory.combine(null, new BigDecimal(4)).toString());

diff --git a/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMinFactoryTest.java b/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalMinFactoryTest.java
@@ -26,7 +26,6 @@
 
 import java.io.IOException;
 import java.math.BigDecimal;
-import java.util.Arrays;
 
 
 public class CompressedBigDecimalMinFactoryTest extends CompressedBigDecimalFactoryTestBase
@@ -50,10 +49,7 @@ public void testCompressedBigDecimalMinAggregatorFactory()
     Assert.assertEquals("5", aggregatorFactory.deserialize(new BigDecimal(5)).toString());
     Assert.assertEquals("5.0", aggregatorFactory.deserialize(5d).toString());
     Assert.assertEquals("5", aggregatorFactory.deserialize("5").toString());
-    Assert.assertEquals(
-        "[CompressedBigDecimalMinAggregatorFactory{name='name', type='COMPLEX<compressedBigDecimal>', fieldName='fieldName', requiredFields='[fieldName]', size='9', scale='0', strictNumberParsing='false'}]",
-        Arrays.toString(aggregatorFactory.getRequiredColumns().toArray())
-    );
+
     // default is to initialize to
     Assert.assertNull(aggregatorFactory.combine(null, null));
     Assert.assertEquals("4", aggregatorFactory.combine(new BigDecimal(4), null).toString());

diff --git a/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSumFactoryTest.java b/...c/test/java/org/apache/druid/compressedbigdecimal/CompressedBigDecimalSumFactoryTest.java
@@ -26,7 +26,6 @@
 
 import java.io.IOException;
 import java.math.BigDecimal;
-import java.util.Arrays;
 
 /**
  * test CompressedBigDecimalSumFactory and various aggregators and combiner produced
@@ -52,10 +51,7 @@ public void testCompressedBigDecimalAggregatorFactory()
     Assert.assertEquals("5", aggregatorFactory.deserialize(new BigDecimal(5)).toString());
     Assert.assertEquals("5.0", aggregatorFactory.deserialize(5d).toString());
     Assert.assertEquals("5", aggregatorFactory.deserialize("5").toString());
-    Assert.assertEquals(
-        "[CompressedBigDecimalSumAggregatorFactory{name='name', type='COMPLEX<compressedBigDecimal>', fieldName='fieldName', requiredFields='[fieldName]', size='9', scale='0', strictNumberParsing='false'}]",
-        Arrays.toString(aggregatorFactory.getRequiredColumns().toArray())
-    );
+
     Assert.assertEquals("0", aggregatorFactory.combine(null, null).toString());
     Assert.assertEquals("4", aggregatorFactory.combine(new BigDecimal(4), null).toString());
     Assert.assertEquals("4", aggregatorFactory.combine(null, new BigDecimal(4)).toString());

diff --git a/...java/org/apache/druid/query/aggregation/distinctcount/DistinctCountAggregatorFactory.java b/...java/org/apache/druid/query/aggregation/distinctcount/DistinctCountAggregatorFactory.java
@@ -141,14 +141,6 @@ public AggregatorFactory getCombiningFactory()
     return new LongSumAggregatorFactory(name, name);
   }
 
-  @Override
-  public List<AggregatorFactory> getRequiredColumns()
-  {
-    return Collections.singletonList(
-        new DistinctCountAggregatorFactory(fieldName, fieldName, bitMapFactory)
-    );
-  }
-
   @Override
   public Object deserialize(Object object)
   {

diff --git a/...apache/druid/query/aggregation/momentsketch/aggregator/MomentSketchAggregatorFactory.java b/...apache/druid/query/aggregation/momentsketch/aggregator/MomentSketchAggregatorFactory.java
@@ -167,19 +167,6 @@ public AggregatorFactory getMergingFactory(AggregatorFactory other) throws Aggre
     }
   }
 
-  @Override
-  public List<AggregatorFactory> getRequiredColumns()
-  {
-    return Collections.singletonList(
-        new MomentSketchAggregatorFactory(
-            fieldName,
-            fieldName,
-            k,
-            compress
-        )
-    );
-  }
-
   private MomentSketchWrapper deserializeFromByteArray(byte[] bytes)
   {
     return MomentSketchWrapper.fromByteArray(bytes);