From 9e9193109084110188e9a97aca6053a110996cb9 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 11 Dec 2017 13:23:36 -0800 Subject: [PATCH 1/9] timewarp and timezones changes: * `TimewarpOperator` will now compensate for daylight savings time shifts between date translation ranges for queries using a `PeriodGranularity` with a timezone defined * introduces a new abstract query type `TimeBucketedQuery` for all queries which have a `Granularity` (100% not attached to this name). `GroupByQuery`, `SearchQuery`, `SelectQuery`, `TimeseriesQuery`, and `TopNQuery` all extend `TimeBucke tedQuery`, cutting down on some duplicate code and providing a mechanism for `TimewarpOperator` (and anything else) that needs to be aware of granularity --- .../io/druid/query/TimeBucketedQuery.java | 77 ++++++++++ .../java/io/druid/query/TimewarpOperator.java | 17 ++- .../io/druid/query/groupby/GroupByQuery.java | 27 ++-- .../io/druid/query/search/SearchQuery.java | 20 +-- .../io/druid/query/select/SelectQuery.java | 20 +-- .../query/timeseries/TimeseriesQuery.java | 19 +-- .../java/io/druid/query/topn/TopNQuery.java | 18 +-- .../io/druid/query/TimewarpOperatorTest.java | 142 +++++++++++++++++- 8 files changed, 257 insertions(+), 83 deletions(-) create mode 100644 processing/src/main/java/io/druid/query/TimeBucketedQuery.java diff --git a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java new file mode 100644 index 000000000000..6fc248f4a1ca --- /dev/null +++ b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java @@ -0,0 +1,77 @@ +/* + * 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; + +import com.fasterxml.jackson.annotation.JsonProperty; +import io.druid.guice.annotations.ExtensionPoint; +import io.druid.java.util.common.granularity.Granularity; +import io.druid.query.spec.QuerySegmentSpec; + +import java.util.Map; +import java.util.Objects; + +@ExtensionPoint +public abstract class TimeBucketedQuery> extends BaseQuery +{ + private final Granularity granularity; + + public TimeBucketedQuery( + DataSource dataSource, + QuerySegmentSpec querySegmentSpec, + boolean descending, + Map context, + Granularity granularity + ) + { + super(dataSource, querySegmentSpec, descending, context); + this.granularity = granularity; + } + + @JsonProperty + public Granularity getGranularity() + { + return granularity; + } + + @Override + public boolean equals(final Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + final TimeBucketedQuery that = (TimeBucketedQuery) o; + return Objects.equals(granularity, that.granularity); + } + + @Override + public int hashCode() + { + return Objects.hash( + super.hashCode(), + granularity + ); + } +} diff --git a/processing/src/main/java/io/druid/query/TimewarpOperator.java b/processing/src/main/java/io/druid/query/TimewarpOperator.java index 64ed0cdd9af2..2f6f4f6e97bf 100644 --- a/processing/src/main/java/io/druid/query/TimewarpOperator.java +++ b/processing/src/main/java/io/druid/query/TimewarpOperator.java @@ -24,12 +24,14 @@ import com.google.common.base.Function; import io.druid.data.input.MapBasedRow; import io.druid.java.util.common.DateTimes; +import io.druid.java.util.common.granularity.PeriodGranularity; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.query.timeboundary.TimeBoundaryQuery; import io.druid.query.timeboundary.TimeBoundaryResultValue; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.joda.time.Interval; import org.joda.time.Period; @@ -77,10 +79,19 @@ public QueryRunner postProcess(final QueryRunner baseRunner, final long no { return new QueryRunner() { + @Override public Sequence run(final QueryPlus queryPlus, final Map responseContext) { - final long offset = computeOffset(now); + final DateTimeZone tz; + if (queryPlus.getQuery() instanceof TimeBucketedQuery + && ((TimeBucketedQuery) queryPlus.getQuery()).getGranularity() instanceof PeriodGranularity) { + tz = ((PeriodGranularity) ((TimeBucketedQuery) queryPlus.getQuery()).getGranularity()).getTimeZone(); + } else { + tz = DateTimeZone.UTC; + } + + final long offset = computeOffset(now, tz); final Interval interval = queryPlus.getQuery().getIntervals().get(0); final Interval modifiedInterval = new Interval( @@ -142,7 +153,7 @@ public T apply(T input) * * @return the offset between the mapped time and time t */ - protected long computeOffset(final long t) + protected long computeOffset(final long t, final DateTimeZone tz) { // start is the beginning of the last period ending within dataInterval long start = dataInterval.getEndMillis() - periodMillis; @@ -159,6 +170,6 @@ protected long computeOffset(final long t) tOffset += periodMillis; } tOffset += start; - return tOffset - t; + return tOffset - t - (tz.getOffset(tOffset) - tz.getOffset(t)); } } 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 72fd236402be..eee4ffd2cabc 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -38,7 +38,7 @@ import io.druid.java.util.common.guava.Comparators; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; -import io.druid.query.BaseQuery; +import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Queries; import io.druid.query.Query; @@ -78,7 +78,7 @@ /** */ -public class GroupByQuery extends BaseQuery +public class GroupByQuery extends TimeBucketedQuery { public final static String CTX_KEY_SORT_BY_DIMS_FIRST = "sortByDimsFirst"; @@ -96,7 +96,6 @@ public static Builder builder() private final LimitSpec limitSpec; private final HavingSpec havingSpec; private final DimFilter dimFilter; - private final Granularity granularity; private final List dimensions; private final List aggregatorSpecs; private final List postAggregatorSpecs; @@ -171,15 +170,16 @@ private GroupByQuery( final Map context ) { - super(dataSource, querySegmentSpec, false, context); + super(dataSource, querySegmentSpec, false, context, granularity); this.virtualColumns = VirtualColumns.nullToEmpty(virtualColumns); this.dimFilter = dimFilter; - this.granularity = granularity; this.dimensions = dimensions == null ? ImmutableList.of() : dimensions; for (DimensionSpec spec : this.dimensions) { Preconditions.checkArgument(spec != null, "dimensions has null DimensionSpec"); } + Preconditions.checkNotNull(granularity, "Must specify a granularity"); + this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( this.dimensions.stream().map(DimensionSpec::getOutputName).collect(Collectors.toList()), @@ -189,7 +189,6 @@ private GroupByQuery( this.havingSpec = havingSpec; this.limitSpec = LimitSpec.nullToNoopLimitSpec(limitSpec); - Preconditions.checkNotNull(this.granularity, "Must specify a granularity"); // Verify no duplicate names between dimensions, aggregators, and postAggregators. // They will all end up in the same namespace in the returned Rows and we can't have them clobbering each other. @@ -214,12 +213,6 @@ public DimFilter getDimFilter() return dimFilter; } - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - @JsonProperty public List getDimensions() { @@ -518,12 +511,12 @@ public Ordering getRowOrdering(final boolean granular) private Comparator getTimeComparator(boolean granular) { - if (Granularities.ALL.equals(granularity)) { + if (Granularities.ALL.equals(getGranularity())) { return null; } else if (granular) { return (lhs, rhs) -> Longs.compare( - granularity.bucketStart(lhs.getTimestamp()).getMillis(), - granularity.bucketStart(rhs.getTimestamp()).getMillis() + getGranularity().bucketStart(lhs.getTimestamp()).getMillis(), + getGranularity().bucketStart(rhs.getTimestamp()).getMillis() ); } else { return NON_GRANULAR_TIME_COMP; @@ -990,7 +983,7 @@ public String toString() ", virtualColumns=" + virtualColumns + ", limitSpec=" + limitSpec + ", dimFilter=" + dimFilter + - ", granularity=" + granularity + + ", granularity=" + getGranularity() + ", dimensions=" + dimensions + ", aggregatorSpecs=" + aggregatorSpecs + ", postAggregatorSpecs=" + postAggregatorSpecs + @@ -1015,7 +1008,6 @@ public boolean equals(final Object o) Objects.equals(limitSpec, that.limitSpec) && Objects.equals(havingSpec, that.havingSpec) && Objects.equals(dimFilter, that.dimFilter) && - Objects.equals(granularity, that.granularity) && Objects.equals(dimensions, that.dimensions) && Objects.equals(aggregatorSpecs, that.aggregatorSpecs) && Objects.equals(postAggregatorSpecs, that.postAggregatorSpecs); @@ -1030,7 +1022,6 @@ public int hashCode() limitSpec, havingSpec, dimFilter, - granularity, dimensions, aggregatorSpecs, postAggregatorSpecs diff --git a/processing/src/main/java/io/druid/query/search/SearchQuery.java b/processing/src/main/java/io/druid/query/search/SearchQuery.java index f9cd866e1f7f..2a828a7cc138 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQuery.java +++ b/processing/src/main/java/io/druid/query/search/SearchQuery.java @@ -24,7 +24,7 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.BaseQuery; +import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; @@ -39,13 +39,12 @@ /** */ -public class SearchQuery extends BaseQuery> +public class SearchQuery extends TimeBucketedQuery> { private static final SearchSortSpec DEFAULT_SORT_SPEC = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); private final DimFilter dimFilter; private final SearchSortSpec sortSpec; - private final Granularity granularity; private final List dimensions; private final SearchQuerySpec querySpec; private final int limit; @@ -63,12 +62,11 @@ public SearchQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, false, context); + super(dataSource, querySegmentSpec, false, context, granularity == null ? Granularities.ALL : granularity); Preconditions.checkNotNull(querySegmentSpec, "Must specify an interval"); this.dimFilter = dimFilter; this.sortSpec = sortSpec == null ? DEFAULT_SORT_SPEC : sortSpec; - this.granularity = granularity == null ? Granularities.ALL : granularity; this.limit = (limit == 0) ? 1000 : limit; this.dimensions = dimensions; this.querySpec = querySpec == null ? new AllSearchQuerySpec() : querySpec; @@ -122,12 +120,6 @@ public DimFilter getDimensionsFilter() return dimFilter; } - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - @JsonProperty public int getLimit() { @@ -163,7 +155,7 @@ public String toString() return "SearchQuery{" + "dataSource='" + getDataSource() + '\'' + ", dimFilter=" + dimFilter + - ", granularity='" + granularity + '\'' + + ", granularity='" + getGranularity() + '\'' + ", dimensions=" + dimensions + ", querySpec=" + querySpec + ", querySegmentSpec=" + getQuerySegmentSpec() + @@ -195,9 +187,6 @@ public boolean equals(Object o) if (dimensions != null ? !dimensions.equals(that.dimensions) : that.dimensions != null) { return false; } - if (granularity != null ? !granularity.equals(that.granularity) : that.granularity != null) { - return false; - } if (querySpec != null ? !querySpec.equals(that.querySpec) : that.querySpec != null) { return false; } @@ -214,7 +203,6 @@ public int hashCode() int result = super.hashCode(); result = 31 * result + (dimFilter != null ? dimFilter.hashCode() : 0); result = 31 * result + (sortSpec != null ? sortSpec.hashCode() : 0); - result = 31 * result + (granularity != null ? granularity.hashCode() : 0); result = 31 * result + (dimensions != null ? dimensions.hashCode() : 0); result = 31 * result + (querySpec != null ? querySpec.hashCode() : 0); result = 31 * result + limit; 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 6676777ba3d1..d2ed1ec94da4 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQuery.java +++ b/processing/src/main/java/io/druid/query/select/SelectQuery.java @@ -25,7 +25,7 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.BaseQuery; +import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; @@ -42,10 +42,9 @@ /** */ @JsonTypeName("select") -public class SelectQuery extends BaseQuery> +public class SelectQuery extends TimeBucketedQuery> { private final DimFilter dimFilter; - private final Granularity granularity; private final List dimensions; private final List metrics; private final VirtualColumns virtualColumns; @@ -65,9 +64,8 @@ public SelectQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, descending, context); + super(dataSource, querySegmentSpec, descending, context, granularity == null ? Granularities.ALL : granularity); this.dimFilter = dimFilter; - this.granularity = granularity == null ? Granularities.ALL : granularity; this.dimensions = dimensions; this.virtualColumns = VirtualColumns.nullToEmpty(virtualColumns); this.metrics = metrics; @@ -111,12 +109,6 @@ public DimFilter getDimensionsFilter() return dimFilter; } - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - @JsonProperty public List getDimensions() { @@ -183,7 +175,7 @@ public String toString() ", querySegmentSpec=" + getQuerySegmentSpec() + ", descending=" + isDescending() + ", dimFilter=" + dimFilter + - ", granularity=" + granularity + + ", granularity=" + getGranularity() + ", dimensions=" + dimensions + ", metrics=" + metrics + ", virtualColumns=" + virtualColumns + @@ -209,9 +201,6 @@ public boolean equals(Object o) if (!Objects.equals(dimFilter, that.dimFilter)) { return false; } - if (!Objects.equals(granularity, that.granularity)) { - return false; - } if (!Objects.equals(dimensions, that.dimensions)) { return false; } @@ -233,7 +222,6 @@ public int hashCode() { int result = super.hashCode(); result = 31 * result + (dimFilter != null ? dimFilter.hashCode() : 0); - result = 31 * result + (granularity != null ? granularity.hashCode() : 0); result = 31 * result + (dimensions != null ? dimensions.hashCode() : 0); result = 31 * result + (metrics != null ? metrics.hashCode() : 0); result = 31 * result + (virtualColumns != null ? virtualColumns.hashCode() : 0); diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java index 2cfa00b6235e..79cd8126e5f2 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java @@ -24,7 +24,7 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.ImmutableList; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.BaseQuery; +import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Queries; @@ -43,11 +43,10 @@ /** */ @JsonTypeName("timeseries") -public class TimeseriesQuery extends BaseQuery> +public class TimeseriesQuery extends TimeBucketedQuery> { private final VirtualColumns virtualColumns; private final DimFilter dimFilter; - private final Granularity granularity; private final List aggregatorSpecs; private final List postAggregatorSpecs; @@ -64,11 +63,10 @@ public TimeseriesQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, descending, context); + super(dataSource, querySegmentSpec, descending, context, granularity); this.virtualColumns = VirtualColumns.nullToEmpty(virtualColumns); this.dimFilter = dimFilter; - this.granularity = granularity; this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( ImmutableList.of(), @@ -107,12 +105,6 @@ public DimFilter getDimensionsFilter() return dimFilter; } - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - @JsonProperty("aggregations") public List getAggregatorSpecs() { @@ -168,7 +160,7 @@ public String toString() ", descending=" + isDescending() + ", virtualColumns=" + virtualColumns + ", dimFilter=" + dimFilter + - ", granularity='" + granularity + '\'' + + ", granularity='" + getGranularity() + '\'' + ", aggregatorSpecs=" + aggregatorSpecs + ", postAggregatorSpecs=" + postAggregatorSpecs + ", context=" + getContext() + @@ -190,7 +182,6 @@ public boolean equals(final Object o) final TimeseriesQuery that = (TimeseriesQuery) o; return Objects.equals(virtualColumns, that.virtualColumns) && Objects.equals(dimFilter, that.dimFilter) && - Objects.equals(granularity, that.granularity) && Objects.equals(aggregatorSpecs, that.aggregatorSpecs) && Objects.equals(postAggregatorSpecs, that.postAggregatorSpecs); } @@ -198,6 +189,6 @@ public boolean equals(final Object o) @Override public int hashCode() { - return Objects.hash(super.hashCode(), virtualColumns, dimFilter, granularity, aggregatorSpecs, postAggregatorSpecs); + return Objects.hash(super.hashCode(), virtualColumns, dimFilter, aggregatorSpecs, postAggregatorSpecs); } } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQuery.java b/processing/src/main/java/io/druid/query/topn/TopNQuery.java index 764990b063ed..85099f3510a5 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQuery.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQuery.java @@ -24,7 +24,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.BaseQuery; +import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Queries; import io.druid.query.Query; @@ -42,7 +42,7 @@ /** */ -public class TopNQuery extends BaseQuery> +public class TopNQuery extends TimeBucketedQuery> { public static final String TOPN = "topN"; @@ -51,7 +51,6 @@ public class TopNQuery extends BaseQuery> private final TopNMetricSpec topNMetricSpec; private final int threshold; private final DimFilter dimFilter; - private final Granularity granularity; private final List aggregatorSpecs; private final List postAggregatorSpecs; @@ -70,7 +69,7 @@ public TopNQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, false, context); + super(dataSource, querySegmentSpec, false, context, granularity); this.virtualColumns = VirtualColumns.nullToEmpty(virtualColumns); this.dimensionSpec = dimensionSpec; @@ -78,7 +77,6 @@ public TopNQuery( this.threshold = threshold; this.dimFilter = dimFilter; - this.granularity = granularity; this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( ImmutableList.of(dimensionSpec.getOutputName()), @@ -143,12 +141,6 @@ public DimFilter getDimensionsFilter() return dimFilter; } - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - @JsonProperty("aggregations") public List getAggregatorSpecs() { @@ -218,7 +210,7 @@ public String toString() ", querySegmentSpec=" + getQuerySegmentSpec() + ", virtualColumns=" + virtualColumns + ", dimFilter=" + dimFilter + - ", granularity='" + granularity + '\'' + + ", granularity='" + getGranularity() + '\'' + ", aggregatorSpecs=" + aggregatorSpecs + ", postAggregatorSpecs=" + postAggregatorSpecs + '}'; @@ -242,7 +234,6 @@ public boolean equals(final Object o) Objects.equals(dimensionSpec, topNQuery.dimensionSpec) && Objects.equals(topNMetricSpec, topNQuery.topNMetricSpec) && Objects.equals(dimFilter, topNQuery.dimFilter) && - Objects.equals(granularity, topNQuery.granularity) && Objects.equals(aggregatorSpecs, topNQuery.aggregatorSpecs) && Objects.equals(postAggregatorSpecs, topNQuery.postAggregatorSpecs); } @@ -257,7 +248,6 @@ public int hashCode() topNMetricSpec, threshold, dimFilter, - granularity, aggregatorSpecs, postAggregatorSpecs ); diff --git a/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java b/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java index 8b5675b646fe..a78ef130931c 100644 --- a/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java +++ b/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java @@ -24,6 +24,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import io.druid.java.util.common.DateTimes; +import io.druid.java.util.common.granularity.PeriodGranularity; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; import io.druid.query.aggregation.AggregatorFactory; @@ -31,6 +32,7 @@ import io.druid.query.timeboundary.TimeBoundaryResultValue; import io.druid.query.timeseries.TimeseriesResultValue; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.joda.time.Interval; import org.joda.time.Period; import org.junit.Assert; @@ -57,14 +59,24 @@ public void testComputeOffset() throws Exception final DateTime t = DateTimes.of("2014-01-23"); final DateTime tOffset = DateTimes.of("2014-01-09"); - Assert.assertEquals(tOffset, t.plus(testOperator.computeOffset(t.getMillis()))); + Assert.assertEquals(tOffset, t.plus(testOperator.computeOffset(t.getMillis(), DateTimeZone.UTC))); } { final DateTime t = DateTimes.of("2014-08-02"); final DateTime tOffset = DateTimes.of("2014-01-11"); - Assert.assertEquals(tOffset, t.plus(testOperator.computeOffset(t.getMillis()))); + Assert.assertEquals(tOffset, t.plus(testOperator.computeOffset(t.getMillis(), DateTimeZone.UTC))); + } + + { + final DateTime t = DateTimes.of("2014-08-02T-07"); + final DateTime tOffset = DateTimes.of("2014-01-11T-08"); + + Assert.assertEquals( + tOffset, + t.plus(testOperator.computeOffset(t.getMillis(), DateTimeZone.forID("America/Los_Angeles"))) + ); } } @@ -183,6 +195,132 @@ public Sequence> run( } + @Test + public void testPostProcessWithTimezonesAndDstShift() throws Exception + { + QueryRunner> queryRunner = testOperator.postProcess( + new QueryRunner>() + { + @Override + public Sequence> run( + QueryPlus> queryPlus, + Map responseContext + ) + { + return Sequences.simple( + ImmutableList.of( + new Result<>( + DateTimes.of("2014-01-09T-08"), + new TimeseriesResultValue(ImmutableMap.of("metric", 2)) + ), + new Result<>( + DateTimes.of("2014-01-11T-08"), + new TimeseriesResultValue(ImmutableMap.of("metric", 3)) + ), + new Result<>( + queryPlus.getQuery().getIntervals().get(0).getEnd(), + new TimeseriesResultValue(ImmutableMap.of("metric", 5)) + ) + ) + ); + } + }, + DateTimes.of("2014-08-02T-07").getMillis() + ); + + final Query> query = + Druids.newTimeseriesQueryBuilder() + .dataSource("dummy") + .intervals("2014-07-31T-07/2014-08-05T-07") + .granularity(new PeriodGranularity(new Period("P1D"), null, DateTimeZone.forID("America/Los_Angeles"))) + .aggregators(Arrays.asList(new CountAggregatorFactory("count"))) + .build(); + + Assert.assertEquals( + Lists.newArrayList( + new Result<>( + DateTimes.of("2014-07-31T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 2)) + ), + new Result<>( + DateTimes.of("2014-08-02T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 3)) + ), + new Result<>( + DateTimes.of("2014-08-02T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 5)) + ) + ), + Sequences.toList( + queryRunner.run(QueryPlus.wrap(query), CONTEXT), + Lists.>newArrayList() + ) + ); + } + + @Test + public void testPostProcessWithTimezonesAndNoDstShift() throws Exception + { + QueryRunner> queryRunner = testOperator.postProcess( + new QueryRunner>() + { + @Override + public Sequence> run( + QueryPlus> queryPlus, + Map responseContext + ) + { + return Sequences.simple( + ImmutableList.of( + new Result<>( + DateTimes.of("2014-01-09T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 2)) + ), + new Result<>( + DateTimes.of("2014-01-11T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 3)) + ), + new Result<>( + queryPlus.getQuery().getIntervals().get(0).getEnd(), + new TimeseriesResultValue(ImmutableMap.of("metric", 5)) + ) + ) + ); + } + }, + DateTimes.of("2014-08-02T-07").getMillis() + ); + + final Query> query = + Druids.newTimeseriesQueryBuilder() + .dataSource("dummy") + .intervals("2014-07-31T-07/2014-08-05T-07") + .granularity(new PeriodGranularity(new Period("P1D"), null, DateTimeZone.forID("America/Phoenix"))) + .aggregators(Arrays.asList(new CountAggregatorFactory("count"))) + .build(); + + Assert.assertEquals( + Lists.newArrayList( + new Result<>( + DateTimes.of("2014-07-31T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 2)) + ), + new Result<>( + DateTimes.of("2014-08-02T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 3)) + ), + new Result<>( + DateTimes.of("2014-08-02T-07"), + new TimeseriesResultValue(ImmutableMap.of("metric", 5)) + ) + ), + Sequences.toList( + queryRunner.run(QueryPlus.wrap(query), CONTEXT), + Lists.>newArrayList() + ) + ); + } + @Test public void testEmptyFutureInterval() throws Exception { From b770a96bca3e534df3c2ff82ae8ba817c12552fa Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Sat, 16 Dec 2017 21:23:56 -0800 Subject: [PATCH 2/9] move precondition check to TimeBucketedQuery, add Granularities.nullToAll, add getTimezone to TimeBucketQuery --- .../common/granularity/Granularities.java | 3 +++ .../io/druid/query/TimeBucketedQuery.java | 8 +++++++ .../java/io/druid/query/TimewarpOperator.java | 5 ++-- .../io/druid/query/groupby/GroupByQuery.java | 3 +-- .../io/druid/query/search/SearchQuery.java | 20 ++++++++-------- .../io/druid/query/select/SelectQuery.java | 24 +++++++++---------- 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java b/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java index 599110550f9b..a82644df99bd 100644 --- a/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java +++ b/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java @@ -41,4 +41,7 @@ public class Granularities public static final Granularity ALL = GranularityType.ALL.getDefaultGranularity(); public static final Granularity NONE = GranularityType.NONE.getDefaultGranularity(); + public static Granularity nullToAll(Granularity granularity) { + return granularity == null ? Granularities.ALL : granularity; + } } diff --git a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java index 6fc248f4a1ca..1fb152797b1b 100644 --- a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java +++ b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java @@ -20,9 +20,12 @@ package io.druid.query; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; import io.druid.guice.annotations.ExtensionPoint; import io.druid.java.util.common.granularity.Granularity; +import io.druid.java.util.common.granularity.PeriodGranularity; import io.druid.query.spec.QuerySegmentSpec; +import org.joda.time.DateTimeZone; import java.util.Map; import java.util.Objects; @@ -41,6 +44,7 @@ public TimeBucketedQuery( ) { super(dataSource, querySegmentSpec, descending, context); + Preconditions.checkNotNull(granularity, "Must specify a granularity"); this.granularity = granularity; } @@ -50,6 +54,10 @@ public Granularity getGranularity() return granularity; } + public DateTimeZone getTimezone() { + return granularity instanceof PeriodGranularity ? ((PeriodGranularity)granularity).getTimeZone() : DateTimeZone.UTC; + } + @Override public boolean equals(final Object o) { diff --git a/processing/src/main/java/io/druid/query/TimewarpOperator.java b/processing/src/main/java/io/druid/query/TimewarpOperator.java index 2f6f4f6e97bf..516cacb9a9f0 100644 --- a/processing/src/main/java/io/druid/query/TimewarpOperator.java +++ b/processing/src/main/java/io/druid/query/TimewarpOperator.java @@ -84,9 +84,8 @@ public QueryRunner postProcess(final QueryRunner baseRunner, final long no public Sequence run(final QueryPlus queryPlus, final Map responseContext) { final DateTimeZone tz; - if (queryPlus.getQuery() instanceof TimeBucketedQuery - && ((TimeBucketedQuery) queryPlus.getQuery()).getGranularity() instanceof PeriodGranularity) { - tz = ((PeriodGranularity) ((TimeBucketedQuery) queryPlus.getQuery()).getGranularity()).getTimeZone(); + if (queryPlus.getQuery() instanceof TimeBucketedQuery) { + tz = ((TimeBucketedQuery) queryPlus.getQuery()).getTimezone(); } else { tz = DateTimeZone.UTC; } 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 eee4ffd2cabc..8296f5de2ac4 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -38,12 +38,12 @@ import io.druid.java.util.common.guava.Comparators; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; -import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Queries; import io.druid.query.Query; import io.druid.query.QueryDataSource; import io.druid.query.TableDataSource; +import io.druid.query.TimeBucketedQuery; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; @@ -178,7 +178,6 @@ private GroupByQuery( for (DimensionSpec spec : this.dimensions) { Preconditions.checkArgument(spec != null, "dimensions has null DimensionSpec"); } - Preconditions.checkNotNull(granularity, "Must specify a granularity"); this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( diff --git a/processing/src/main/java/io/druid/query/search/SearchQuery.java b/processing/src/main/java/io/druid/query/search/SearchQuery.java index 2a828a7cc138..6566fa33aa6c 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQuery.java +++ b/processing/src/main/java/io/druid/query/search/SearchQuery.java @@ -24,11 +24,11 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; import io.druid.query.Result; +import io.druid.query.TimeBucketedQuery; import io.druid.query.dimension.DimensionSpec; import io.druid.query.filter.DimFilter; import io.druid.query.ordering.StringComparators; @@ -62,7 +62,7 @@ public SearchQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, false, context, granularity == null ? Granularities.ALL : granularity); + super(dataSource, querySegmentSpec, false, context, Granularities.nullToAll(granularity)); Preconditions.checkNotNull(querySegmentSpec, "Must specify an interval"); this.dimFilter = dimFilter; @@ -153,14 +153,14 @@ public SearchQuery withLimit(int newLimit) public String toString() { return "SearchQuery{" + - "dataSource='" + getDataSource() + '\'' + - ", dimFilter=" + dimFilter + - ", granularity='" + getGranularity() + '\'' + - ", dimensions=" + dimensions + - ", querySpec=" + querySpec + - ", querySegmentSpec=" + getQuerySegmentSpec() + - ", limit=" + limit + - '}'; + "dataSource='" + getDataSource() + '\'' + + ", dimFilter=" + dimFilter + + ", granularity='" + getGranularity() + '\'' + + ", dimensions=" + dimensions + + ", querySpec=" + querySpec + + ", querySegmentSpec=" + getQuerySegmentSpec() + + ", limit=" + limit + + '}'; } @Override 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 d2ed1ec94da4..8baea8c9d4c4 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQuery.java +++ b/processing/src/main/java/io/druid/query/select/SelectQuery.java @@ -25,11 +25,11 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.TimeBucketedQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; import io.druid.query.Result; +import io.druid.query.TimeBucketedQuery; import io.druid.query.dimension.DimensionSpec; import io.druid.query.filter.DimFilter; import io.druid.query.spec.QuerySegmentSpec; @@ -64,7 +64,7 @@ public SelectQuery( @JsonProperty("context") Map context ) { - super(dataSource, querySegmentSpec, descending, context, granularity == null ? Granularities.ALL : granularity); + super(dataSource, querySegmentSpec, descending, context, Granularities.nullToAll(granularity)); this.dimFilter = dimFilter; this.dimensions = dimensions; this.virtualColumns = VirtualColumns.nullToEmpty(virtualColumns); @@ -171,16 +171,16 @@ public SelectQuery withDimFilter(DimFilter dimFilter) public String toString() { return "SelectQuery{" + - "dataSource='" + getDataSource() + '\'' + - ", querySegmentSpec=" + getQuerySegmentSpec() + - ", descending=" + isDescending() + - ", dimFilter=" + dimFilter + - ", granularity=" + getGranularity() + - ", dimensions=" + dimensions + - ", metrics=" + metrics + - ", virtualColumns=" + virtualColumns + - ", pagingSpec=" + pagingSpec + - '}'; + "dataSource='" + getDataSource() + '\'' + + ", querySegmentSpec=" + getQuerySegmentSpec() + + ", descending=" + isDescending() + + ", dimFilter=" + dimFilter + + ", granularity=" + getGranularity() + + ", dimensions=" + dimensions + + ", metrics=" + metrics + + ", virtualColumns=" + virtualColumns + + ", pagingSpec=" + pagingSpec + + '}'; } @Override From e41fe28f3a2dcd12c3ff48efb7c1b009d43b6299 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Sat, 16 Dec 2017 21:38:00 -0800 Subject: [PATCH 3/9] formatting --- .../io/druid/java/util/common/granularity/Granularities.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java b/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java index a82644df99bd..de8b6f688d2f 100644 --- a/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java +++ b/java-util/src/main/java/io/druid/java/util/common/granularity/Granularities.java @@ -41,7 +41,8 @@ public class Granularities public static final Granularity ALL = GranularityType.ALL.getDefaultGranularity(); public static final Granularity NONE = GranularityType.NONE.getDefaultGranularity(); - public static Granularity nullToAll(Granularity granularity) { + public static Granularity nullToAll(Granularity granularity) + { return granularity == null ? Granularities.ALL : granularity; } } From 140ce000cb84cf212082499cc9d1404804914e50 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Sat, 16 Dec 2017 22:09:13 -0800 Subject: [PATCH 4/9] more formatting --- .../src/main/java/io/druid/query/TimeBucketedQuery.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java index 1fb152797b1b..7a950977ad40 100644 --- a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java +++ b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java @@ -54,8 +54,11 @@ public Granularity getGranularity() return granularity; } - public DateTimeZone getTimezone() { - return granularity instanceof PeriodGranularity ? ((PeriodGranularity)granularity).getTimeZone() : DateTimeZone.UTC; + public DateTimeZone getTimezone() + { + return granularity instanceof PeriodGranularity + ? ((PeriodGranularity) granularity).getTimeZone() + : DateTimeZone.UTC; } @Override From a8aae84dc9964c3eaf88614ce350e0b1830ffc77 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Sun, 17 Dec 2017 00:47:38 -0800 Subject: [PATCH 5/9] unused import --- processing/src/main/java/io/druid/query/TimewarpOperator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/processing/src/main/java/io/druid/query/TimewarpOperator.java b/processing/src/main/java/io/druid/query/TimewarpOperator.java index 516cacb9a9f0..6fa7f70603ed 100644 --- a/processing/src/main/java/io/druid/query/TimewarpOperator.java +++ b/processing/src/main/java/io/druid/query/TimewarpOperator.java @@ -24,7 +24,6 @@ import com.google.common.base.Function; import io.druid.data.input.MapBasedRow; import io.druid.java.util.common.DateTimes; -import io.druid.java.util.common.granularity.PeriodGranularity; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; import io.druid.query.spec.MultipleIntervalSegmentSpec; From 4eaeb403bd4d3fec0a30da6439af387ff5fcf798 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 5 Jan 2018 13:52:23 -0800 Subject: [PATCH 6/9] changes: * add 'getGranularity' and 'getTimezone' to 'Query' interface * merge 'TimeBucketedQuery' into 'BaseQuery' * fixup tests from resulting serialization changes --- .../main/java/io/druid/query/BaseQuery.java | 78 ++++++++++------ .../src/main/java/io/druid/query/Query.java | 6 ++ .../io/druid/query/TimeBucketedQuery.java | 88 ------------------- .../java/io/druid/query/TimewarpOperator.java | 9 +- .../io/druid/query/groupby/GroupByQuery.java | 4 +- .../io/druid/query/search/SearchQuery.java | 4 +- .../io/druid/query/select/SelectQuery.java | 4 +- .../query/timeseries/TimeseriesQuery.java | 4 +- .../java/io/druid/query/topn/TopNQuery.java | 4 +- .../druid/query/scan/ScanQuerySpecTest.java | 3 +- .../druid/sql/calcite/CalciteQueryTest.java | 8 +- 11 files changed, 73 insertions(+), 139 deletions(-) delete mode 100644 processing/src/main/java/io/druid/query/TimeBucketedQuery.java diff --git a/processing/src/main/java/io/druid/query/BaseQuery.java b/processing/src/main/java/io/druid/query/BaseQuery.java index 7b511097a832..8be90a294e5a 100644 --- a/processing/src/main/java/io/druid/query/BaseQuery.java +++ b/processing/src/main/java/io/druid/query/BaseQuery.java @@ -25,12 +25,17 @@ import com.google.common.collect.Maps; import com.google.common.collect.Ordering; import io.druid.guice.annotations.ExtensionPoint; +import io.druid.java.util.common.granularity.Granularities; +import io.druid.java.util.common.granularity.Granularity; +import io.druid.java.util.common.granularity.PeriodGranularity; import io.druid.query.spec.QuerySegmentSpec; +import org.joda.time.DateTimeZone; import org.joda.time.Duration; import org.joda.time.Interval; import java.util.List; import java.util.Map; +import java.util.Objects; /** */ @@ -50,6 +55,7 @@ public static void checkInterrupted() private final Map context; private final QuerySegmentSpec querySegmentSpec; private volatile Duration duration; + private final Granularity granularity; public BaseQuery( DataSource dataSource, @@ -65,6 +71,26 @@ public BaseQuery( this.context = context; this.querySegmentSpec = querySegmentSpec; this.descending = descending; + this.granularity = Granularities.ALL; + } + + public BaseQuery( + DataSource dataSource, + QuerySegmentSpec querySegmentSpec, + boolean descending, + Map context, + Granularity granularity + ) + { + Preconditions.checkNotNull(dataSource, "dataSource can't be null"); + Preconditions.checkNotNull(querySegmentSpec, "querySegmentSpec can't be null"); + Preconditions.checkNotNull(granularity, "Must specify a granularity"); + + this.dataSource = dataSource; + this.context = context; + this.querySegmentSpec = querySegmentSpec; + this.descending = descending; + this.granularity = granularity; } @JsonProperty @@ -115,6 +141,21 @@ public Duration getDuration() return duration; } + @Override + @JsonProperty + public Granularity getGranularity() + { + return granularity; + } + + @Override + public DateTimeZone getTimezone() + { + return granularity instanceof PeriodGranularity + ? ((PeriodGranularity) granularity).getTimeZone() + : DateTimeZone.UTC; + } + @Override @JsonProperty public Map getContext() @@ -193,38 +234,19 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } - - BaseQuery baseQuery = (BaseQuery) o; - - if (descending != baseQuery.descending) { - return false; - } - if (context != null ? !context.equals(baseQuery.context) : baseQuery.context != null) { - return false; - } - if (dataSource != null ? !dataSource.equals(baseQuery.dataSource) : baseQuery.dataSource != null) { - return false; - } - if (duration != null ? !duration.equals(baseQuery.duration) : baseQuery.duration != null) { - return false; - } - if (querySegmentSpec != null - ? !querySegmentSpec.equals(baseQuery.querySegmentSpec) - : baseQuery.querySegmentSpec != null) { - return false; - } - - return true; + BaseQuery baseQuery = (BaseQuery) o; + return descending == baseQuery.descending && + Objects.equals(dataSource, baseQuery.dataSource) && + Objects.equals(context, baseQuery.context) && + Objects.equals(querySegmentSpec, baseQuery.querySegmentSpec) && + Objects.equals(duration, baseQuery.duration) && + Objects.equals(granularity, baseQuery.granularity); } @Override public int hashCode() { - int result = dataSource != null ? dataSource.hashCode() : 0; - result = 31 * result + (descending ? 1 : 0); - result = 31 * result + (context != null ? context.hashCode() : 0); - result = 31 * result + (querySegmentSpec != null ? querySegmentSpec.hashCode() : 0); - result = 31 * result + (duration != null ? duration.hashCode() : 0); - return result; + + return Objects.hash(dataSource, descending, context, querySegmentSpec, duration, granularity); } } diff --git a/processing/src/main/java/io/druid/query/Query.java b/processing/src/main/java/io/druid/query/Query.java index efb9fd500e09..da30bb62b82b 100644 --- a/processing/src/main/java/io/druid/query/Query.java +++ b/processing/src/main/java/io/druid/query/Query.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.google.common.collect.Ordering; import io.druid.guice.annotations.ExtensionPoint; +import io.druid.java.util.common.granularity.Granularity; import io.druid.query.datasourcemetadata.DataSourceMetadataQuery; import io.druid.query.filter.DimFilter; import io.druid.query.groupby.GroupByQuery; @@ -34,6 +35,7 @@ import io.druid.query.timeboundary.TimeBoundaryQuery; import io.druid.query.timeseries.TimeseriesQuery; import io.druid.query.topn.TopNQuery; +import org.joda.time.DateTimeZone; import org.joda.time.Duration; import org.joda.time.Interval; @@ -80,6 +82,10 @@ public interface Query Duration getDuration(); + Granularity getGranularity(); + + DateTimeZone getTimezone(); + Map getContext(); ContextType getContextValue(String key); diff --git a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java b/processing/src/main/java/io/druid/query/TimeBucketedQuery.java deleted file mode 100644 index 7a950977ad40..000000000000 --- a/processing/src/main/java/io/druid/query/TimeBucketedQuery.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * 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; - -import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.base.Preconditions; -import io.druid.guice.annotations.ExtensionPoint; -import io.druid.java.util.common.granularity.Granularity; -import io.druid.java.util.common.granularity.PeriodGranularity; -import io.druid.query.spec.QuerySegmentSpec; -import org.joda.time.DateTimeZone; - -import java.util.Map; -import java.util.Objects; - -@ExtensionPoint -public abstract class TimeBucketedQuery> extends BaseQuery -{ - private final Granularity granularity; - - public TimeBucketedQuery( - DataSource dataSource, - QuerySegmentSpec querySegmentSpec, - boolean descending, - Map context, - Granularity granularity - ) - { - super(dataSource, querySegmentSpec, descending, context); - Preconditions.checkNotNull(granularity, "Must specify a granularity"); - this.granularity = granularity; - } - - @JsonProperty - public Granularity getGranularity() - { - return granularity; - } - - public DateTimeZone getTimezone() - { - return granularity instanceof PeriodGranularity - ? ((PeriodGranularity) granularity).getTimeZone() - : DateTimeZone.UTC; - } - - @Override - public boolean equals(final Object o) - { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - if (!super.equals(o)) { - return false; - } - final TimeBucketedQuery that = (TimeBucketedQuery) o; - return Objects.equals(granularity, that.granularity); - } - - @Override - public int hashCode() - { - return Objects.hash( - super.hashCode(), - granularity - ); - } -} diff --git a/processing/src/main/java/io/druid/query/TimewarpOperator.java b/processing/src/main/java/io/druid/query/TimewarpOperator.java index 6fa7f70603ed..cee9047e5bf2 100644 --- a/processing/src/main/java/io/druid/query/TimewarpOperator.java +++ b/processing/src/main/java/io/druid/query/TimewarpOperator.java @@ -78,17 +78,10 @@ public QueryRunner postProcess(final QueryRunner baseRunner, final long no { return new QueryRunner() { - @Override public Sequence run(final QueryPlus queryPlus, final Map responseContext) { - final DateTimeZone tz; - if (queryPlus.getQuery() instanceof TimeBucketedQuery) { - tz = ((TimeBucketedQuery) queryPlus.getQuery()).getTimezone(); - } else { - tz = DateTimeZone.UTC; - } - + final DateTimeZone tz = queryPlus.getQuery().getTimezone(); final long offset = computeOffset(now, tz); final Interval interval = queryPlus.getQuery().getIntervals().get(0); 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 8296f5de2ac4..9043ad6a1e12 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -38,12 +38,12 @@ import io.druid.java.util.common.guava.Comparators; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; +import io.druid.query.BaseQuery; import io.druid.query.DataSource; import io.druid.query.Queries; import io.druid.query.Query; import io.druid.query.QueryDataSource; import io.druid.query.TableDataSource; -import io.druid.query.TimeBucketedQuery; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; @@ -78,7 +78,7 @@ /** */ -public class GroupByQuery extends TimeBucketedQuery +public class GroupByQuery extends BaseQuery { public final static String CTX_KEY_SORT_BY_DIMS_FIRST = "sortByDimsFirst"; diff --git a/processing/src/main/java/io/druid/query/search/SearchQuery.java b/processing/src/main/java/io/druid/query/search/SearchQuery.java index 6566fa33aa6c..df298d55c5ba 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQuery.java +++ b/processing/src/main/java/io/druid/query/search/SearchQuery.java @@ -24,11 +24,11 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; +import io.druid.query.BaseQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; import io.druid.query.Result; -import io.druid.query.TimeBucketedQuery; import io.druid.query.dimension.DimensionSpec; import io.druid.query.filter.DimFilter; import io.druid.query.ordering.StringComparators; @@ -39,7 +39,7 @@ /** */ -public class SearchQuery extends TimeBucketedQuery> +public class SearchQuery extends BaseQuery> { private static final SearchSortSpec DEFAULT_SORT_SPEC = new SearchSortSpec(StringComparators.LEXICOGRAPHIC); 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 8baea8c9d4c4..2bc972b9fac7 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQuery.java +++ b/processing/src/main/java/io/druid/query/select/SelectQuery.java @@ -25,11 +25,11 @@ import com.google.common.base.Preconditions; import io.druid.java.util.common.granularity.Granularities; import io.druid.java.util.common.granularity.Granularity; +import io.druid.query.BaseQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Query; import io.druid.query.Result; -import io.druid.query.TimeBucketedQuery; import io.druid.query.dimension.DimensionSpec; import io.druid.query.filter.DimFilter; import io.druid.query.spec.QuerySegmentSpec; @@ -42,7 +42,7 @@ /** */ @JsonTypeName("select") -public class SelectQuery extends TimeBucketedQuery> +public class SelectQuery extends BaseQuery> { private final DimFilter dimFilter; private final List dimensions; diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java index 79cd8126e5f2..d1ac95cdadce 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java @@ -24,7 +24,7 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.ImmutableList; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.TimeBucketedQuery; +import io.druid.query.BaseQuery; import io.druid.query.DataSource; import io.druid.query.Druids; import io.druid.query.Queries; @@ -43,7 +43,7 @@ /** */ @JsonTypeName("timeseries") -public class TimeseriesQuery extends TimeBucketedQuery> +public class TimeseriesQuery extends BaseQuery> { private final VirtualColumns virtualColumns; private final DimFilter dimFilter; diff --git a/processing/src/main/java/io/druid/query/topn/TopNQuery.java b/processing/src/main/java/io/druid/query/topn/TopNQuery.java index 85099f3510a5..844142d01c50 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQuery.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQuery.java @@ -24,7 +24,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import io.druid.java.util.common.granularity.Granularity; -import io.druid.query.TimeBucketedQuery; +import io.druid.query.BaseQuery; import io.druid.query.DataSource; import io.druid.query.Queries; import io.druid.query.Query; @@ -42,7 +42,7 @@ /** */ -public class TopNQuery extends TimeBucketedQuery> +public class TopNQuery extends BaseQuery> { public static final String TOPN = "topN"; diff --git a/processing/src/test/java/io/druid/query/scan/ScanQuerySpecTest.java b/processing/src/test/java/io/druid/query/scan/ScanQuerySpecTest.java index 774a3b798a6e..8b903f7b72f5 100644 --- a/processing/src/test/java/io/druid/query/scan/ScanQuerySpecTest.java +++ b/processing/src/test/java/io/druid/query/scan/ScanQuerySpecTest.java @@ -57,7 +57,8 @@ public void testSerializationLegacyString() throws Exception + "\"columns\":[\"market\",\"quality\",\"index\"]," + "\"legacy\":null," + "\"context\":null," - + "\"descending\":false}"; + + "\"descending\":false," + + "\"granularity\":{\"type\":\"all\"}}"; ScanQuery query = new ScanQuery( new TableDataSource(QueryRunnerTestHelper.dataSource), diff --git a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java index 97502ea8c8cd..8e8d3771236c 100644 --- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java @@ -555,7 +555,7 @@ public void testExplainSelectStar() throws Exception ImmutableList.of(), ImmutableList.of( new Object[]{ - "DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"__time\",\"cnt\",\"dim1\",\"dim2\",\"m1\",\"m2\",\"unique_dim1\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false}], signature=[{__time:LONG, cnt:LONG, dim1:STRING, dim2:STRING, m1:FLOAT, m2:DOUBLE, unique_dim1:COMPLEX}])\n" + "DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"__time\",\"cnt\",\"dim1\",\"dim2\",\"m1\",\"m2\",\"unique_dim1\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false,\"granularity\":{\"type\":\"all\"}}], signature=[{__time:LONG, cnt:LONG, dim1:STRING, dim2:STRING, m1:FLOAT, m2:DOUBLE, unique_dim1:COMPLEX}])\n" } ) ); @@ -801,8 +801,8 @@ public void testExplainSelfJoinWithFallback() throws Exception { final String explanation = "BindableJoin(condition=[=($0, $2)], joinType=[inner])\n" - + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":{\"type\":\"not\",\"field\":{\"type\":\"selector\",\"dimension\":\"dim1\",\"value\":\"\",\"extractionFn\":null}},\"columns\":[\"dim1\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false}], signature=[{dim1:STRING}])\n" - + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"dim1\",\"dim2\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false}], signature=[{dim1:STRING, dim2:STRING}])\n"; + + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":{\"type\":\"not\",\"field\":{\"type\":\"selector\",\"dimension\":\"dim1\",\"value\":\"\",\"extractionFn\":null}},\"columns\":[\"dim1\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false,\"granularity\":{\"type\":\"all\"}}], signature=[{dim1:STRING}])\n" + + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"dim1\",\"dim2\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false,\"granularity\":{\"type\":\"all\"}}], signature=[{dim1:STRING, dim2:STRING}])\n"; testQuery( PLANNER_CONFIG_FALLBACK, @@ -6094,7 +6094,7 @@ public void testUsingSubqueryAsPartOfOrFilter() throws Exception + " BindableFilter(condition=[OR(=($0, 'xxx'), CAST(AND(IS NOT NULL($4), <>($2, 0))):BOOLEAN)])\n" + " BindableJoin(condition=[=($1, $3)], joinType=[left])\n" + " BindableJoin(condition=[true], joinType=[inner])\n" - + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"dim1\",\"dim2\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false}], signature=[{dim1:STRING, dim2:STRING}])\n" + + " DruidQueryRel(query=[{\"queryType\":\"scan\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[],\"resultFormat\":\"compactedList\",\"batchSize\":20480,\"limit\":9223372036854775807,\"filter\":null,\"columns\":[\"dim1\",\"dim2\"],\"legacy\":false,\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false,\"granularity\":{\"type\":\"all\"}}], signature=[{dim1:STRING, dim2:STRING}])\n" + " DruidQueryRel(query=[{\"queryType\":\"timeseries\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"descending\":false,\"virtualColumns\":[],\"filter\":{\"type\":\"like\",\"dimension\":\"dim1\",\"pattern\":\"%bc\",\"escape\":null,\"extractionFn\":null},\"granularity\":{\"type\":\"all\"},\"aggregations\":[{\"type\":\"count\",\"name\":\"a0\"}],\"postAggregations\":[],\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"skipEmptyBuckets\":true,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"}}], signature=[{a0:LONG}])\n" + " DruidQueryRel(query=[{\"queryType\":\"groupBy\",\"dataSource\":{\"type\":\"table\",\"name\":\"foo\"},\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"-146136543-09-08T08:23:32.096Z/146140482-04-24T15:36:27.903Z\"]},\"virtualColumns\":[{\"type\":\"expression\",\"name\":\"d1:v\",\"expression\":\"1\",\"outputType\":\"LONG\"}],\"filter\":{\"type\":\"like\",\"dimension\":\"dim1\",\"pattern\":\"%bc\",\"escape\":null,\"extractionFn\":null},\"granularity\":{\"type\":\"all\"},\"dimensions\":[{\"type\":\"default\",\"dimension\":\"dim1\",\"outputName\":\"d0\",\"outputType\":\"STRING\"},{\"type\":\"default\",\"dimension\":\"d1:v\",\"outputName\":\"d1\",\"outputType\":\"LONG\"}],\"aggregations\":[],\"postAggregations\":[],\"having\":null,\"limitSpec\":{\"type\":\"NoopLimitSpec\"},\"context\":{\"defaultTimeout\":300000,\"maxScatterGatherBytes\":9223372036854775807,\"sqlCurrentTimestamp\":\"2000-01-01T00:00:00Z\"},\"descending\":false}], signature=[{d0:STRING, d1:LONG}])\n"; From 963173f46ae4e2b1f209b0a81738c3e56e20e9a9 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 5 Jan 2018 14:45:40 -0800 Subject: [PATCH 7/9] dedupe --- processing/src/main/java/io/druid/query/BaseQuery.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/processing/src/main/java/io/druid/query/BaseQuery.java b/processing/src/main/java/io/druid/query/BaseQuery.java index 8be90a294e5a..86587d47ae84 100644 --- a/processing/src/main/java/io/druid/query/BaseQuery.java +++ b/processing/src/main/java/io/druid/query/BaseQuery.java @@ -64,14 +64,7 @@ public BaseQuery( Map context ) { - Preconditions.checkNotNull(dataSource, "dataSource can't be null"); - Preconditions.checkNotNull(querySegmentSpec, "querySegmentSpec can't be null"); - - this.dataSource = dataSource; - this.context = context; - this.querySegmentSpec = querySegmentSpec; - this.descending = descending; - this.granularity = Granularities.ALL; + this(dataSource, querySegmentSpec, descending, context, Granularities.ALL); } public BaseQuery( From caf857f24fcfae09f3d5da0d589d3ecfc7d0a66a Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 5 Jan 2018 23:17:51 -0800 Subject: [PATCH 8/9] fix after merge --- .../test/java/io/druid/query/TimewarpOperatorTest.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java b/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java index 2191513a5ce2..d4f9bc2f8b4f 100644 --- a/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java +++ b/processing/src/test/java/io/druid/query/TimewarpOperatorTest.java @@ -245,10 +245,7 @@ public Sequence> run( new TimeseriesResultValue(ImmutableMap.of("metric", 5)) ) ), - Sequences.toList( - queryRunner.run(QueryPlus.wrap(query), CONTEXT), - Lists.>newArrayList() - ) + queryRunner.run(QueryPlus.wrap(query), CONTEXT).toList() ); } @@ -308,10 +305,7 @@ public Sequence> run( new TimeseriesResultValue(ImmutableMap.of("metric", 5)) ) ), - Sequences.toList( - queryRunner.run(QueryPlus.wrap(query), CONTEXT), - Lists.>newArrayList() - ) + queryRunner.run(QueryPlus.wrap(query), CONTEXT).toList() ); } From 2752ee0ef032053bd5f63f33bedf3244f03b7a7a Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 10 Jan 2018 19:03:43 -0800 Subject: [PATCH 9/9] suppress warning --- processing/src/main/java/io/druid/query/Query.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/processing/src/main/java/io/druid/query/Query.java b/processing/src/main/java/io/druid/query/Query.java index da30bb62b82b..06ff069b205d 100644 --- a/processing/src/main/java/io/druid/query/Query.java +++ b/processing/src/main/java/io/druid/query/Query.java @@ -82,6 +82,8 @@ public interface Query Duration getDuration(); + // currently unused, but helping enforce the idea that all queries have a Granularity + @SuppressWarnings("unused") Granularity getGranularity(); DateTimeZone getTimezone();