Skip to content

Commit b3a7728

Browse files
nik9000nik9000
authored
Begin moving date_histogram to offset rounding (#50873)
We added a new rounding in #50609 that handles offsets to the start and end of the rounding so that we could support `offset` in the `composite` aggregation. This starts moving `date_histogram` to that new offset.
1 parent 375c140 commit b3a7728

File tree

10 files changed

+77
-33
lines changed

10 files changed

+77
-33
lines changed

‎server/src/main/java/org/elasticsearch/common/Rounding.java‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,19 @@ public void writeTo(StreamOutput out) throws IOException {
164164
*/
165165
public abstract long nextRoundingValue(long value);
166166

167+
/**
168+
* How "offset" this rounding is from the traditional "start" of the period.
169+
* @deprecated We're in the process of abstracting offset *into* Rounding
170+
* so keep any usage to migratory shims
171+
*/
172+
@Deprecated
173+
public abstract long offset();
174+
175+
/**
176+
* Strip the {@code offset} from these bounds.
177+
*/
178+
public abstract Rounding withoutOffset();
179+
167180
@Override
168181
public abstract boolean equals(Object obj);
169182

@@ -420,6 +433,16 @@ public long nextRoundingValue(long utcMillis) {
420433
}
421434
}
422435

436+
@Override
437+
public long offset() {
438+
return 0;
439+
}
440+
441+
@Override
442+
public Rounding withoutOffset() {
443+
return this;
444+
}
445+
423446
@Override
424447
public int hashCode() {
425448
return Objects.hash(unit, timeZone);
@@ -546,6 +569,16 @@ public long nextRoundingValue(long time) {
546569
.toInstant().toEpochMilli();
547570
}
548571

572+
@Override
573+
public long offset() {
574+
return 0;
575+
}
576+
577+
@Override
578+
public Rounding withoutOffset() {
579+
return this;
580+
}
581+
549582
@Override
550583
public int hashCode() {
551584
return Objects.hash(interval, timeZone);
@@ -607,8 +640,17 @@ public long round(long value) {
607640

608641
@Override
609642
public long nextRoundingValue(long value) {
610-
// This isn't needed by the current users. We'll implement it when we migrate other users to it.
611-
throw new UnsupportedOperationException("not yet supported");
643+
return delegate.nextRoundingValue(value - offset) + offset;
644+
}
645+
646+
@Override
647+
public long offset() {
648+
return offset;
649+
}
650+
651+
@Override
652+
public Rounding withoutOffset() {
653+
return delegate;
612654
}
613655

614656
@Override

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -494,21 +494,21 @@ protected ValuesSourceAggregatorFactory<ValuesSource> innerBuild(QueryShardConte
494494
Builder subFactoriesBuilder) throws IOException {
495495
final ZoneId tz = timeZone();
496496
// TODO use offset here rather than explicitly in the aggregation
497-
final Rounding rounding = dateHistogramInterval.createRounding(tz, 0);
497+
final Rounding rounding = dateHistogramInterval.createRounding(tz, offset);
498498
final ZoneId rewrittenTimeZone = rewriteTimeZone(queryShardContext);
499499
final Rounding shardRounding;
500500
if (tz == rewrittenTimeZone) {
501501
shardRounding = rounding;
502502
} else {
503-
shardRounding = dateHistogramInterval.createRounding(rewrittenTimeZone, 0);
503+
shardRounding = dateHistogramInterval.createRounding(rewrittenTimeZone, offset);
504504
}
505505

506506
ExtendedBounds roundedBounds = null;
507507
if (this.extendedBounds != null) {
508508
// parse any string bounds to longs and round
509509
roundedBounds = this.extendedBounds.parseAndValidate(name, queryShardContext, config.format()).round(rounding);
510510
}
511-
return new DateHistogramAggregatorFactory(name, config, offset, order, keyed, minDocCount,
511+
return new DateHistogramAggregatorFactory(name, config, order, keyed, minDocCount,
512512
rounding, shardRounding, roundedBounds, queryShardContext, parent, subFactoriesBuilder, metaData);
513513
}
514514

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,16 @@ class DateHistogramAggregator extends BucketsAggregator {
6464
private final ExtendedBounds extendedBounds;
6565

6666
private final LongHash bucketOrds;
67-
private long offset;
6867

6968
DateHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, Rounding shardRounding,
70-
long offset, BucketOrder order, boolean keyed,
69+
BucketOrder order, boolean keyed,
7170
long minDocCount, @Nullable ExtendedBounds extendedBounds, @Nullable ValuesSource.Numeric valuesSource,
7271
DocValueFormat formatter, SearchContext aggregationContext,
7372
Aggregator parent, List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
7473

7574
super(name, factories, aggregationContext, parent, pipelineAggregators, metaData);
7675
this.rounding = rounding;
7776
this.shardRounding = shardRounding;
78-
this.offset = offset;
7977
this.order = InternalOrder.validate(order, this);
8078
this.keyed = keyed;
8179
this.minDocCount = minDocCount;
@@ -113,7 +111,7 @@ public void collect(int doc, long bucket) throws IOException {
113111
long value = values.nextValue();
114112
// We can use shardRounding here, which is sometimes more efficient
115113
// if daylight saving times are involved.
116-
long rounded = shardRounding.round(value - offset) + offset;
114+
long rounded = shardRounding.round(value);
117115
assert rounded >= previousRounded;
118116
if (rounded == previousRounded) {
119117
continue;
@@ -150,7 +148,7 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE
150148
InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0
151149
? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds)
152150
: null;
153-
return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, formatter, keyed,
151+
return new InternalDateHistogram(name, buckets, order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, keyed,
154152
pipelineAggregators(), metaData());
155153
}
156154

@@ -159,8 +157,8 @@ public InternalAggregation buildEmptyAggregation() {
159157
InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0
160158
? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds)
161159
: null;
162-
return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, offset, emptyBucketInfo, formatter, keyed,
163-
pipelineAggregators(), metaData());
160+
return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, rounding.offset(), emptyBucketInfo, formatter,
161+
keyed, pipelineAggregators(), metaData());
164162
}
165163

166164
@Override

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
public final class DateHistogramAggregatorFactory
4040
extends ValuesSourceAggregatorFactory<ValuesSource> {
4141

42-
private final long offset;
4342
private final BucketOrder order;
4443
private final boolean keyed;
4544
private final long minDocCount;
@@ -48,12 +47,11 @@ public final class DateHistogramAggregatorFactory
4847
private final Rounding shardRounding;
4948

5049
public DateHistogramAggregatorFactory(String name, ValuesSourceConfig<ValuesSource> config,
51-
long offset, BucketOrder order, boolean keyed, long minDocCount,
50+
BucketOrder order, boolean keyed, long minDocCount,
5251
Rounding rounding, Rounding shardRounding, ExtendedBounds extendedBounds, QueryShardContext queryShardContext,
5352
AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
5453
Map<String, Object> metaData) throws IOException {
5554
super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData);
56-
this.offset = offset;
5755
this.order = order;
5856
this.keyed = keyed;
5957
this.minDocCount = minDocCount;
@@ -104,7 +102,7 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource,
104102
private Aggregator createAggregator(ValuesSource.Numeric valuesSource, SearchContext searchContext,
105103
Aggregator parent, List<PipelineAggregator> pipelineAggregators,
106104
Map<String, Object> metaData) throws IOException {
107-
return new DateHistogramAggregator(name, factories, rounding, shardRounding, offset, order, keyed, minDocCount, extendedBounds,
105+
return new DateHistogramAggregator(name, factories, rounding, shardRounding, order, keyed, minDocCount, extendedBounds,
108106
valuesSource, config.format(), searchContext, parent, pipelineAggregators, metaData);
109107
}
110108

@@ -113,7 +111,7 @@ private Aggregator createRangeAggregator(ValuesSource.Range valuesSource,
113111
Aggregator parent,
114112
List<PipelineAggregator> pipelineAggregators,
115113
Map<String, Object> metaData) throws IOException {
116-
return new DateRangeHistogramAggregator(name, factories, rounding, shardRounding, offset, order, keyed, minDocCount, extendedBounds,
114+
return new DateRangeHistogramAggregator(name, factories, rounding, shardRounding, order, keyed, minDocCount, extendedBounds,
117115
valuesSource, config.format(), searchContext, parent, pipelineAggregators, metaData);
118116
}
119117

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateRangeHistogramAggregator.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ class DateRangeHistogramAggregator extends BucketsAggregator {
6767
private final ExtendedBounds extendedBounds;
6868

6969
private final LongHash bucketOrds;
70-
private long offset;
7170

7271
DateRangeHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, Rounding shardRounding,
73-
long offset, BucketOrder order, boolean keyed,
72+
BucketOrder order, boolean keyed,
7473
long minDocCount, @Nullable ExtendedBounds extendedBounds, @Nullable ValuesSource.Range valuesSource,
7574
DocValueFormat formatter, SearchContext aggregationContext,
7675
Aggregator parent, List<PipelineAggregator> pipelineAggregators,
@@ -79,7 +78,6 @@ class DateRangeHistogramAggregator extends BucketsAggregator {
7978
super(name, factories, aggregationContext, parent, pipelineAggregators, metaData);
8079
this.rounding = rounding;
8180
this.shardRounding = shardRounding;
82-
this.offset = offset;
8381
this.order = InternalOrder.validate(order, this);
8482
this.keyed = keyed;
8583
this.minDocCount = minDocCount;
@@ -126,8 +124,8 @@ public void collect(int doc, long bucket) throws IOException {
126124
// The encoding should ensure that this assert is always true.
127125
assert from >= previousFrom : "Start of range not >= previous start";
128126
final Long to = (Long) range.getTo();
129-
final long startKey = offsetAwareRounding(shardRounding, from, offset);
130-
final long endKey = offsetAwareRounding(shardRounding, to, offset);
127+
final long startKey = shardRounding.round(from);
128+
final long endKey = shardRounding.round(to);
131129
for (long key = startKey > previousKey ? startKey : previousKey; key <= endKey;
132130
key = shardRounding.nextRoundingValue(key)) {
133131
if (key == previousKey) {
@@ -153,10 +151,6 @@ public void collect(int doc, long bucket) throws IOException {
153151
};
154152
}
155153

156-
private long offsetAwareRounding(Rounding rounding, long value, long offset) {
157-
return rounding.round(value - offset) + offset;
158-
}
159-
160154
@Override
161155
public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOException {
162156
assert owningBucketOrdinal == 0;
@@ -175,7 +169,7 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE
175169
InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0
176170
? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds)
177171
: null;
178-
return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, formatter, keyed,
172+
return new InternalDateHistogram(name, buckets, order, minDocCount, rounding.offset(), emptyBucketInfo, formatter, keyed,
179173
pipelineAggregators(), metaData());
180174
}
181175

@@ -184,8 +178,8 @@ public InternalAggregation buildEmptyAggregation() {
184178
InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0
185179
? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds)
186180
: null;
187-
return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, offset, emptyBucketInfo, formatter, keyed,
188-
pipelineAggregators(), metaData());
181+
return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, rounding.offset(), emptyBucketInfo, formatter,
182+
keyed, pipelineAggregators(), metaData());
189183
}
190184

191185
@Override

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBounds.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,11 @@ ExtendedBounds parseAndValidate(String aggName, QueryShardContext queryShardCont
166166
}
167167

168168
ExtendedBounds round(Rounding rounding) {
169-
return new ExtendedBounds(min != null ? rounding.round(min) : null, max != null ? rounding.round(max) : null);
169+
// Extended bounds shouldn't be effected by the offset
170+
Rounding effectiveRounding = rounding.withoutOffset();
171+
return new ExtendedBounds(
172+
min != null ? effectiveRounding.round(min) : null,
173+
max != null ? effectiveRounding.round(max) : null);
170174
}
171175

172176
@Override

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ public Number getKey(MultiBucketsAggregation.Bucket bucket) {
497497

498498
@Override
499499
public Number nextKey(Number key) {
500-
return emptyBucketInfo.rounding.nextRoundingValue(key.longValue() - offset) + offset;
500+
return emptyBucketInfo.rounding.nextRoundingValue(key.longValue());
501501
}
502502

503503
@Override

‎server/src/test/java/org/elasticsearch/common/RoundingTests.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,18 @@ public void testOffsetRounding() {
201201
Rounding rounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).offset(twoHours).build();
202202
assertThat(rounding.round(0), equalTo(-oneDay + twoHours));
203203
assertThat(rounding.round(twoHours), equalTo(twoHours));
204+
assertThat(rounding.nextRoundingValue(-oneDay), equalTo(-oneDay + twoHours));
205+
assertThat(rounding.nextRoundingValue(0), equalTo(twoHours));
206+
assertThat(rounding.withoutOffset().round(0), equalTo(0L));
207+
assertThat(rounding.withoutOffset().nextRoundingValue(0), equalTo(oneDay));
204208

205209
rounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).offset(-twoHours).build();
206210
assertThat(rounding.round(0), equalTo(-twoHours));
207211
assertThat(rounding.round(oneDay - twoHours), equalTo(oneDay - twoHours));
212+
assertThat(rounding.nextRoundingValue(-oneDay), equalTo(-twoHours));
213+
assertThat(rounding.nextRoundingValue(0), equalTo(oneDay - twoHours));
214+
assertThat(rounding.withoutOffset().round(0), equalTo(0L));
215+
assertThat(rounding.withoutOffset().nextRoundingValue(0), equalTo(oneDay));
208216
}
209217

210218
/**

‎server/src/test/java/org/elasticsearch/search/aggregations/pipeline/PipelineAggregationHelperTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ static AggregatorFactory getRandomSequentiallyOrderedParentAgg() throws IOExcept
164164
new AggregatorFactories.Builder(), Collections.emptyMap());
165165
break;
166166
case 1:
167-
factory = new DateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class), 0L,
167+
factory = new DateHistogramAggregatorFactory("name", mock(ValuesSourceConfig.class),
168168
mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class),
169169
mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class),
170170
new AggregatorFactories.Builder(), Collections.emptyMap());

‎x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityAggregatorTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public void testParentValidations() throws IOException {
131131
// Date Histogram
132132
aggBuilders.clear();
133133
aggBuilders.add(new CumulativeCardinalityPipelineAggregationBuilder("cumulative_card", "sum"));
134-
parent = new DateHistogramAggregatorFactory("name", valuesSource, 0L,
134+
parent = new DateHistogramAggregatorFactory("name", valuesSource,
135135
mock(InternalOrder.class), false, 0L, mock(Rounding.class), mock(Rounding.class),
136136
mock(ExtendedBounds.class), mock(QueryShardContext.class), mock(AggregatorFactory.class),
137137
new AggregatorFactories.Builder(), Collections.emptyMap());

0 commit comments

Comments
 (0)