Skip to content

Commit bebc168

Browse files
committed
Fixed a bug in date_histogram aggregation parsing
- pre_zone_adjust_large_interval was not parsed properly - added tests for pre_zone and pre_zone_adjust_large_interval - changed DateHistogram#getBucketByKey(String) to support date formats (next to numeric strings) - added randomized testing for fetching the bucket by key in date_histogram tests - added missing "format" support in DateHistogramBuilder Closes #5375
1 parent d027f71 commit bebc168

File tree

5 files changed

+144
-13
lines changed

5 files changed

+144
-13
lines changed

src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramBuilder.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public class DateHistogramBuilder extends ValuesSourceAggregationBuilder<DateHis
3737
private String preZone;
3838
private String postZone;
3939
private boolean preZoneAdjustLargeInterval;
40+
private String format;
4041
long preOffset = 0;
4142
long postOffset = 0;
4243
float factor = 1.0f;
@@ -95,6 +96,11 @@ public DateHistogramBuilder factor(float factor) {
9596
return this;
9697
}
9798

99+
public DateHistogramBuilder format(String format) {
100+
this.format = format;
101+
return this;
102+
}
103+
98104
@Override
99105
protected XContentBuilder doInternalXContent(XContentBuilder builder, Params params) throws IOException {
100106
if (interval == null) {
@@ -138,6 +144,10 @@ protected XContentBuilder doInternalXContent(XContentBuilder builder, Params par
138144
builder.field("factor", factor);
139145
}
140146

147+
if (format != null) {
148+
builder.field("format", format);
149+
}
150+
141151
return builder;
142152
}
143153

src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,6 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se
113113
preZone = parseZone(parser.text());
114114
} else if ("pre_zone".equals(currentFieldName) || "preZone".equals(currentFieldName)) {
115115
preZone = parseZone(parser.text());
116-
} else if ("pre_zone_adjust_large_interval".equals(currentFieldName) || "preZoneAdjustLargeInterval".equals(currentFieldName)) {
117-
preZoneAdjustLargeInterval = parser.booleanValue();
118116
} else if ("post_zone".equals(currentFieldName) || "postZone".equals(currentFieldName)) {
119117
postZone = parseZone(parser.text());
120118
} else if ("pre_offset".equals(currentFieldName) || "preOffset".equals(currentFieldName)) {
@@ -133,6 +131,8 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se
133131
keyed = parser.booleanValue();
134132
} else if ("script_values_sorted".equals(currentFieldName) || "scriptValuesSorted".equals(currentFieldName)) {
135133
assumeSorted = parser.booleanValue();
134+
} else if ("pre_zone_adjust_large_interval".equals(currentFieldName) || "preZoneAdjustLargeInterval".equals(currentFieldName)) {
135+
preZoneAdjustLargeInterval = parser.booleanValue();
136136
} else {
137137
throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
138138
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.elasticsearch.search.aggregations.bucket.histogram;
2020

21+
import com.carrotsearch.hppc.ObjectObjectOpenHashMap;
2122
import org.elasticsearch.common.io.stream.StreamInput;
2223
import org.elasticsearch.index.mapper.core.DateFieldMapper;
2324
import org.elasticsearch.search.aggregations.AggregationStreams;
@@ -91,6 +92,8 @@ public InternalDateHistogram.Bucket createBucket(long key, long docCount, Intern
9192
}
9293
}
9394

95+
private ObjectObjectOpenHashMap<String, InternalDateHistogram.Bucket> bucketsMap;
96+
9497
InternalDateHistogram() {} // for serialization
9598

