diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 5a85aa0d1906f..1ae53a8919ad4 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -164,19 +164,6 @@ public void writeTo(StreamOutput out) throws IOException { */ public abstract long nextRoundingValue(long value); - /** - * How "offset" this rounding is from the traditional "start" of the period. - * @deprecated We're in the process of abstracting offset *into* Rounding - * so keep any usage to migratory shims - */ - @Deprecated - public abstract long offset(); - - /** - * Strip the {@code offset} from these bounds. - */ - public abstract Rounding withoutOffset(); - @Override public abstract boolean equals(Object obj); @@ -438,16 +425,6 @@ public long nextRoundingValue(long utcMillis) { } } - @Override - public long offset() { - return 0; - } - - @Override - public Rounding withoutOffset() { - return this; - } - @Override public int hashCode() { return Objects.hash(unit, timeZone); @@ -579,16 +556,6 @@ public long nextRoundingValue(long time) { .toInstant().toEpochMilli(); } - @Override - public long offset() { - return 0; - } - - @Override - public Rounding withoutOffset() { - return this; - } - @Override public int hashCode() { return Objects.hash(interval, timeZone); @@ -650,17 +617,8 @@ public long round(long value) { @Override public long nextRoundingValue(long value) { - return delegate.nextRoundingValue(value - offset) + offset; - } - - @Override - public long offset() { - return offset; - } - - @Override - public Rounding withoutOffset() { - return delegate; + // This isn't needed by the current users. We'll implement it when we migrate other users to it. + throw new UnsupportedOperationException("not yet supported"); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java index c58cce7c2e76d..3e9c219d0235d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java @@ -500,13 +500,13 @@ protected ValuesSourceAggregatorFactory innerBuild(QueryShardConte Builder subFactoriesBuilder) throws IOException { final ZoneId tz = timeZone(); // TODO use offset here rather than explicitly in the aggregation - final Rounding rounding = dateHistogramInterval.createRounding(tz, offset); + final Rounding rounding = dateHistogramInterval.createRounding(tz, 0); final ZoneId rewrittenTimeZone = rewriteTimeZone(queryShardContext); final Rounding shardRounding; if (tz == rewrittenTimeZone) { shardRounding = rounding; } else { - shardRounding = dateHistogramInterval.createRounding(rewrittenTimeZone, offset); + shardRounding = dateHistogramInterval.createRounding(rewrittenTimeZone, 0); } ExtendedBounds roundedBounds = null; @@ -514,7 +514,7 @@ protected ValuesSourceAggregatorFactory innerBuild(QueryShardConte // parse any string bounds to longs and round roundedBounds = this.extendedBounds.parseAndValidate(name, queryShardContext, config.format()).round(rounding); } - return new DateHistogramAggregatorFactory(name, config, order, keyed, minDocCount, + return new DateHistogramAggregatorFactory(name, config, offset, order, keyed, minDocCount, rounding, shardRounding, roundedBounds, queryShardContext, parent, subFactoriesBuilder, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java index 8479ea066aa50..0c7a91505ae88 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java @@ -64,9 +64,10 @@ class DateHistogramAggregator extends BucketsAggregator { private final ExtendedBounds extendedBounds; private final LongHash bucketOrds; + private long offset; DateHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, Rounding shardRounding, - BucketOrder order, boolean keyed, + long offset, BucketOrder order, boolean keyed, long minDocCount, @Nullable ExtendedBounds extendedBounds, @Nullable ValuesSource.Numeric valuesSource, DocValueFormat formatter, SearchContext aggregationContext, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { @@ -74,6 +75,7 @@ class DateHistogramAggregator extends BucketsAggregator { super(name, factories, aggregationContext, parent, pipelineAggregators, metaData); this.rounding = rounding; this.shardRounding = shardRounding; + this.offset = offset; this.order = InternalOrder.validate(order, this); this.keyed = keyed; this.minDocCount = minDocCount; @@ -111,7 +113,7 @@ public void collect(int doc, long bucket) throws IOException { long value = values.nextValue(); // We can use shardRounding here, which is sometimes more efficient // if daylight saving times are involved. - long rounded = shardRounding.round(value); + long rounded = shardRounding.round(value - offset) + offset; assert rounded >= previousRounded; if (rounded == previousRounded) { continue; @@ -148,7 +150,7 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, buckets, order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, keyed, + return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, formatter, keyed, pipelineAggregators(), metaData()); } @@ -157,8 +159,8 @@ public InternalAggregation buildEmptyAggregation() { InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, - keyed, pipelineAggregators(), metaData()); + return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, offset, emptyBucketInfo, formatter, keyed, + pipelineAggregators(), metaData()); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index d68cf814f32bc..86555767e25ea 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -39,6 +39,7 @@ public final class DateHistogramAggregatorFactory extends ValuesSourceAggregatorFactory { + private final long offset; private final BucketOrder order; private final boolean keyed; private final long minDocCount; @@ -47,11 +48,12 @@ public final class DateHistogramAggregatorFactory private final Rounding shardRounding; public DateHistogramAggregatorFactory(String name, ValuesSourceConfig config, - BucketOrder order, boolean keyed, long minDocCount, + long offset, BucketOrder order, boolean keyed, long minDocCount, Rounding rounding, Rounding shardRounding, ExtendedBounds extendedBounds, QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); + this.offset = offset; this.order = order; this.keyed = keyed; this.minDocCount = minDocCount; @@ -102,7 +104,7 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, private Aggregator createAggregator(ValuesSource.Numeric valuesSource, SearchContext searchContext, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { - return new DateHistogramAggregator(name, factories, rounding, shardRounding, order, keyed, minDocCount, extendedBounds, + return new DateHistogramAggregator(name, factories, rounding, shardRounding, offset, order, keyed, minDocCount, extendedBounds, valuesSource, config.format(), searchContext, parent, pipelineAggregators, metaData); } @@ -111,7 +113,7 @@ private Aggregator createRangeAggregator(ValuesSource.Range valuesSource, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { - return new DateRangeHistogramAggregator(name, factories, rounding, shardRounding, order, keyed, minDocCount, extendedBounds, + return new DateRangeHistogramAggregator(name, factories, rounding, shardRounding, offset, order, keyed, minDocCount, extendedBounds, valuesSource, config.format(), searchContext, parent, pipelineAggregators, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregator.java index 9c6417165ab30..9eed2a542f9dd 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregator.java @@ -67,9 +67,10 @@ class DateRangeHistogramAggregator extends BucketsAggregator { private final ExtendedBounds extendedBounds; private final LongHash bucketOrds; + private long offset; DateRangeHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, Rounding shardRounding, - BucketOrder order, boolean keyed, + long offset, BucketOrder order, boolean keyed, long minDocCount, @Nullable ExtendedBounds extendedBounds, @Nullable ValuesSource.Range valuesSource, DocValueFormat formatter, SearchContext aggregationContext, Aggregator parent, List pipelineAggregators, @@ -78,6 +79,7 @@ class DateRangeHistogramAggregator extends BucketsAggregator { super(name, factories, aggregationContext, parent, pipelineAggregators, metaData); this.rounding = rounding; this.shardRounding = shardRounding; + this.offset = offset; this.order = InternalOrder.validate(order, this); this.keyed = keyed; this.minDocCount = minDocCount; @@ -124,8 +126,8 @@ public void collect(int doc, long bucket) throws IOException { // The encoding should ensure that this assert is always true. assert from >= previousFrom : "Start of range not >= previous start"; final Long to = (Long) range.getTo(); - final long startKey = shardRounding.round(from); - final long endKey = shardRounding.round(to); + final long startKey = offsetAwareRounding(shardRounding, from, offset); + final long endKey = offsetAwareRounding(shardRounding, to, offset); for (long key = startKey > previousKey ? startKey : previousKey; key <= endKey; key = shardRounding.nextRoundingValue(key)) { if (key == previousKey) { @@ -151,6 +153,10 @@ public void collect(int doc, long bucket) throws IOException { }; } + private long offsetAwareRounding(Rounding rounding, long value, long offset) { + return rounding.round(value - offset) + offset; + } + @Override public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOException { assert owningBucketOrdinal == 0; @@ -169,7 +175,7 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, buckets, order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, keyed, + return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, formatter, keyed, pipelineAggregators(), metaData()); } @@ -178,8 +184,8 @@ public InternalAggregation buildEmptyAggregation() { InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, - keyed, pipelineAggregators(), metaData()); + return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, offset, emptyBucketInfo, formatter, keyed, + pipelineAggregators(), metaData()); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java index 4a9deb9bdedfc..dc20ff291e0d1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java @@ -166,11 +166,7 @@ ExtendedBounds parseAndValidate(String aggName, QueryShardContext queryShardCont } ExtendedBounds round(Rounding rounding) { - // Extended bounds shouldn't be effected by the offset - Rounding effectiveRounding = rounding.withoutOffset(); - return new ExtendedBounds( - min != null ? effectiveRounding.round(min) : null, - max != null ? effectiveRounding.round(max) : null); + return new ExtendedBounds(min != null ? rounding.round(min) : null, max != null ? rounding.round(max) : null); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java index af30108dbdfb7..b4e0ba659aff5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java @@ -497,7 +497,7 @@ public Number getKey(MultiBucketsAggregation.Bucket bucket) { @Override public Number nextKey(Number key) { - return emptyBucketInfo.rounding.nextRoundingValue(key.longValue()); + return emptyBucketInfo.rounding.nextRoundingValue(key.longValue() - offset) + offset; } @Override diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index 8e19bdaf5547a..7de894d081f21 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -201,18 +201,10 @@ public void testOffsetRounding() { Rounding rounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).offset(twoHours).build(); assertThat(rounding.round(0), equalTo(-oneDay + twoHours)); assertThat(rounding.round(twoHours), equalTo(twoHours)); - assertThat(rounding.nextRoundingValue(-oneDay), equalTo(-oneDay + twoHours)); - assertThat(rounding.nextRoundingValue(0), equalTo(twoHours)); - assertThat(rounding.withoutOffset().round(0), equalTo(0L)); - assertThat(rounding.withoutOffset().nextRoundingValue(0), equalTo(oneDay)); rounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).offset(-twoHours).build(); assertThat(rounding.round(0), equalTo(-twoHours)); assertThat(rounding.round(oneDay - twoHours), equalTo(oneDay - twoHours)); - assertThat(rounding.nextRoundingValue(-oneDay), equalTo(-twoHours)); - assertThat(rounding.nextRoundingValue(0), equalTo(oneDay - twoHours)); - assertThat(rounding.withoutOffset().round(0), equalTo(0L)); - assertThat(rounding.withoutOffset().nextRoundingValue(0), equalTo(oneDay)); } /** diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java index 9a2d5a411d4ad..ddfd2c8c82c31 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java @@ -164,7 +164,7 @@ static AggregatorFactory getRandomSequentiallyOrderedParentAgg() throws IOExcept new AggregatorFactories.Builder(), Collections.emptyMap()); break; case 1: - factory = new DateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), + factory = new DateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), 0L, mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class), mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class), new AggregatorFactories.Builder(), Collections.emptyMap()); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java index 9f49588d7fc14..8f5cfb661927d 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java @@ -131,7 +131,7 @@ public void testParentValidations() throws IOException { // Date Histogram aggBuilders.clear(); aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum")); - parent = new DateHistogramAggregatorFactory("name", valuesSourceConfig, + parent = new DateHistogramAggregatorFactory("name", valuesSourceConfig, 0L, mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class), mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class), new AggregatorFactories.Builder(), Collections.emptyMap());