Skip to content

Commit 8550b9e

Browse files
colings86colings86
committed
Aggregations: Fixes pre and post offset serialisation for histogram aggs
Changes the serialisation of pre and post offset to use Long instead of VLong so that negative values are supported. This actually only showed up in the case where minDocCount=0 as the rounding is only serialised in this case. Closes #7312
1 parent f956920 commit 8550b9e

File tree

5 files changed

+299
-83
lines changed

5 files changed

+299
-83
lines changed

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.elasticsearch.common.rounding;
2020

2121
import org.elasticsearch.ElasticsearchException;
22+
import org.elasticsearch.Version;
2223
import org.elasticsearch.common.io.stream.StreamInput;
2324
import org.elasticsearch.common.io.stream.StreamOutput;
2425
import org.elasticsearch.common.io.stream.Streamable;
@@ -218,15 +219,25 @@ public long nextRoundingValue(long value) {
218219
@Override
219220
public void readFrom(StreamInput in) throws IOException {
220221
rounding = Rounding.Streams.read(in);
221-
preOffset = in.readVLong();
222-
postOffset = in.readVLong();
222+
if (in.getVersion().before(Version.V_1_4_0)) {
223+
preOffset = in.readVLong();
224+
postOffset = in.readVLong();
225+
} else {
226+
preOffset = in.readLong();
227+
postOffset = in.readLong();
228+
}
223229
}
224230

225231
@Override
226232
public void writeTo(StreamOutput out) throws IOException {
227233
Rounding.Streams.write(rounding, out);
228-
out.writeVLong(preOffset);
229-
out.writeVLong(postOffset);
234+
if (out.getVersion().before(Version.V_1_4_0)) {
235+
out.writeVLong(preOffset);
236+
out.writeVLong(postOffset);
237+
} else {
238+
out.writeLong(preOffset);
239+
out.writeLong(postOffset);
240+
}
230241
}
231242
}
232243