9699
InternalDateHistogram(String name, List<InternalDateHistogram.Bucket> buckets, InternalOrder order, long minDocCount,
@@ -103,6 +106,23 @@ public Type type() {
103106
return TYPE;
104107
}
105108

109+
@Override
110+
public Bucket getBucketByKey(String key) {
111+
try {
112+
long time = Long.parseLong(key);
113+
return super.getBucketByKey(time);
114+
} catch (NumberFormatException nfe) {
115+
// it's not a number, so lets try to parse it as a date using the formatter.
116+
}
117+
if (bucketsMap == null) {
118+
bucketsMap = new ObjectObjectOpenHashMap<String, InternalDateHistogram.Bucket>();
119+
for (InternalDateHistogram.Bucket bucket : buckets) {
120+
bucketsMap.put(bucket.getKey(), bucket);
121+
}
122+
}
123+
return bucketsMap.get(key);
124+
}
125+
106126
@Override
107127
public DateHistogram.Bucket getBucketByKey(DateTime key) {
108128
return getBucketByKey(key.getMillis());
@@ -112,4 +132,11 @@ public DateHistogram.Bucket getBucketByKey(DateTime key) {
112132
protected InternalDateHistogram.Bucket createBucket(long key, long docCount, InternalAggregations aggregations, ValueFormatter formatter) {
113133
return new Bucket(key, docCount, aggregations, formatter);
114134
}
135+
136+
@Override
137+
public void readFrom(StreamInput in) throws IOException {
138+
super.readFrom(in);
139+
bucketsMap = null; // we need to reset this on read (as it's lazily created on demand)
140+
}
141+
115142
}

src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public B createBucket(long key, long docCount, InternalAggregations aggregations
161161

162162
}
163163

164-
private List<B> buckets;
164+
protected List<B> buckets;
165165
private LongObjectOpenHashMap<B> bucketsMap;
166166
private InternalOrder order;
167167
private ValueFormatter formatter;

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

Lines changed: 104 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020

2121
import org.elasticsearch.action.index.IndexRequestBuilder;
2222
import org.elasticsearch.action.search.SearchResponse;
23+
import org.elasticsearch.common.joda.Joda;
2324
import org.elasticsearch.common.settings.ImmutableSettings;
2425
import org.elasticsearch.common.settings.Settings;
2526
import org.elasticsearch.common.xcontent.XContentBuilder;
27+
import org.elasticsearch.index.mapper.core.DateFieldMapper;
2628
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
2729
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogram;
2830
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
@@ -37,6 +39,7 @@
3739

3840
import java.io.IOException;
3941
import java.util.ArrayList;
42+
import java.util.Collection;
4043
import java.util.List;
4144

4245
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -64,6 +67,10 @@ private DateTime date(int month, int day) {
6467
return new DateTime(2012, month, day, 0, 0, DateTimeZone.UTC);
6568
}
6669

70+
private DateTime date(String date) {
71+
return DateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date);
72+
}
73+
6774
private IndexRequestBuilder indexDoc(int month, int day, int value) throws Exception {
6875
return client().prepareIndex("idx", "type").setSource(jsonBuilder()
6976
.startObject()
@@ -88,6 +95,23 @@ public void init() throws Exception {
8895
ensureSearchable();
8996
}
9097

98+
private static DateHistogram.Bucket getBucket(DateHistogram histogram, DateTime key) {
99+
return getBucket(histogram, key, DateFieldMapper.Defaults.DATE_TIME_FORMATTER.format());
100+
}
101+
102+
private static DateHistogram.Bucket getBucket(DateHistogram histogram, DateTime key, String format) {
103+
if (randomBoolean()) {
104+
if (randomBoolean()) {
105+
return histogram.getBucketByKey(key);
106+
}
107+
return histogram.getBucketByKey(key.getMillis());
108+
}
109+
if (randomBoolean()) {
110+
return histogram.getBucketByKey("" + key.getMillis());
111+
}
112+
return histogram.getBucketByKey(Joda.forPattern(format).printer().print(key));
113+
}
114+
91115
@Test
92116
public void singleValuedField() throws Exception {
93117
SearchResponse response = client().prepareSearch("idx")
@@ -102,22 +126,22 @@ public void singleValuedField() throws Exception {
102126
assertThat(histo.getName(), equalTo("histo"));
103127
assertThat(histo.getBuckets().size(), equalTo(3));
104128

105-
long key = new DateTime(2012, 1, 1, 0, 0, DateTimeZone.UTC).getMillis();
106-
DateHistogram.Bucket bucket = histo.getBucketByKey(key);
129+
DateTime key = new DateTime(2012, 1, 1, 0, 0, DateTimeZone.UTC);
130+
DateHistogram.Bucket bucket = getBucket(histo, key);
107131
assertThat(bucket, notNullValue());
108-
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key));
132+
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
109133
assertThat(bucket.getDocCount(), equalTo(1l));
110134

111-
key = new DateTime(2012, 2, 1, 0, 0, DateTimeZone.UTC).getMillis();
112-
bucket = histo.getBucketByKey(key);
135+
key = new DateTime(2012, 2, 1, 0, 0, DateTimeZone.UTC);
136+
bucket = getBucket(histo, key);
113137
assertThat(bucket, notNullValue());
114-
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key));
138+
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
115139
assertThat(bucket.getDocCount(), equalTo(2l));
116140

117-
key = new DateTime(2012, 3, 1, 0, 0, DateTimeZone.UTC).getMillis();
118-
bucket = histo.getBucketByKey(key);
141+
key = new DateTime(2012, 3, 1, 0, 0, DateTimeZone.UTC);
142+
bucket = getBucket(histo, key);
119143
assertThat(bucket, notNullValue());
120-
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key));
144+
assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
121145
assertThat(bucket.getDocCount(), equalTo(3l));
122146
}
123147

