Aggregations: Fix setting timezone on default DateTime formatter
This PR prevents setting timezone on ValueFormatter.DateTime. Instead the timezone information needed when printing buckets key-as-string information is provided at constrution time of the ValueFormatter, making sure we don't overwrite any constants. This, however, made it necessary to be able to access the timezone information when resolving the format in ValueSourseParser, so the `time_zone` parameter is now parsed alongside the `format` parameter in ValueSourceParser rather than in DateHistogramParser. Closes #12531
This commit is contained in:
Christoph Büscher
parent
8efa18e616
commit
cdb39cf5dc
|
|
@ -64,6 +64,9 @@ public abstract class TimeZoneRounding extends Rounding {
|
|||
}
|
||||
|
||||
public Builder timeZone(DateTimeZone timeZone) {
|
||||
if (timeZone == null) {
|
||||
throw new IllegalArgumentException("Setting null as timezone is not supported");
|
||||
}
|
||||
this.timeZone = timeZone;
|
||||
return this;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,10 +33,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactory;
|
|||
import org.elasticsearch.search.aggregations.support.ValueType;
|
||||
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
|
||||
import org.elasticsearch.search.aggregations.support.ValuesSourceParser;
|
||||
import org.elasticsearch.search.aggregations.support.format.ValueFormatter.DateTime;
|
||||
import org.elasticsearch.search.internal.SearchContext;
|
||||
import org.joda.time.DateTimeZone;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
/**
|
||||
|
|
@ -45,7 +42,6 @@ import java.io.IOException;
|
|||
public class DateHistogramParser implements Aggregator.Parser {
|
||||
|
||||
static final ParseField EXTENDED_BOUNDS = new ParseField("extended_bounds");
|
||||
static final ParseField TIME_ZONE = new ParseField("time_zone");
|
||||
static final ParseField OFFSET = new ParseField("offset");
|
||||
static final ParseField INTERVAL = new ParseField("interval");
|
||||
|
||||
|
|
@ -83,6 +79,7 @@ public class DateHistogramParser implements Aggregator.Parser {
|
|||
ValuesSourceParser vsParser = ValuesSourceParser.numeric(aggregationName, InternalDateHistogram.TYPE, context)
|
||||
.targetValueType(ValueType.DATE)
|
||||
.formattable(true)
|
||||
.timezoneAware(true)
|
||||
.build();
|
||||
|
||||
boolean keyed = false;
|
||||
|
|
@ -90,7 +87,6 @@ public class DateHistogramParser implements Aggregator.Parser {
|
|||
ExtendedBounds extendedBounds = null;
|
||||
InternalOrder order = (InternalOrder) Histogram.Order.KEY_ASC;
|
||||
String interval = null;
|
||||
DateTimeZone timeZone = DateTimeZone.UTC;
|
||||
long offset = 0;
|
||||
|
||||
XContentParser.Token token;
|
||||
|
|
@ -101,9 +97,7 @@ public class DateHistogramParser implements Aggregator.Parser {
|
|||
} else if (vsParser.token(currentFieldName, token, parser)) {
|
||||
continue;
|
||||
} else if (token == XContentParser.Token.VALUE_STRING) {
|
||||
if (context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
|
||||
timeZone = DateTimeZone.forID(parser.text());
|
||||
} else if (context.parseFieldMatcher().match(currentFieldName, OFFSET)) {
|
||||
if (context.parseFieldMatcher().match(currentFieldName, OFFSET)) {
|
||||
offset = parseOffset(parser.text());
|
||||
} else if (context.parseFieldMatcher().match(currentFieldName, INTERVAL)) {
|
||||
interval = parser.text();
|
||||
|
|
@ -121,8 +115,6 @@ public class DateHistogramParser implements Aggregator.Parser {
|
|||
} else if (token == XContentParser.Token.VALUE_NUMBER) {
|
||||
if ("min_doc_count".equals(currentFieldName) || "minDocCount".equals(currentFieldName)) {
|
||||
minDocCount = parser.longValue();
|
||||
} else if ("time_zone".equals(currentFieldName) || "timeZone".equals(currentFieldName)) {
|
||||
timeZone = DateTimeZone.forOffsetHours(parser.intValue());
|
||||
} else {
|
||||
throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: ["
|
||||
+ currentFieldName + "].", parser.getTokenLocation());
|
||||
|
|
@ -193,13 +185,10 @@ public class DateHistogramParser implements Aggregator.Parser {
|
|||
}
|
||||
|
||||
Rounding rounding = tzRoundingBuilder
|
||||
.timeZone(timeZone)
|
||||
.timeZone(vsParser.input().timezone())
|
||||
.offset(offset).build();
|
||||
|
||||
ValuesSourceConfig config = vsParser.config();
|
||||
if (config.formatter()!=null) {
|
||||
((DateTime) config.formatter()).setTimeZone(timeZone);
|
||||
}
|
||||
return new HistogramAggregator.Factory(aggregationName, config, rounding, order, keyed, minDocCount, extendedBounds,
|
||||
new InternalDateHistogram.Factory());
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
package org.elasticsearch.search.aggregations.support;
|
||||
|
||||
import org.elasticsearch.common.Nullable;
|
||||
import org.elasticsearch.common.ParseField;
|
||||
import org.elasticsearch.common.xcontent.XContentParser;
|
||||
import org.elasticsearch.index.fielddata.IndexFieldData;
|
||||
import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
|
||||
|
|
@ -39,6 +40,7 @@ import org.elasticsearch.search.SearchParseException;
|
|||
import org.elasticsearch.search.aggregations.InternalAggregation;
|
||||
import org.elasticsearch.search.aggregations.support.format.ValueFormat;
|
||||
import org.elasticsearch.search.internal.SearchContext;
|
||||
import org.joda.time.DateTimeZone;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Map;
|
||||
|
|
@ -50,6 +52,8 @@ import static com.google.common.collect.Maps.newHashMap;
|
|||
*/
|
||||
public class ValuesSourceParser<VS extends ValuesSource> {
|
||||
|
||||
static final ParseField TIME_ZONE = new ParseField("time_zone");
|
||||
|
||||
public static Builder any(String aggName, InternalAggregation.Type aggType, SearchContext context) {
|
||||
return new Builder<>(aggName, aggType, context, ValuesSource.class);
|
||||
}
|
||||
|
|
@ -66,14 +70,19 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
return new Builder<>(aggName, aggType, context, ValuesSource.GeoPoint.class).targetValueType(ValueType.GEOPOINT).scriptable(false);
|
||||
}
|
||||
|
||||
private static class Input {
|
||||
String field = null;
|
||||
Script script = null;
|
||||
public static class Input {
|
||||
private String field = null;
|
||||
private Script script = null;
|
||||
@Deprecated
|
||||
Map<String, Object> params = null; // TODO Remove in 3.0
|
||||
ValueType valueType = null;
|
||||
String format = null;
|
||||
Object missing = null;
|
||||
private Map<String, Object> params = null; // TODO Remove in 3.0
|
||||
private ValueType valueType = null;
|
||||
private String format = null;
|
||||
private Object missing = null;
|
||||
private DateTimeZone timezone = DateTimeZone.UTC;
|
||||
|
||||
public DateTimeZone timezone() {
|
||||
return this.timezone;
|
||||
}
|
||||
}
|
||||
|
||||
private final String aggName;
|
||||
|
|
@ -83,6 +92,7 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
|
||||
private boolean scriptable = true;
|
||||
private boolean formattable = false;
|
||||
private boolean timezoneAware = false;
|
||||
private ValueType targetValueType = null;
|
||||
private ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
|
||||
|
||||
|
|
@ -105,6 +115,8 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
input.field = parser.text();
|
||||
} else if (formattable && "format".equals(currentFieldName)) {
|
||||
input.format = parser.text();
|
||||
} else if (timezoneAware && context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
|
||||
input.timezone = DateTimeZone.forID(parser.text());
|
||||
} else if (scriptable) {
|
||||
if ("value_type".equals(currentFieldName) || "valueType".equals(currentFieldName)) {
|
||||
input.valueType = ValueType.resolveForScript(parser.text());
|
||||
|
|
@ -123,6 +135,14 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
}
|
||||
return true;
|
||||
}
|
||||
if (token == XContentParser.Token.VALUE_NUMBER) {
|
||||
if (timezoneAware && context.parseFieldMatcher().match(currentFieldName, TIME_ZONE)) {
|
||||
input.timezone = DateTimeZone.forOffsetHours(parser.intValue());
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
if (scriptable && token == XContentParser.Token.START_OBJECT) {
|
||||
if (context.parseFieldMatcher().match(currentFieldName, ScriptField.SCRIPT)) {
|
||||
input.script = Script.parse(parser, context.parseFieldMatcher());
|
||||
|
|
@ -203,7 +223,7 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
config.fieldContext = new FieldContext(input.field, indexFieldData, fieldType);
|
||||
config.missing = input.missing;
|
||||
config.script = createScript();
|
||||
config.format = resolveFormat(input.format, fieldType);
|
||||
config.format = resolveFormat(input.format, input.timezone, fieldType);
|
||||
return config;
|
||||
}
|
||||
|
||||
|
|
@ -222,9 +242,9 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
return valueFormat;
|
||||
}
|
||||
|
||||
private static ValueFormat resolveFormat(@Nullable String format, MappedFieldType fieldType) {
|
||||
private static ValueFormat resolveFormat(@Nullable String format, @Nullable DateTimeZone timezone, MappedFieldType fieldType) {
|
||||
if (fieldType instanceof DateFieldMapper.DateFieldType) {
|
||||
return format != null ? ValueFormat.DateTime.format(format) : ValueFormat.DateTime.mapper((DateFieldMapper.DateFieldType) fieldType);
|
||||
return format != null ? ValueFormat.DateTime.format(format, timezone) : ValueFormat.DateTime.mapper((DateFieldMapper.DateFieldType) fieldType, timezone);
|
||||
}
|
||||
if (fieldType instanceof IpFieldMapper.IpFieldType) {
|
||||
return ValueFormat.IPv4;
|
||||
|
|
@ -238,6 +258,10 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
return ValueFormat.RAW;
|
||||
}
|
||||
|
||||
public Input input() {
|
||||
return this.input;
|
||||
}
|
||||
|
||||
public static class Builder<VS extends ValuesSource> {
|
||||
|
||||
private final ValuesSourceParser<VS> parser;
|
||||
|
|
@ -256,6 +280,11 @@ public class ValuesSourceParser<VS extends ValuesSource> {
|
|||
return this;
|
||||
}
|
||||
|
||||
public Builder<VS> timezoneAware(boolean timezoneAware) {
|
||||
parser.timezoneAware = timezoneAware;
|
||||
return this;
|
||||
}
|
||||
|
||||
public Builder<VS> targetValueType(ValueType valueType) {
|
||||
parser.targetValueType = valueType;
|
||||
return this;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
package org.elasticsearch.search.aggregations.support.format;
|
||||
|
||||
import org.elasticsearch.index.mapper.core.DateFieldMapper;
|
||||
import org.joda.time.DateTimeZone;
|
||||
|
||||
/**
|
||||
*
|
||||
|
|
@ -67,12 +68,12 @@ public class ValueFormat {
|
|||
|
||||
public static final DateTime DEFAULT = new DateTime(DateFieldMapper.Defaults.DATE_TIME_FORMATTER.format(), ValueFormatter.DateTime.DEFAULT, ValueParser.DateMath.DEFAULT);
|
||||
|
||||
public static DateTime format(String format) {
|
||||
return new DateTime(format, new ValueFormatter.DateTime(format), new ValueParser.DateMath(format));
|
||||
public static DateTime format(String format, DateTimeZone timezone) {
|
||||
return new DateTime(format, new ValueFormatter.DateTime(format, timezone), new ValueParser.DateMath(format));
|
||||
}
|
||||
|
||||
public static DateTime mapper(DateFieldMapper.DateFieldType fieldType) {
|
||||
return new DateTime(fieldType.dateTimeFormatter().format(), ValueFormatter.DateTime.mapper(fieldType), ValueParser.DateMath.mapper(fieldType));
|
||||
public static DateTime mapper(DateFieldMapper.DateFieldType fieldType, DateTimeZone timezone) {
|
||||
return new DateTime(fieldType.dateTimeFormatter().format(), ValueFormatter.DateTime.mapper(fieldType, timezone), ValueParser.DateMath.mapper(fieldType));
|
||||
}
|
||||
|
||||
public DateTime(String pattern, ValueFormatter formatter, ValueParser parser) {
|
||||
|
|
@ -81,7 +82,7 @@ public class ValueFormat {
|
|||
|
||||
@Override
|
||||
public DateTime create(String pattern) {
|
||||
return format(pattern);
|
||||
return format(pattern, DateTimeZone.UTC);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@ import java.text.DecimalFormat;
|
|||
import java.text.DecimalFormatSymbols;
|
||||
import java.text.NumberFormat;
|
||||
import java.util.Locale;
|
||||
import java.util.TimeZone;
|
||||
|
||||
/**
|
||||
* A strategy for formatting time represented as millis long value to string
|
||||
|
|
@ -61,7 +60,6 @@ public interface ValueFormatter extends Streamable {
|
|||
String format(long value);
|
||||
|
||||
/**
|
||||
* The
|
||||
* @param value double The double value to format.
|
||||
* @return The formatted value as string
|
||||
*/
|
||||
|
|
@ -104,8 +102,8 @@ public interface ValueFormatter extends Streamable {
|
|||
public static final ValueFormatter DEFAULT = new ValueFormatter.DateTime(DateFieldMapper.Defaults.DATE_TIME_FORMATTER);
|
||||
private DateTimeZone timeZone = DateTimeZone.UTC;
|
||||
|
||||
public static DateTime mapper(DateFieldMapper.DateFieldType fieldType) {
|
||||
return new DateTime(fieldType.dateTimeFormatter());
|
||||
public static DateTime mapper(DateFieldMapper.DateFieldType fieldType, DateTimeZone timezone) {
|
||||
return new DateTime(fieldType.dateTimeFormatter(), timezone);
|
||||
}
|
||||
|
||||
static final byte ID = 2;
|
||||
|
|
@ -122,15 +120,21 @@ public interface ValueFormatter extends Streamable {
|
|||
this.formatter = formatter;
|
||||
}
|
||||
|
||||
public DateTime(String format, DateTimeZone timezone) {
|
||||
this.formatter = Joda.forPattern(format);
|
||||
this.timeZone = timezone != null ? timezone : DateTimeZone.UTC;
|
||||
}
|
||||
|
||||
public DateTime(FormatDateTimeFormatter formatter, DateTimeZone timezone) {
|
||||
this.formatter = formatter;
|
||||
this.timeZone = timezone != null ? timezone : DateTimeZone.UTC;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String format(long time) {
|
||||
return formatter.printer().withZone(timeZone).print(time);
|
||||
}
|
||||
|
||||
public void setTimeZone(DateTimeZone timeZone) {
|
||||
this.timeZone = timeZone;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String format(double value) {
|
||||
return format((long) value);
|
||||
|
|
@ -264,7 +268,7 @@ public interface ValueFormatter extends Streamable {
|
|||
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
static class BooleanFormatter implements ValueFormatter {
|
||||
|
||||
static final byte ID = 10;
|
||||
|
|
|
|||
Loading…
Reference in New Issue