‎src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramOffsetTests.java‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.search.aggregations.bucket;
20+
21+
import org.elasticsearch.Version;
22+
import org.elasticsearch.action.index.IndexRequestBuilder;
23+
import org.elasticsearch.action.search.SearchResponse;
24+
import org.elasticsearch.common.settings.ImmutableSettings;
25+
import org.elasticsearch.common.settings.Settings;
26+
import org.elasticsearch.index.mapper.core.DateFieldMapper;
27+
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogram;
28+
import org.elasticsearch.test.ElasticsearchIntegrationTest;
29+
import org.elasticsearch.test.transport.AssertingLocalTransport;
30+
import org.hamcrest.Matchers;
31+
import org.joda.time.DateTime;
32+
import org.junit.After;
33+
import org.junit.Test;
34+
35+
import java.io.IOException;
36+
import java.util.Collection;
37+
38+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
39+
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
40+
import static org.elasticsearch.search.aggregations.AggregationBuilders.dateHistogram;
41+
import static org.hamcrest.Matchers.equalTo;
42+
43+
/**
44+
* The serialisation of pre and post offsets for the date histogram aggregation was corrected in version 1.4 to allow negative offsets and as such the
45+
* serialisation of negative offsets in these tests would break in pre 1.4 versions. These tests are separated from the other DateHistogramTests so the
46+
* AssertingLocalTransport for these tests can be set to only use versions 1.4 onwards while keeping the other tests using all versions
47+
*/
48+
@ElasticsearchIntegrationTest.SuiteScopeTest
49+
@ElasticsearchIntegrationTest.ClusterScope(scope=ElasticsearchIntegrationTest.Scope.SUITE)
50+
public class DateHistogramOffsetTests extends ElasticsearchIntegrationTest {
51+
52+
private DateTime date(String date) {
53+
return DateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date);
54+
}
55+
56+
@Override
57+
protected Settings nodeSettings(int nodeOrdinal) {
58+
return ImmutableSettings.builder()
59+
.put(AssertingLocalTransport.ASSERTING_TRANSPORT_MIN_VERSION_KEY, Version.V_1_4_0).build();
60+
}
61+
62+
@After
63+
public void afterEachTest() throws IOException {
64+
internalCluster().wipeIndices("idx2");
65+
}
66+
67+
@Test
68+
public void singleValue_WithPreOffset() throws Exception {
69+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
70+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
71+
DateTime date = date("2014-03-11T00:00:00+00:00");
72+
for (int i = 0; i < reqs.length; i++) {
73+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
74+
date = date.plusHours(1);
75+
}
76+
indexRandom(true, reqs);
77+
78+
SearchResponse response = client().prepareSearch("idx2")
79+
.setQuery(matchAllQuery())
80+
.addAggregation(dateHistogram("date_histo")
81+
.field("date")
82+
.preOffset("-2h")
83+
.interval(DateHistogram.Interval.DAY)
84+
.format("yyyy-MM-dd"))
85+
.execute().actionGet();
86+
87+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
88+
89+
DateHistogram histo = response.getAggregations().get("date_histo");
90+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
91+
assertThat(buckets.size(), equalTo(2));
92+
93+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-10");
94+
assertThat(bucket, Matchers.notNullValue());
95+
assertThat(bucket.getDocCount(), equalTo(2l));
96+
97+
bucket = histo.getBucketByKey("2014-03-11");
98+
assertThat(bucket, Matchers.notNullValue());
99+
assertThat(bucket.getDocCount(), equalTo(3l));
100+
}
101+
102+
@Test
103+
public void singleValue_WithPreOffset_MinDocCount() throws Exception {
104+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
105+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
106+
DateTime date = date("2014-03-11T00:00:00+00:00");
107+
for (int i = 0; i < reqs.length; i++) {
108+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
109+
date = date.plusHours(1);
110+
}
111+
indexRandom(true, reqs);
112+
113+
SearchResponse response = client().prepareSearch("idx2")
114+
.setQuery(matchAllQuery())
115+
.addAggregation(dateHistogram("date_histo")
116+
.field("date")
117+
.preOffset("-2h")
118+
.minDocCount(0)
119+
.interval(DateHistogram.Interval.DAY)
120+
.format("yyyy-MM-dd"))
121+
.execute().actionGet();
122+
123+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
124+
125+
DateHistogram histo = response.getAggregations().get("date_histo");
126+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
127+
assertThat(buckets.size(), equalTo(2));
128+
129+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-10");
130+
assertThat(bucket, Matchers.notNullValue());
131+
assertThat(bucket.getDocCount(), equalTo(2l));
132+
133+
bucket = histo.getBucketByKey("2014-03-11");
134+
assertThat(bucket, Matchers.notNullValue());
135+
assertThat(bucket.getDocCount(), equalTo(3l));
136+
}
137+
138+
@Test
139+
public void singleValue_WithPostOffset() throws Exception {
140+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
141+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
142+
DateTime date = date("2014-03-11T00:00:00+00:00");
143+
for (int i = 0; i < reqs.length; i++) {
144+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
145+
date = date.plusHours(6);
146+
}
147+
indexRandom(true, reqs);
148+
149+
SearchResponse response = client().prepareSearch("idx2")
150+
.setQuery(matchAllQuery())
151+
.addAggregation(dateHistogram("date_histo")
152+
.field("date")
153+
.postOffset("2d")
154+
.interval(DateHistogram.Interval.DAY)
155+
.format("yyyy-MM-dd"))
156+
.execute().actionGet();
157+
158+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
159+
160+
DateHistogram histo = response.getAggregations().get("date_histo");
161+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
162+
assertThat(buckets.size(), equalTo(2));
163+
164+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-13");
165+
assertThat(bucket, Matchers.notNullValue());
166+
assertThat(bucket.getDocCount(), equalTo(4l));
167+
168+
bucket = histo.getBucketByKey("2014-03-14");
169+
assertThat(bucket, Matchers.notNullValue());
170+
assertThat(bucket.getDocCount(), equalTo(1l));
171+
}
172+
173+
@Test
174+
public void singleValue_WithPostOffset_MinDocCount() throws Exception {
175+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
176+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
177+
DateTime date = date("2014-03-11T00:00:00+00:00");
178+
for (int i = 0; i < reqs.length; i++) {
179+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
180+
date = date.plusHours(6);
181+
}
182+
indexRandom(true, reqs);
183+
184+
SearchResponse response = client().prepareSearch("idx2")
185+
.setQuery(matchAllQuery())
186+
.addAggregation(dateHistogram("date_histo")
187+
.field("date")
188+
.postOffset("2d")
189+
.minDocCount(0)
190+
.interval(DateHistogram.Interval.DAY)
191+
.format("yyyy-MM-dd"))
192+
.execute().actionGet();
193+
194+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
195+
196+
DateHistogram histo = response.getAggregations().get("date_histo");
197+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
198+
assertThat(buckets.size(), equalTo(2));
199+
200+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-13");
201+
assertThat(bucket, Matchers.notNullValue());
202+
assertThat(bucket.getDocCount(), equalTo(4l));
203+
204+
bucket = histo.getBucketByKey("2014-03-14");
205+
assertThat(bucket, Matchers.notNullValue());
206+
assertThat(bucket.getDocCount(), equalTo(1l));
207+
}
208+
}