@@ -437,7 +461,6 @@ public void singleValuedField_OrderedByMultiValuedSubAggregationAsc_Inherited()
437461

438462
assertSearchResponse(response);
439463

440-
441464
DateHistogram histo = response.getAggregations().get("histo");
442465
assertThat(histo, notNullValue());
443466
assertThat(histo.getName(), equalTo("histo"));
@@ -980,4 +1003,75 @@ public void emptyAggregation() throws Exception {
9801003
assertThat(dateHisto.getBuckets().isEmpty(), is(true));
9811004

9821005
}
1006+
1007+
@Test
1008+
public void singleValue_WithPreZone() throws Exception {
1009+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
1010+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
1011+
DateTime date = date("2014-03-11T00:00:00+00:00");
1012+
for (int i = 0; i < reqs.length; i++) {
1013+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
1014+
date = date.plusHours(1);
1015+
}
1016+
indexRandom(true, reqs);
1017+
1018+
SearchResponse response = client().prepareSearch("idx2")
1019+
.setQuery(matchAllQuery())
1020+
.addAggregation(dateHistogram("date_histo")
1021+
.field("date")
1022+
.preZone("-2:00")
1023+
.interval(DateHistogram.Interval.DAY)
1024+
.format("yyyy-MM-dd"))
1025+
.execute().actionGet();
1026+
1027+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
1028+
1029+
DateHistogram histo = response.getAggregations().get("date_histo");
1030+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
1031+
assertThat(buckets.size(), equalTo(2));
1032+
1033+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-10");
1034+
assertThat(bucket, Matchers.notNullValue());
1035+
assertThat(bucket.getDocCount(), equalTo(2l));
1036+
1037+
bucket = histo.getBucketByKey("2014-03-11");
1038+
assertThat(bucket, Matchers.notNullValue());
1039+
assertThat(bucket.getDocCount(), equalTo(3l));
1040+
}
1041+
1042+
@Test
1043+
public void singleValue_WithPreZone_WithAadjustLargeInterval() throws Exception {
1044+
prepareCreate("idx2").addMapping("type", "date", "type=date").execute().actionGet();
1045+
IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
1046+
DateTime date = date("2014-03-11T00:00:00+00:00");
1047+
for (int i = 0; i < reqs.length; i++) {
1048+
reqs[i] = client().prepareIndex("idx2", "type", "" + i).setSource(jsonBuilder().startObject().field("date", date).endObject());
1049+
date = date.plusHours(1);
1050+
}
1051+
indexRandom(true, reqs);
1052+
1053+
SearchResponse response = client().prepareSearch("idx2")
1054+
.setQuery(matchAllQuery())
1055+
.addAggregation(dateHistogram("date_histo")
1056+
.field("date")
1057+
.preZone("-2:00")
1058+
.interval(DateHistogram.Interval.DAY)
1059+
.preZoneAdjustLargeInterval(true)
1060+
.format("yyyy-MM-dd'T'HH:mm:ss"))
1061+
.execute().actionGet();
1062+
1063+
assertThat(response.getHits().getTotalHits(), equalTo(5l));
1064+
1065+
DateHistogram histo = response.getAggregations().get("date_histo");
1066+
Collection<? extends DateHistogram.Bucket> buckets = histo.getBuckets();
1067+
assertThat(buckets.size(), equalTo(2));
1068+
1069+
DateHistogram.Bucket bucket = histo.getBucketByKey("2014-03-10T02:00:00");
1070+
assertThat(bucket, Matchers.notNullValue());
1071+
assertThat(bucket.getDocCount(), equalTo(2l));
1072+
1073+
bucket = histo.getBucketByKey("2014-03-11T02:00:00");
1074+
assertThat(bucket, Matchers.notNullValue());
1075+
assertThat(bucket.getDocCount(), equalTo(3l));
1076+
}
9831077
}

0 commit comments

Comments
 (0)