From 35ae08c7b949a10bfe2bdee7bafffcced5555ac2 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 28 Feb 2017 14:19:08 -0800 Subject: [PATCH 1/4] Add default configuration for select query 'fromNext' parameter --- .../benchmark/query/SelectBenchmark.java | 10 ++- docs/content/querying/select-query.md | 11 +++ .../druid/segment/MapVirtualColumnTest.java | 10 ++- .../io/druid/query/select/PagingSpec.java | 21 +++--- .../io/druid/query/select/SelectQuery.java | 4 +- .../druid/query/select/SelectQueryConfig.java | 40 +++++++++++ .../druid/query/select/SelectQueryEngine.java | 15 +++- .../select/SelectQueryQueryToolChest.java | 17 +++-- .../aggregation/AggregationTestHelper.java | 14 +++- .../select/MultiSegmentSelectQueryTest.java | 9 ++- .../query/select/SelectQueryConfigTest.java | 58 ++++++++++++++++ .../query/select/SelectQueryRunnerTest.java | 31 ++++++--- .../query/select/SelectQuerySpecTest.java | 69 ++++++++++++++++++- .../io/druid/guice/QueryToolChestModule.java | 2 + .../client/CachingClusteredClientTest.java | 15 +++- .../druid/sql/calcite/util/CalciteTests.java | 8 ++- 16 files changed, 291 insertions(+), 43 deletions(-) create mode 100644 processing/src/main/java/io/druid/query/select/SelectQueryConfig.java create mode 100644 processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java index 37c17d7f94c7..e9b12dda1069 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java @@ -20,6 +20,8 @@ package io.druid.benchmark.query; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.io.Files; @@ -49,6 +51,7 @@ import io.druid.query.select.EventHolder; import io.druid.query.select.PagingSpec; import io.druid.query.select.SelectQuery; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryEngine; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.select.SelectQueryRunnerFactory; @@ -227,12 +230,15 @@ public void setup() throws IOException qIndexes.add(qIndex); } + final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( JSON_MAPPER, - QueryBenchmarkUtil.NoopIntervalChunkingQueryRunnerDecorator() + QueryBenchmarkUtil.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ), - new SelectQueryEngine(), + new SelectQueryEngine(selectConfigSupplier), QueryBenchmarkUtil.NOOP_QUERYWATCHER ); } diff --git a/docs/content/querying/select-query.md b/docs/content/querying/select-query.md index 274da89cc9fd..4d4c8f4c2c33 100644 --- a/docs/content/querying/select-query.md +++ b/docs/content/querying/select-query.md @@ -179,3 +179,14 @@ Note that in the second query, an offset is specified and that it is 1 greater t "pagingSpec":{"pagingIdentifiers": {}, "threshold":5, "fromNext": true} } ``` + +Note that specifying the `fromNext` option in the `pagingSpec` overrides the default value set by `druid.query.select.enableFromNextDefault` in the server configuration. See [Server configuration](#server-configuration) for more details. + + +#### Server configuration + +The following runtime properties apply to select queries: + +|Property|Description|Default| +|--------|-----------|-------| +|`druid.query.select.enableFromNextDefault`|If the `fromNext` property in a query's `pagingSpec` is left unspecified, the system will use the value of this property as the default value for `fromNext`.|true| \ No newline at end of file diff --git a/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java b/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java index b5ff60aa7225..e11e38b30e8d 100644 --- a/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java +++ b/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java @@ -19,6 +19,8 @@ package io.druid.segment; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -37,6 +39,7 @@ import io.druid.query.select.EventHolder; import io.druid.query.select.PagingSpec; import io.druid.query.select.SelectQuery; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryEngine; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.select.SelectQueryRunnerFactory; @@ -69,12 +72,15 @@ public class MapVirtualColumnTest @Parameterized.Parameters public static Iterable constructorFeeder() throws IOException { + final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + SelectQueryRunnerFactory factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ), - new SelectQueryEngine(), + new SelectQueryEngine(selectConfigSupplier), QueryRunnerTestHelper.NOOP_QUERYWATCHER ); diff --git a/processing/src/main/java/io/druid/query/select/PagingSpec.java b/processing/src/main/java/io/druid/query/select/PagingSpec.java index 274f034919be..7a2dd18fa21b 100644 --- a/processing/src/main/java/io/druid/query/select/PagingSpec.java +++ b/processing/src/main/java/io/druid/query/select/PagingSpec.java @@ -58,13 +58,13 @@ public static Map next(Map cursor, boolean des private final Map pagingIdentifiers; private final int threshold; - private final boolean fromNext; + private final Boolean fromNext; @JsonCreator public PagingSpec( @JsonProperty("pagingIdentifiers") Map pagingIdentifiers, @JsonProperty("threshold") int threshold, - @JsonProperty("fromNext") boolean fromNext + @JsonProperty("fromNext") Boolean fromNext ) { this.pagingIdentifiers = pagingIdentifiers == null ? Maps.newHashMap() : pagingIdentifiers; @@ -74,7 +74,7 @@ public PagingSpec( public PagingSpec(Map pagingIdentifiers, int threshold) { - this(pagingIdentifiers, threshold, false); + this(pagingIdentifiers, threshold, null); } @JsonProperty @@ -90,12 +90,12 @@ public int getThreshold() } @JsonProperty - public boolean isFromNext() + public Boolean isFromNext() { return fromNext; } - public byte[] getCacheKey() + public byte[] getCacheKey(boolean defaultFromNext) { final byte[][] pagingKeys = new byte[pagingIdentifiers.size()][]; final byte[][] pagingValues = new byte[pagingIdentifiers.size()][]; @@ -124,17 +124,17 @@ public byte[] getCacheKey() } queryCacheKey.put(thresholdBytes); - queryCacheKey.put(isFromNext() ? (byte) 0x01 : 0x00); + queryCacheKey.put(shouldApplyFromNext(defaultFromNext) ? (byte) 0x01 : 0x00); return queryCacheKey.array(); } - public PagingOffset getOffset(String identifier, boolean descending) + public PagingOffset getOffset(String identifier, boolean descending, boolean defaultFromNext) { Integer offset = pagingIdentifiers.get(identifier); if (offset == null) { offset = PagingOffset.toOffset(0, descending); - } else if (fromNext) { + } else if (shouldApplyFromNext(defaultFromNext)) { offset = descending ? offset - 1 : offset + 1; } return PagingOffset.of(offset, threshold); @@ -183,4 +183,9 @@ public String toString() ", fromNext=" + fromNext + '}'; } + + private boolean shouldApplyFromNext(boolean defaultFromNext) + { + return fromNext == null ? defaultFromNext : fromNext; + } } diff --git a/processing/src/main/java/io/druid/query/select/SelectQuery.java b/processing/src/main/java/io/druid/query/select/SelectQuery.java index 4c0154857c6a..8e8119fb79eb 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQuery.java +++ b/processing/src/main/java/io/druid/query/select/SelectQuery.java @@ -139,9 +139,9 @@ public VirtualColumns getVirtualColumns() return virtualColumns; } - public PagingOffset getPagingOffset(String identifier) + public PagingOffset getPagingOffset(String identifier, boolean defaultFromNext) { - return pagingSpec.getOffset(identifier, isDescending()); + return pagingSpec.getOffset(identifier, isDescending(), defaultFromNext); } public SelectQuery withQuerySegmentSpec(QuerySegmentSpec querySegmentSpec) diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java new file mode 100644 index 000000000000..78befc151aab --- /dev/null +++ b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java @@ -0,0 +1,40 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.query.select; + +import com.fasterxml.jackson.annotation.JsonProperty; + +public class SelectQueryConfig +{ + public static String CTX_KEY_ENABLE_FROM_NEXT_DEFAULT = "enableFromNextDefault"; + + @JsonProperty + private boolean enableFromNextDefault = true; + + public boolean getEnableFromNextDefault() + { + return enableFromNextDefault; + } + + public void setEnableFromNextDefault(boolean enableFromNextDefault) + { + this.enableFromNextDefault = enableFromNextDefault; + } +} diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java b/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java index 4bb3b4fe996e..260fe77cd472 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java @@ -21,9 +21,11 @@ import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.inject.Inject; import io.druid.java.util.common.IAE; import io.druid.java.util.common.ISE; import io.druid.java.util.common.guava.Sequence; @@ -158,6 +160,16 @@ public void addRowValuesToSelectResult( } } + private final Supplier configSupplier; + + @Inject + public SelectQueryEngine( + Supplier configSupplier + ) + { + this.configSupplier = configSupplier; + } + public Sequence> process(final SelectQuery query, final Segment segment) { final StorageAdapter adapter = segment.asStorageAdapter(); @@ -231,7 +243,8 @@ public Result apply(Cursor cursor) builder.addMetric(metric); } - final PagingOffset offset = query.getPagingOffset(segmentId); + final boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); + final PagingOffset offset = query.getPagingOffset(segmentId, defaultFromNext); cursor.advanceTo(offset.startDelta()); diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java index 45ab92587667..2248f38957ca 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java @@ -24,6 +24,7 @@ import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -77,15 +78,19 @@ public class SelectQueryQueryToolChest extends QueryToolChest configSupplier; @Inject - public SelectQueryQueryToolChest(ObjectMapper jsonMapper, - IntervalChunkingQueryRunnerDecorator intervalChunkingQueryRunnerDecorator) + public SelectQueryQueryToolChest( + ObjectMapper jsonMapper, + IntervalChunkingQueryRunnerDecorator intervalChunkingQueryRunnerDecorator, + Supplier configSupplier + ) { this.jsonMapper = jsonMapper; this.intervalChunkingQueryRunnerDecorator = intervalChunkingQueryRunnerDecorator; + this.configSupplier = configSupplier; } @Override @@ -197,13 +202,15 @@ public byte[] computeCacheKey(SelectQuery query) ++index; } + final boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); + final byte[] virtualColumnsCacheKey = query.getVirtualColumns().getCacheKey(); final ByteBuffer queryCacheKey = ByteBuffer .allocate( 1 + granularityBytes.length + filterBytes.length - + query.getPagingSpec().getCacheKey().length + + query.getPagingSpec().getCacheKey(defaultFromNext).length + dimensionsBytesSize + metricBytesSize + virtualColumnsCacheKey.length @@ -211,7 +218,7 @@ public byte[] computeCacheKey(SelectQuery query) .put(SELECT_QUERY) .put(granularityBytes) .put(filterBytes) - .put(query.getPagingSpec().getCacheKey()); + .put(query.getPagingSpec().getCacheKey(defaultFromNext)); for (byte[] dimensionsByte : dimensionsBytes) { queryCacheKey.put(dimensionsByte); diff --git a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java index d2a0ad976262..2c520b673805 100644 --- a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java +++ b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.databind.type.TypeFactory; import com.google.common.base.Function; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.base.Throwables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -56,6 +57,7 @@ import io.druid.query.groupby.GroupByQueryConfig; import io.druid.query.groupby.GroupByQueryRunnerFactory; import io.druid.query.groupby.GroupByQueryRunnerTest; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryEngine; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.select.SelectQueryRunnerFactory; @@ -165,17 +167,23 @@ public static final AggregationTestHelper createSelectQueryAggregationTestHelper { ObjectMapper mapper = new DefaultObjectMapper(); + Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + SelectQueryQueryToolChest toolchest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + configSupplier ); SelectQueryRunnerFactory factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + configSupplier + ), + new SelectQueryEngine( + configSupplier ), - new SelectQueryEngine(), QueryRunnerTestHelper.NOOP_QUERYWATCHER ); diff --git a/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java b/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java index d22952d725a8..c368c01acb34 100644 --- a/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java +++ b/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java @@ -19,6 +19,8 @@ package io.druid.query.select; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.io.CharSource; @@ -65,14 +67,17 @@ @RunWith(Parameterized.class) public class MultiSegmentSelectQueryTest { + private static final Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + configSupplier ); private static final QueryRunnerFactory factory = new SelectQueryRunnerFactory( toolChest, - new SelectQueryEngine(), + new SelectQueryEngine(configSupplier), QueryRunnerTestHelper.NOOP_QUERYWATCHER ); diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java new file mode 100644 index 000000000000..1950c92fd05a --- /dev/null +++ b/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java @@ -0,0 +1,58 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.query.select; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableMap; +import io.druid.jackson.DefaultObjectMapper; +import org.junit.Assert; +import org.junit.Test; + +public class SelectQueryConfigTest +{ + private final ObjectMapper MAPPER = new DefaultObjectMapper(); + + private final ImmutableMap CONFIG_MAP = ImmutableMap + .builder() + .put(SelectQueryConfig.CTX_KEY_ENABLE_FROM_NEXT_DEFAULT, "false") + .build(); + + private final ImmutableMap CONFIG_MAP2 = ImmutableMap + .builder() + .put(SelectQueryConfig.CTX_KEY_ENABLE_FROM_NEXT_DEFAULT, "true") + .build(); + + private final ImmutableMap CONFIG_MAP_EMPTY = ImmutableMap + .builder() + .build(); + + @Test + public void testSerde() + { + final SelectQueryConfig config = MAPPER.convertValue(CONFIG_MAP, SelectQueryConfig.class); + Assert.assertEquals(false, config.getEnableFromNextDefault()); + + final SelectQueryConfig config2 = MAPPER.convertValue(CONFIG_MAP2, SelectQueryConfig.class); + Assert.assertEquals(true, config2.getEnableFromNextDefault()); + + final SelectQueryConfig configEmpty = MAPPER.convertValue(CONFIG_MAP_EMPTY, SelectQueryConfig.class); + Assert.assertEquals(true, configEmpty.getEnableFromNextDefault()); + } +} diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index 76eadc50dbbd..ab1ff83932e5 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -20,6 +20,8 @@ package io.druid.query.select; import com.google.common.base.Function; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -109,9 +111,17 @@ public class SelectQueryRunnerTest ); public static final String[] V_0112_0114 = ObjectArrays.concat(V_0112, V_0113, String.class); + private static final boolean DEFAULT_FROM_NEXT = true; + private static final SelectQueryConfig config = new SelectQueryConfig(); + { + config.setEnableFromNextDefault(DEFAULT_FROM_NEXT); + } + private static final Supplier configSupplier = Suppliers.ofInstance(config); + private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + configSupplier ); @Parameterized.Parameters(name = "{0}:descending={1}") @@ -121,14 +131,13 @@ public static Iterable constructorFeeder() throws IOException QueryRunnerTestHelper.makeQueryRunners( new SelectQueryRunnerFactory( toolChest, - new SelectQueryEngine(), + new SelectQueryEngine(configSupplier), QueryRunnerTestHelper.NOOP_QUERYWATCHER ) ), Arrays.asList(false, true) ); } - private final QueryRunner runner; private final boolean descending; @@ -162,7 +171,7 @@ public void testFullOnSelect() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( toFullEvents(V_0112_0114), Lists.newArrayList("market", "quality", "qualityNumericString", "placement", "placementish", "partial_null_column", "null_column"), @@ -194,7 +203,7 @@ public void testSequentialPaging() Assert.assertEquals(offset, pagingIdentifiers.get(QueryRunnerTestHelper.segmentId).intValue()); Map next = PagingSpec.next(pagingIdentifiers, descending); - query = query.withPagingSpec(new PagingSpec(next, 3)); + query = query.withPagingSpec(new PagingSpec(next, 3, false)); } query = newTestQuery().intervals(I_0112_0114).build(); @@ -359,7 +368,7 @@ public void testSelectWithDimsAndMets() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( toEvents( new String[]{ @@ -398,7 +407,7 @@ public void testSelectPagination() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( toEvents( new String[]{ @@ -470,7 +479,7 @@ public void testFullOnSelectWithFilter() } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( events, Lists.newArrayList("quality"), @@ -524,7 +533,7 @@ public void testFullOnSelectWithFilterOnVirtualColumn() } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( events, Lists.newArrayList("quality"), @@ -579,7 +588,7 @@ public void testSelectWithFilterLookupExtractionFn () { } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( events, Lists.newArrayList(QueryRunnerTestHelper.qualityDimension), @@ -650,7 +659,7 @@ public void testFullSelectNoDimensionAndMetric() V_0112_0114 ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); List> expectedResults = toExpected( events, Lists.newArrayList("foo"), diff --git a/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java b/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java index dd103ee990a0..ce2fe57d4da8 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java @@ -63,7 +63,7 @@ public void testSerializationLegacyString() throws Exception + "{\"type\":\"default\",\"dimension\":\"quality\",\"outputName\":\"quality\",\"outputType\":\"STRING\"}]," + "\"metrics\":[\"index\"]," + "\"virtualColumns\":[]," - + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":false}," + + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":null}," + "\"context\":null}"; SelectQuery query = new SelectQuery( @@ -75,7 +75,7 @@ public void testSerializationLegacyString() throws Exception DefaultDimensionSpec.toSpec(Arrays.asList("market", "quality")), Arrays.asList("index"), null, - new PagingSpec(null, 3), + new PagingSpec(null, 3, null), null ); @@ -84,4 +84,69 @@ public void testSerializationLegacyString() throws Exception Assert.assertEquals(query, jsonMapper.readValue(actual, SelectQuery.class)); Assert.assertEquals(query, jsonMapper.readValue(legacy, SelectQuery.class)); } + + @Test + public void testPagingSpecFromNext() throws Exception + { + String baseQueryJson = + "{\"queryType\":\"select\",\"dataSource\":{\"type\":\"table\",\"name\":\"testing\"}," + + "\"intervals\":{\"type\":\"LegacySegmentSpec\",\"intervals\":[\"2011-01-12T00:00:00.000Z/2011-01-14T00:00:00.000Z\"]}," + + "\"descending\":true," + + "\"filter\":null," + + "\"granularity\":{\"type\":\"all\"}," + + "\"dimensions\":" + + "[{\"type\":\"default\",\"dimension\":\"market\",\"outputName\":\"market\",\"outputType\":\"STRING\"}," + + "{\"type\":\"default\",\"dimension\":\"quality\",\"outputName\":\"quality\",\"outputType\":\"STRING\"}]," + + "\"metrics\":[\"index\"]," + + "\"virtualColumns\":[],"; + + String withNull = + baseQueryJson + + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":null}," + + "\"context\":null}"; + + String withFalse = + baseQueryJson + + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":false}," + + "\"context\":null}"; + + String withTrue = + baseQueryJson + + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":true}," + + "\"context\":null}"; + + SelectQuery queryWithNull = new SelectQuery( + new TableDataSource(QueryRunnerTestHelper.dataSource), + new LegacySegmentSpec(new Interval("2011-01-12/2011-01-14")), + true, + null, + QueryRunnerTestHelper.allGran, + DefaultDimensionSpec.toSpec(Arrays.asList("market", "quality")), + Arrays.asList("index"), + null, + new PagingSpec(null, 3, null), + null + ); + + SelectQuery queryWithFalse = queryWithNull.withPagingSpec( + new PagingSpec(null, 3, false) + ); + + SelectQuery queryWithTrue = queryWithNull.withPagingSpec( + new PagingSpec(null, 3, true) + ); + + String actualWithNull = jsonMapper.writeValueAsString(queryWithNull); + Assert.assertEquals(withNull, actualWithNull); + + String actualWithFalse = jsonMapper.writeValueAsString(queryWithFalse); + Assert.assertEquals(withFalse, actualWithFalse); + + String actualWithTrue = jsonMapper.writeValueAsString(queryWithTrue); + Assert.assertEquals(withTrue, actualWithTrue); + + Assert.assertEquals(queryWithNull, jsonMapper.readValue(actualWithNull, SelectQuery.class)); + Assert.assertEquals(queryWithFalse, jsonMapper.readValue(actualWithFalse, SelectQuery.class)); + Assert.assertEquals(queryWithTrue, jsonMapper.readValue(actualWithTrue, SelectQuery.class)); + } } diff --git a/server/src/main/java/io/druid/guice/QueryToolChestModule.java b/server/src/main/java/io/druid/guice/QueryToolChestModule.java index 4c07cdbf0f8d..3eac45099048 100644 --- a/server/src/main/java/io/druid/guice/QueryToolChestModule.java +++ b/server/src/main/java/io/druid/guice/QueryToolChestModule.java @@ -39,6 +39,7 @@ import io.druid.query.search.search.SearchQuery; import io.druid.query.search.search.SearchQueryConfig; import io.druid.query.select.SelectQuery; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.timeboundary.TimeBoundaryQuery; import io.druid.query.timeboundary.TimeBoundaryQueryQueryToolChest; @@ -82,5 +83,6 @@ public void configure(Binder binder) JsonConfigProvider.bind(binder, "druid.query.search", SearchQueryConfig.class); JsonConfigProvider.bind(binder, "druid.query.topN", TopNQueryConfig.class); JsonConfigProvider.bind(binder, "druid.query.segmentMetadata", SegmentMetadataQueryConfig.class); + JsonConfigProvider.bind(binder, "druid.query.select", SelectQueryConfig.class); } } diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index bf403d221ef9..b92f136a034a 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -25,6 +25,8 @@ import com.google.common.base.Charsets; import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -102,6 +104,7 @@ import io.druid.query.select.EventHolder; import io.druid.query.select.PagingSpec; import io.druid.query.select.SelectQuery; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.select.SelectResultValue; import io.druid.query.spec.MultipleIntervalSegmentSpec; @@ -247,6 +250,9 @@ public class CachingClusteredClientTest private static final DateTimeZone TIMEZONE = DateTimeZone.forID("America/Los_Angeles"); private static final Granularity PT1H_TZ_GRANULARITY = new PeriodGranularity(new Period("PT1H"), null, TIMEZONE); private static final String TOP_DIM = "a_dim"; + + private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + static final QueryToolChestWarehouse WAREHOUSE = new MapQueryToolChestWarehouse( ImmutableMap., QueryToolChest>builder() .put( @@ -271,7 +277,8 @@ SearchQuery.class, new SearchQueryQueryToolChest( SelectQuery.class, new SelectQueryQueryToolChest( jsonMapper, - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ) ) .put( @@ -1328,7 +1335,8 @@ public void testSelectCaching() throws Exception client, new SelectQueryQueryToolChest( jsonMapper, - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ) ); HashMap context = new HashMap(); @@ -1404,7 +1412,8 @@ public void testSelectCachingRenamedOutputName() throws Exception client, new SelectQueryQueryToolChest( jsonMapper, - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ) ); HashMap context = new HashMap(); diff --git a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java index a89cdef5d21d..1e8124c37809 100644 --- a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java @@ -20,6 +20,7 @@ package io.druid.sql.calcite.util; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -50,6 +51,7 @@ import io.druid.query.metadata.SegmentMetadataQueryRunnerFactory; import io.druid.query.metadata.metadata.SegmentMetadataQuery; import io.druid.query.select.SelectQuery; +import io.druid.query.select.SelectQueryConfig; import io.druid.query.select.SelectQueryEngine; import io.druid.query.select.SelectQueryQueryToolChest; import io.druid.query.select.SelectQueryRunnerFactory; @@ -88,6 +90,7 @@ public class CalciteTests public static final String DATASOURCE2 = "foo2"; private static final String TIMESTAMP_COLUMN = "t"; + private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); private static final QueryRunnerFactoryConglomerate CONGLOMERATE = new DefaultQueryRunnerFactoryConglomerate( ImmutableMap., QueryRunnerFactory>builder() @@ -105,9 +108,10 @@ public class CalciteTests new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( TestHelper.getObjectMapper(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator(), + selectConfigSupplier ), - new SelectQueryEngine(), + new SelectQueryEngine(selectConfigSupplier), QueryRunnerTestHelper.NOOP_QUERYWATCHER ) ) From 8b0cc855e46a48eb0d67e8c241aacd3aeb9e0f7b Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 1 Mar 2017 13:32:17 -0800 Subject: [PATCH 2/4] PR comments --- docs/content/querying/select-query.md | 61 +++++++++++++++---- .../io/druid/query/select/PagingSpec.java | 37 ++++++----- .../io/druid/query/select/SelectQuery.java | 4 +- .../druid/query/select/SelectQueryConfig.java | 2 +- .../druid/query/select/SelectQueryEngine.java | 3 +- .../select/SelectQueryQueryToolChest.java | 6 +- .../query/select/SelectQueryConfigTest.java | 4 +- .../query/select/SelectQueryRunnerTest.java | 14 ++--- 8 files changed, 87 insertions(+), 44 deletions(-) diff --git a/docs/content/querying/select-query.md b/docs/content/querying/select-query.md index 4d4c8f4c2c33..1a6cce58dce1 100644 --- a/docs/content/querying/select-query.md +++ b/docs/content/querying/select-query.md @@ -152,41 +152,78 @@ The results above include: }, ``` -This can be used with the next query's pagingSpec: +### Result Pagination + +The PagingSpec allows the user to specify that the results of a select query should be returned as a paginated set. + +The `threshold` option controls how many rows are returned in each block of paginated results. + +To initiate a paginated query, the user should specify a PagingSpec with a `threshold` set and a blank `pagingIdentifiers` field, e.g.: + +```json +"pagingSpec":{"pagingIdentifiers": {}, "threshold":5} +``` + +When the query returns, the results will contain a `pagingIndentifers` field indicating the current pagination point in the result set (an identifier and an offset). + +To retrieve the next part of the result set, the user should issue the same select query, but replace the `pagingIdentifiers` field of the query with the `pagingIdentifiers` from the previous result. + +When an empty result set is received, all rows have been returned. + +#### fromNext Backwards Compatibility + +In older versions of Druid, when using paginated select queries, it was necessary for the user to manually increment the paging offset by 1 in each `pagingIdentifiers` before submitting the next query to retrieve the next set of results. This offset increment happens automatically in the current version of Druid by default, the user does not need to modify the `pagingIdentifiers` offset to retrieve the next result set. + +Setting the `fromNext` field of the PagingSpec to false instructs Druid to operate in the older mode where the user must manually increment the offset (or decrement for descending queries). + +For example, suppose the user issues the following initial paginated query, with `fromNext` false: ```json { "queryType": "select", "dataSource": "wikipedia", + "descending": "false", "dimensions":[], "metrics":[], "granularity": "all", "intervals": [ "2013-01-01/2013-01-02" ], - "pagingSpec":{"pagingIdentifiers": {"wikipedia_2012-12-29T00:00:00.000Z_2013-01-10T08:00:00.000Z_2013-01-10T08:13:47.830Z_v9" : 5}, "threshold":5} - + "pagingSpec":{"fromNext": "false", "pagingIdentifiers": {}, "threshold":5} } ``` -Note that in the second query, an offset is specified and that it is 1 greater than the largest offset found in the initial results. To return the next "page", this offset must be incremented by 1 (should be decremented by 1 for descending query), with each new query, but with option `fromNext` enabled, this operation is not needed. When an empty results set is received, the very last page has been returned. -`fromNext` options is in pagingSpec: +The paginated query with `fromNext` set to false returns a result set with the following `pagingIdentifiers`: ```json - { - ... - "pagingSpec":{"pagingIdentifiers": {}, "threshold":5, "fromNext": true} - } + "pagingIdentifiers" : { + "wikipedia_2012-12-29T00:00:00.000Z_2013-01-10T08:00:00.000Z_2013-01-10T08:13:47.830Z_v9" : 4 + }, ``` -Note that specifying the `fromNext` option in the `pagingSpec` overrides the default value set by `druid.query.select.enableFromNextDefault` in the server configuration. See [Server configuration](#server-configuration) for more details. +To retrieve the next result set, the next query must be sent with the paging offset (4) incremented by 1. + +```json + { + "queryType": "select", + "dataSource": "wikipedia", + "dimensions":[], + "metrics":[], + "granularity": "all", + "intervals": [ + "2013-01-01/2013-01-02" + ], + "pagingSpec":{"fromNext": "false", "pagingIdentifiers": {"wikipedia_2012-12-29T00:00:00.000Z_2013-01-10T08:00:00.000Z_2013-01-10T08:13:47.830Z_v9" : 5}, "threshold":5} + } +``` +Note that specifying the `fromNext` option in the `pagingSpec` overrides the default value set by `druid.query.select.enableFromNextDefault` in the server configuration. See [Server configuration](#server-configuration) for more details. -#### Server configuration +### Server configuration The following runtime properties apply to select queries: |Property|Description|Default| |--------|-----------|-------| -|`druid.query.select.enableFromNextDefault`|If the `fromNext` property in a query's `pagingSpec` is left unspecified, the system will use the value of this property as the default value for `fromNext`.|true| \ No newline at end of file +|`druid.query.select.enableFromNextDefault`|If the `fromNext` property in a query's `pagingSpec` is left unspecified, the system will use the value of this property as the default value for `fromNext`. This option is true by default: the option of setting `fromNext` to false by default is intended to support backwards compatibility for deployments where some users may still expect behavior from older versions of Druid.|true| \ No newline at end of file diff --git a/processing/src/main/java/io/druid/query/select/PagingSpec.java b/processing/src/main/java/io/druid/query/select/PagingSpec.java index 7a2dd18fa21b..b950a6af0133 100644 --- a/processing/src/main/java/io/druid/query/select/PagingSpec.java +++ b/processing/src/main/java/io/druid/query/select/PagingSpec.java @@ -21,8 +21,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.Maps; import com.google.common.primitives.Ints; +import com.google.inject.Inject; import io.druid.java.util.common.StringUtils; import java.nio.ByteBuffer; @@ -58,23 +61,34 @@ public static Map next(Map cursor, boolean des private final Map pagingIdentifiers; private final int threshold; - private final Boolean fromNext; + private final boolean fromNext; + private final Supplier configSupplier; @JsonCreator + @Inject public PagingSpec( @JsonProperty("pagingIdentifiers") Map pagingIdentifiers, @JsonProperty("threshold") int threshold, - @JsonProperty("fromNext") Boolean fromNext + @JsonProperty("fromNext") Boolean fromNext, + Supplier configSupplier ) { this.pagingIdentifiers = pagingIdentifiers == null ? Maps.newHashMap() : pagingIdentifiers; this.threshold = threshold; - this.fromNext = fromNext; + this.configSupplier = configSupplier; + + boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); + this.fromNext = fromNext == null ? defaultFromNext : fromNext; } public PagingSpec(Map pagingIdentifiers, int threshold) { - this(pagingIdentifiers, threshold, null); + this(pagingIdentifiers, threshold, null, Suppliers.ofInstance(new SelectQueryConfig())); + } + + public PagingSpec(Map pagingIdentifiers, int threshold, Boolean fromNext) + { + this(pagingIdentifiers, threshold, fromNext, Suppliers.ofInstance(new SelectQueryConfig())); } @JsonProperty @@ -90,12 +104,12 @@ public int getThreshold() } @JsonProperty - public Boolean isFromNext() + public boolean isFromNext() { return fromNext; } - public byte[] getCacheKey(boolean defaultFromNext) + public byte[] getCacheKey() { final byte[][] pagingKeys = new byte[pagingIdentifiers.size()][]; final byte[][] pagingValues = new byte[pagingIdentifiers.size()][]; @@ -124,17 +138,17 @@ public byte[] getCacheKey(boolean defaultFromNext) } queryCacheKey.put(thresholdBytes); - queryCacheKey.put(shouldApplyFromNext(defaultFromNext) ? (byte) 0x01 : 0x00); + queryCacheKey.put(isFromNext() ? (byte) 0x01 : 0x00); return queryCacheKey.array(); } - public PagingOffset getOffset(String identifier, boolean descending, boolean defaultFromNext) + public PagingOffset getOffset(String identifier, boolean descending) { Integer offset = pagingIdentifiers.get(identifier); if (offset == null) { offset = PagingOffset.toOffset(0, descending); - } else if (shouldApplyFromNext(defaultFromNext)) { + } else if (fromNext) { offset = descending ? offset - 1 : offset + 1; } return PagingOffset.of(offset, threshold); @@ -183,9 +197,4 @@ public String toString() ", fromNext=" + fromNext + '}'; } - - private boolean shouldApplyFromNext(boolean defaultFromNext) - { - return fromNext == null ? defaultFromNext : fromNext; - } } diff --git a/processing/src/main/java/io/druid/query/select/SelectQuery.java b/processing/src/main/java/io/druid/query/select/SelectQuery.java index 8e8119fb79eb..4c0154857c6a 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQuery.java +++ b/processing/src/main/java/io/druid/query/select/SelectQuery.java @@ -139,9 +139,9 @@ public VirtualColumns getVirtualColumns() return virtualColumns; } - public PagingOffset getPagingOffset(String identifier, boolean defaultFromNext) + public PagingOffset getPagingOffset(String identifier) { - return pagingSpec.getOffset(identifier, isDescending(), defaultFromNext); + return pagingSpec.getOffset(identifier, isDescending()); } public SelectQuery withQuerySegmentSpec(QuerySegmentSpec querySegmentSpec) diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java index 78befc151aab..18b717da32e6 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java @@ -23,7 +23,7 @@ public class SelectQueryConfig { - public static String CTX_KEY_ENABLE_FROM_NEXT_DEFAULT = "enableFromNextDefault"; + public static String ENABLE_FROM_NEXT_DEFAULT = "enableFromNextDefault"; @JsonProperty private boolean enableFromNextDefault = true; diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java b/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java index 260fe77cd472..387183902470 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryEngine.java @@ -243,8 +243,7 @@ public Result apply(Cursor cursor) builder.addMetric(metric); } - final boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); - final PagingOffset offset = query.getPagingOffset(segmentId, defaultFromNext); + final PagingOffset offset = query.getPagingOffset(segmentId); cursor.advanceTo(offset.startDelta()); diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java index 2248f38957ca..2c92ff1badb7 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java @@ -202,15 +202,13 @@ public byte[] computeCacheKey(SelectQuery query) ++index; } - final boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); - final byte[] virtualColumnsCacheKey = query.getVirtualColumns().getCacheKey(); final ByteBuffer queryCacheKey = ByteBuffer .allocate( 1 + granularityBytes.length + filterBytes.length - + query.getPagingSpec().getCacheKey(defaultFromNext).length + + query.getPagingSpec().getCacheKey().length + dimensionsBytesSize + metricBytesSize + virtualColumnsCacheKey.length @@ -218,7 +216,7 @@ public byte[] computeCacheKey(SelectQuery query) .put(SELECT_QUERY) .put(granularityBytes) .put(filterBytes) - .put(query.getPagingSpec().getCacheKey(defaultFromNext)); + .put(query.getPagingSpec().getCacheKey()); for (byte[] dimensionsByte : dimensionsBytes) { queryCacheKey.put(dimensionsByte); diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java index 1950c92fd05a..7ce42f3ad2c5 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryConfigTest.java @@ -31,12 +31,12 @@ public class SelectQueryConfigTest private final ImmutableMap CONFIG_MAP = ImmutableMap .builder() - .put(SelectQueryConfig.CTX_KEY_ENABLE_FROM_NEXT_DEFAULT, "false") + .put(SelectQueryConfig.ENABLE_FROM_NEXT_DEFAULT, "false") .build(); private final ImmutableMap CONFIG_MAP2 = ImmutableMap .builder() - .put(SelectQueryConfig.CTX_KEY_ENABLE_FROM_NEXT_DEFAULT, "true") + .put(SelectQueryConfig.ENABLE_FROM_NEXT_DEFAULT, "true") .build(); private final ImmutableMap CONFIG_MAP_EMPTY = ImmutableMap diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index ab1ff83932e5..3196b6448d02 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -171,7 +171,7 @@ public void testFullOnSelect() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( toFullEvents(V_0112_0114), Lists.newArrayList("market", "quality", "qualityNumericString", "placement", "placementish", "partial_null_column", "null_column"), @@ -368,7 +368,7 @@ public void testSelectWithDimsAndMets() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( toEvents( new String[]{ @@ -407,7 +407,7 @@ public void testSelectPagination() Lists.>newArrayList() ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( toEvents( new String[]{ @@ -479,7 +479,7 @@ public void testFullOnSelectWithFilter() } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( events, Lists.newArrayList("quality"), @@ -533,7 +533,7 @@ public void testFullOnSelectWithFilterOnVirtualColumn() } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( events, Lists.newArrayList("quality"), @@ -588,7 +588,7 @@ public void testSelectWithFilterLookupExtractionFn () { } ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( events, Lists.newArrayList(QueryRunnerTestHelper.qualityDimension), @@ -659,7 +659,7 @@ public void testFullSelectNoDimensionAndMetric() V_0112_0114 ); - PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId, DEFAULT_FROM_NEXT); + PagingOffset offset = query.getPagingOffset(QueryRunnerTestHelper.segmentId); List> expectedResults = toExpected( events, Lists.newArrayList("foo"), From f7256faa59f5286f6ede57a07c3ce910c9b0d70a Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 1 Mar 2017 14:15:48 -0800 Subject: [PATCH 3/4] Fix PagingSpec config injection --- .../benchmark/query/SelectBenchmark.java | 2 +- .../druid/segment/MapVirtualColumnTest.java | 2 +- .../io/druid/query/select/PagingSpec.java | 17 ++++------ .../druid/query/select/SelectQueryConfig.java | 11 +++++++ .../aggregation/AggregationTestHelper.java | 2 +- .../select/MultiSegmentSelectQueryTest.java | 2 +- .../query/select/SelectQueryRunnerTest.java | 2 +- .../query/select/SelectQuerySpecTest.java | 33 ++++++++++++------- .../client/CachingClusteredClientTest.java | 2 +- .../druid/sql/calcite/util/CalciteTests.java | 2 +- 10 files changed, 46 insertions(+), 29 deletions(-) diff --git a/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java b/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java index e9b12dda1069..21033adc7a72 100644 --- a/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java +++ b/benchmarks/src/main/java/io/druid/benchmark/query/SelectBenchmark.java @@ -230,7 +230,7 @@ public void setup() throws IOException qIndexes.add(qIndex); } - final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( diff --git a/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java b/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java index e11e38b30e8d..46d99cbbcc96 100644 --- a/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java +++ b/extensions-contrib/virtual-columns/src/test/java/io/druid/segment/MapVirtualColumnTest.java @@ -72,7 +72,7 @@ public class MapVirtualColumnTest @Parameterized.Parameters public static Iterable constructorFeeder() throws IOException { - final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); SelectQueryRunnerFactory factory = new SelectQueryRunnerFactory( new SelectQueryQueryToolChest( diff --git a/processing/src/main/java/io/druid/query/select/PagingSpec.java b/processing/src/main/java/io/druid/query/select/PagingSpec.java index b950a6af0133..45a7605b2b1b 100644 --- a/processing/src/main/java/io/druid/query/select/PagingSpec.java +++ b/processing/src/main/java/io/druid/query/select/PagingSpec.java @@ -19,13 +19,11 @@ package io.druid.query.select; +import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import com.google.common.collect.Maps; import com.google.common.primitives.Ints; -import com.google.inject.Inject; import io.druid.java.util.common.StringUtils; import java.nio.ByteBuffer; @@ -62,33 +60,32 @@ public static Map next(Map cursor, boolean des private final Map pagingIdentifiers; private final int threshold; private final boolean fromNext; - private final Supplier configSupplier; + private final SelectQueryConfig config; @JsonCreator - @Inject public PagingSpec( @JsonProperty("pagingIdentifiers") Map pagingIdentifiers, @JsonProperty("threshold") int threshold, @JsonProperty("fromNext") Boolean fromNext, - Supplier configSupplier + @JacksonInject SelectQueryConfig config ) { this.pagingIdentifiers = pagingIdentifiers == null ? Maps.newHashMap() : pagingIdentifiers; this.threshold = threshold; - this.configSupplier = configSupplier; + this.config = config; - boolean defaultFromNext = configSupplier.get().getEnableFromNextDefault(); + boolean defaultFromNext = config.getEnableFromNextDefault(); this.fromNext = fromNext == null ? defaultFromNext : fromNext; } public PagingSpec(Map pagingIdentifiers, int threshold) { - this(pagingIdentifiers, threshold, null, Suppliers.ofInstance(new SelectQueryConfig())); + this(pagingIdentifiers, threshold, null, new SelectQueryConfig(true)); } public PagingSpec(Map pagingIdentifiers, int threshold, Boolean fromNext) { - this(pagingIdentifiers, threshold, fromNext, Suppliers.ofInstance(new SelectQueryConfig())); + this(pagingIdentifiers, threshold, fromNext, new SelectQueryConfig(true)); } @JsonProperty diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java index 18b717da32e6..0255c8606969 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryConfig.java @@ -19,6 +19,7 @@ package io.druid.query.select; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; public class SelectQueryConfig @@ -28,6 +29,16 @@ public class SelectQueryConfig @JsonProperty private boolean enableFromNextDefault = true; + @JsonCreator + public SelectQueryConfig( + @JsonProperty("enableFromNextDefault") Boolean enableFromNextDefault + ) + { + if (enableFromNextDefault != null) { + this.enableFromNextDefault = enableFromNextDefault.booleanValue(); + } + } + public boolean getEnableFromNextDefault() { return enableFromNextDefault; diff --git a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java index 2c520b673805..af4cb05c5b86 100644 --- a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java +++ b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java @@ -167,7 +167,7 @@ public static final AggregationTestHelper createSelectQueryAggregationTestHelper { ObjectMapper mapper = new DefaultObjectMapper(); - Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); SelectQueryQueryToolChest toolchest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), diff --git a/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java b/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java index c368c01acb34..dc5e654124e1 100644 --- a/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java +++ b/processing/src/test/java/io/druid/query/select/MultiSegmentSelectQueryTest.java @@ -67,7 +67,7 @@ @RunWith(Parameterized.class) public class MultiSegmentSelectQueryTest { - private static final Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + private static final Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); private static final SelectQueryQueryToolChest toolChest = new SelectQueryQueryToolChest( new DefaultObjectMapper(), diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index 3196b6448d02..bccfda46ec44 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -112,7 +112,7 @@ public class SelectQueryRunnerTest public static final String[] V_0112_0114 = ObjectArrays.concat(V_0112, V_0113, String.class); private static final boolean DEFAULT_FROM_NEXT = true; - private static final SelectQueryConfig config = new SelectQueryConfig(); + private static final SelectQueryConfig config = new SelectQueryConfig(true); { config.setEnableFromNextDefault(DEFAULT_FROM_NEXT); } diff --git a/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java b/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java index ce2fe57d4da8..96bb12a38174 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQuerySpecTest.java @@ -19,6 +19,7 @@ package io.druid.query.select; +import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.ObjectMapper; import io.druid.jackson.DefaultObjectMapper; import io.druid.query.QueryRunnerTestHelper; @@ -35,7 +36,15 @@ */ public class SelectQuerySpecTest { - private static final ObjectMapper jsonMapper = new DefaultObjectMapper(); + private final ObjectMapper objectMapper = new DefaultObjectMapper(); + { + objectMapper.setInjectableValues( + new InjectableValues.Std().addValue( + SelectQueryConfig.class, + new SelectQueryConfig(true) + ) + ); + } @Test public void testSerializationLegacyString() throws Exception @@ -63,7 +72,7 @@ public void testSerializationLegacyString() throws Exception + "{\"type\":\"default\",\"dimension\":\"quality\",\"outputName\":\"quality\",\"outputType\":\"STRING\"}]," + "\"metrics\":[\"index\"]," + "\"virtualColumns\":[]," - + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":null}," + + "\"pagingSpec\":{\"pagingIdentifiers\":{},\"threshold\":3,\"fromNext\":true}," + "\"context\":null}"; SelectQuery query = new SelectQuery( @@ -79,10 +88,10 @@ public void testSerializationLegacyString() throws Exception null ); - String actual = jsonMapper.writeValueAsString(query); + String actual = objectMapper.writeValueAsString(query); Assert.assertEquals(current, actual); - Assert.assertEquals(query, jsonMapper.readValue(actual, SelectQuery.class)); - Assert.assertEquals(query, jsonMapper.readValue(legacy, SelectQuery.class)); + Assert.assertEquals(query, objectMapper.readValue(actual, SelectQuery.class)); + Assert.assertEquals(query, objectMapper.readValue(legacy, SelectQuery.class)); } @Test @@ -136,17 +145,17 @@ public void testPagingSpecFromNext() throws Exception new PagingSpec(null, 3, true) ); - String actualWithNull = jsonMapper.writeValueAsString(queryWithNull); - Assert.assertEquals(withNull, actualWithNull); + String actualWithNull = objectMapper.writeValueAsString(queryWithNull); + Assert.assertEquals(withTrue, actualWithNull); - String actualWithFalse = jsonMapper.writeValueAsString(queryWithFalse); + String actualWithFalse = objectMapper.writeValueAsString(queryWithFalse); Assert.assertEquals(withFalse, actualWithFalse); - String actualWithTrue = jsonMapper.writeValueAsString(queryWithTrue); + String actualWithTrue = objectMapper.writeValueAsString(queryWithTrue); Assert.assertEquals(withTrue, actualWithTrue); - Assert.assertEquals(queryWithNull, jsonMapper.readValue(actualWithNull, SelectQuery.class)); - Assert.assertEquals(queryWithFalse, jsonMapper.readValue(actualWithFalse, SelectQuery.class)); - Assert.assertEquals(queryWithTrue, jsonMapper.readValue(actualWithTrue, SelectQuery.class)); + Assert.assertEquals(queryWithNull, objectMapper.readValue(actualWithNull, SelectQuery.class)); + Assert.assertEquals(queryWithFalse, objectMapper.readValue(actualWithFalse, SelectQuery.class)); + Assert.assertEquals(queryWithTrue, objectMapper.readValue(actualWithTrue, SelectQuery.class)); } } diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index b92f136a034a..6cd7dcfa774e 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -251,7 +251,7 @@ public class CachingClusteredClientTest private static final Granularity PT1H_TZ_GRANULARITY = new PeriodGranularity(new Period("PT1H"), null, TIMEZONE); private static final String TOP_DIM = "a_dim"; - private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); static final QueryToolChestWarehouse WAREHOUSE = new MapQueryToolChestWarehouse( ImmutableMap., QueryToolChest>builder() diff --git a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java index 1e8124c37809..9bf1205c2af0 100644 --- a/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/io/druid/sql/calcite/util/CalciteTests.java @@ -90,7 +90,7 @@ public class CalciteTests public static final String DATASOURCE2 = "foo2"; private static final String TIMESTAMP_COLUMN = "t"; - private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig()); + private static final Supplier selectConfigSupplier = Suppliers.ofInstance(new SelectQueryConfig(true)); private static final QueryRunnerFactoryConglomerate CONGLOMERATE = new DefaultQueryRunnerFactoryConglomerate( ImmutableMap., QueryRunnerFactory>builder() From 5c9d11e06a2e946722c1947d96b4c9476c75ebbb Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 1 Mar 2017 15:44:06 -0800 Subject: [PATCH 4/4] Injection fix for test --- .../io/druid/query/aggregation/AggregationTestHelper.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java index af4cb05c5b86..0473213fe825 100644 --- a/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java +++ b/processing/src/test/java/io/druid/query/aggregation/AggregationTestHelper.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.ObjectCodec; import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; @@ -166,6 +167,12 @@ public static final AggregationTestHelper createSelectQueryAggregationTestHelper ) { ObjectMapper mapper = new DefaultObjectMapper(); + mapper.setInjectableValues( + new InjectableValues.Std().addValue( + SelectQueryConfig.class, + new SelectQueryConfig(true) + ) + ); Supplier configSupplier = Suppliers.ofInstance(new SelectQueryConfig(true));