‎src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramTests.java‎

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,76 +1051,6 @@ public void singleValue_WithPreZone() throws Exception {
10511051
assertThat(bucket.getDocCount(), equalTo(3l));
10521052
}
10531053

1054-
@Test
1055-
public void singleValue_WithPreOffset() throws Exception {
1056-
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
1057-
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
1058-
DateTime date = date("2014-03-11T00:00:00+00:00");
1059-
for (int i = 0; i < reqs.length; i++) {
1060-
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
1061-
date = date.plusHours(1);
1062-
}
1063-
indexRandom(true, reqs);
1064-
1065-
SearchResponse response = client().prepareSearch("idx2")
1066-
.setQuery(matchAllQuery())
1067-
.addAggregation(dateHistogram("date_histo")
1068-
.field("date")
1069-
.preOffset("-2h")
1070-
.interval(DateHistogram.Interval.DAY)
1071-
.format("yyyy-MM-dd"))
1072-
.execute().actionGet();
1073-
1074-
assertThat(response.getHits().getTotalHits(), equalTo(5l));
1075-
1076-
DateHistogram histo = response.getAggregations().get("date_histo");
1077-
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
1078-
assertThat(buckets.size(), equalTo(2));
1079-
1080-
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-10");
1081-
assertThat(bucket, Matchers.notNullValue());
1082-
assertThat(bucket.getDocCount(), equalTo(2l));
1083-
1084-
bucket = histo.getBucketByKey("2014-03-11");
1085-
assertThat(bucket, Matchers.notNullValue());
1086-
assertThat(bucket.getDocCount(), equalTo(3l));
1087-
}
1088-
1089-
@Test
1090-
public void singleValue_WithPostOffset() throws Exception {
1091-
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
1092-
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
1093-
DateTime date = date("2014-03-11T00:00:00+00:00");
1094-
for (int i = 0; i < reqs.length; i++) {
1095-
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
1096-
date = date.plusHours(6);
1097-
}
1098-
indexRandom(true, reqs);
1099-
1100-
SearchResponse response = client().prepareSearch("idx2")
1101-
.setQuery(matchAllQuery())
1102-
.addAggregation(dateHistogram("date_histo")
1103-
.field("date")
1104-
.postOffset("2d")
1105-
.interval(DateHistogram.Interval.DAY)
1106-
.format("yyyy-MM-dd"))
1107-
.execute().actionGet();
1108-
1109-
assertThat(response.getHits().getTotalHits(), equalTo(5l));
1110-
1111-
DateHistogram histo = response.getAggregations().get("date_histo");
1112-
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
1113-
assertThat(buckets.size(), equalTo(2));
1114-
1115-
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-13");
1116-
assertThat(bucket, Matchers.notNullValue());
1117-
assertThat(bucket.getDocCount(), equalTo(4l));
1118-
1119-
bucket = histo.getBucketByKey("2014-03-14");
1120-
assertThat(bucket, Matchers.notNullValue());
1121-
assertThat(bucket.getDocCount(), equalTo(1l));
1122-
}
1123-
11241054
@Test
11251055
public void singleValue_WithPreZone_WithAadjustLargeInterval() throws Exception {
11261056
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();

0 commit comments

Comments
 (0)