diff --git a/extensions-core/stats/src/test/java/io/druid/query/aggregation/variance/VarianceGroupByQueryTest.java b/extensions-core/stats/src/test/java/io/druid/query/aggregation/variance/VarianceGroupByQueryTest.java index 131b84b52fb4..d290a1faa0e6 100644 --- a/extensions-core/stats/src/test/java/io/druid/query/aggregation/variance/VarianceGroupByQueryTest.java +++ b/extensions-core/stats/src/test/java/io/druid/query/aggregation/variance/VarianceGroupByQueryTest.java @@ -61,6 +61,7 @@ public class VarianceGroupByQueryTest private final QueryRunner runner; private final GroupByQueryRunnerFactory factory; private final String testName; + private final boolean compatibilityMode; @Parameterized.Parameters(name="{0}") public static Collection constructorFeeder() throws IOException @@ -68,12 +69,18 @@ public static Collection constructorFeeder() throws IOException return GroupByQueryRunnerTest.constructorFeeder(); } - public VarianceGroupByQueryTest(String testName, GroupByQueryConfig config, GroupByQueryRunnerFactory factory, QueryRunner runner) + public VarianceGroupByQueryTest( + String testName, + GroupByQueryConfig config, + GroupByQueryRunnerFactory factory, + QueryRunner runner, + boolean compatibilityMode) { this.testName = testName; this.config = config; this.factory = factory; this.runner = factory.mergeRunners(MoreExecutors.sameThreadExecutor(), ImmutableList.>of(runner)); + this.compatibilityMode = compatibilityMode; } @Test diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java index 9184ad386893..709141544612 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -281,7 +281,7 @@ public boolean getContextSortByDimsFirst() @Override public Ordering getResultOrdering() { - final Ordering rowOrdering = getRowOrdering(false); + final Ordering rowOrdering = getRowOrdering(false, false); return Ordering.from( new Comparator() @@ -300,11 +300,11 @@ public int compare(Object lhs, Object rhs) ); } - public Ordering getRowOrdering(final boolean granular) + public Ordering getRowOrdering(final boolean granular, final boolean compatibilityMode) { final boolean sortByDimsFirst = getContextSortByDimsFirst(); - final Comparator timeComparator = getTimeComparator(granular); + final Comparator timeComparator = getTimeComparator(granular, compatibilityMode); if (timeComparator == null) { return Ordering.from( @@ -353,10 +353,24 @@ public int compare(Row lhs, Row rhs) } } - private Comparator getTimeComparator(boolean granular) + private Comparator getTimeComparator(boolean granular, boolean compatibilityMode) { if (Granularities.ALL.equals(granularity)) { - return null; + if (compatibilityMode) { + return new Comparator() + { + @Override + public int compare(Row lhs, Row rhs) + { + return Longs.compare( + granularity.bucketStart(lhs.getTimestamp()).getMillis(), + granularity.bucketStart(rhs.getTimestamp()).getMillis() + ); + } + }; + } else { + return null; + } } else if (granular) { return new Comparator() { diff --git a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategy.java b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategy.java index 5d140fae8267..28ebb687c1b0 100644 --- a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategy.java +++ b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategy.java @@ -53,6 +53,14 @@ public interface GroupByStrategy */ boolean isCacheable(boolean willMergeRunners); + /** + * Indicates this strategy is compatible with GroupByStrategyV1 or not. + * + * @param compatibilityMode indicates whether to operate in compatibility mode or not + * @return true if this strategy is compatible with GroupByStrategyV1, otherwise false. + */ + boolean isInCompatibilityMode(boolean compatibilityMode); + /** * Decorate a runner with an interval chunking decorator. */ diff --git a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV1.java b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV1.java index 544e6c9ce1cf..95066e6846bb 100644 --- a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV1.java +++ b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV1.java @@ -91,6 +91,12 @@ public boolean isCacheable(boolean willMergeRunners) return true; } + @Override + public boolean isInCompatibilityMode(boolean compatibilityMode) + { + return false; + } + @Override public QueryRunner createIntervalChunkingRunner( final IntervalChunkingQueryRunnerDecorator decorator, diff --git a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV2.java b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV2.java index e4dc798d17c8..88a93512efba 100644 --- a/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV2.java +++ b/processing/src/main/java/io/druid/query/groupby/strategy/GroupByStrategyV2.java @@ -179,6 +179,12 @@ public boolean isCacheable(boolean willMergeRunners) return willMergeRunners; } + @Override + public boolean isInCompatibilityMode(boolean compatibilityMode) + { + return compatibilityMode; + } + @Override public QueryRunner createIntervalChunkingRunner( final IntervalChunkingQueryRunnerDecorator decorator, @@ -209,7 +215,8 @@ public Sequence mergeResults( @Override protected Ordering makeOrdering(Query queryParam) { - return ((GroupByQuery) queryParam).getRowOrdering(true); + return ((GroupByQuery) queryParam).getRowOrdering(true, + isInCompatibilityMode(query.getContextValue(GroupByQueryConfig.CTX_KEY_STRATEGY) == null ? true : false)); } @Override diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index d0a69ee4546e..a244820692d7 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -151,6 +151,7 @@ public class GroupByQueryRunnerTest { public static final ObjectMapper DEFAULT_MAPPER = new DefaultObjectMapper(new SmileFactory()); + public static final List COMPATIBILITY_MODES = ImmutableList.of(true, false); public static final DruidProcessingConfig DEFAULT_PROCESSING_CONFIG = new DruidProcessingConfig() { @Override @@ -183,6 +184,7 @@ public int getNumThreads() private GroupByQueryRunnerFactory factory; private GroupByQueryConfig config; private final String testName; + private final boolean compatibilityMode; @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -380,12 +382,15 @@ public static Collection constructorFeeder() throws IOException for (GroupByQueryConfig config : testConfigs()) { final GroupByQueryRunnerFactory factory = makeQueryRunnerFactory(config); for (QueryRunner runner : QueryRunnerTestHelper.makeQueryRunners(factory)) { - final String testName = String.format( - "config=%s, runner=%s", - config.toString(), - runner.toString() - ); - constructors.add(new Object[]{testName, config, factory, runner}); + for (boolean compatibilityMode : COMPATIBILITY_MODES) { + final String testName = String.format( + "config=%s, runner=%s, compatibilityMode=%s", + config.toString(), + runner.toString(), + compatibilityMode + ); + constructors.add(new Object[] { testName, config, factory, runner, compatibilityMode }); + } } } @@ -393,13 +398,18 @@ public static Collection constructorFeeder() throws IOException } public GroupByQueryRunnerTest( - String testName, GroupByQueryConfig config, GroupByQueryRunnerFactory factory, QueryRunner runner + String testName, + GroupByQueryConfig config, + GroupByQueryRunnerFactory factory, + QueryRunner runner, + boolean compatibilityMode ) { this.testName = testName; this.config = config; this.factory = factory; this.runner = factory.mergeRunners(MoreExecutors.sameThreadExecutor(), ImmutableList.>of(runner)); + this.compatibilityMode = compatibilityMode; } @Test