From 8a454dae331e607457e9ee2d031cfd2459b88f1b Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 18 Nov 2015 08:09:47 +0100 Subject: [PATCH] field stats: Added a `format` option to index constraint that allows to specify date index constraint values in a different format then the for specified in the mapping. Closes #14804 --- .../action/fieldstats/FieldStats.java | 34 +++++++++++---- .../action/fieldstats/FieldStatsRequest.java | 36 +++++++++------- .../action/fieldstats/IndexConstraint.java | 41 +++++++++++++++++-- .../fieldstats/FieldStatsRequestTests.java | 12 +++++- .../fieldstats/FieldStatsTests.java | 31 ++++++++++++++ .../fieldstats-index-constraints-request.json | 10 +++++ docs/reference/search/field-stats.asciidoc | 24 ++++++++++- 7 files changed, 159 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStats.java b/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStats.java index ae72e27e79b..54e941d937a 100644 --- a/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStats.java +++ b/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStats.java @@ -122,9 +122,11 @@ public abstract class FieldStats> implements Streamable, /** * @param value The string to be parsed - * @return The concrete object represented by the string argument + * @param optionalFormat A string describing how to parse the specified value. Whether this parameter is supported + * depends on the implementation. If optionalFormat is specified and the implementation + * doesn't support it an {@link UnsupportedOperationException} is thrown */ - protected abstract T valueOf(String value); + protected abstract T valueOf(String value, String optionalFormat); /** * Merges the provided stats into this stats instance. @@ -153,7 +155,7 @@ public abstract class FieldStats> implements Streamable, */ public boolean match(IndexConstraint constraint) { int cmp; - T value = valueOf(constraint.getValue()); + T value = valueOf(constraint.getValue(), constraint.getOptionalFormat()); if (constraint.getProperty() == IndexConstraint.Property.MIN) { cmp = minValue.compareTo(value); } else if (constraint.getProperty() == IndexConstraint.Property.MAX) { @@ -245,7 +247,10 @@ public abstract class FieldStats> implements Streamable, } @Override - protected java.lang.Long valueOf(String value) { + protected java.lang.Long valueOf(String value, String optionalFormat) { + if (optionalFormat != null) { + throw new UnsupportedOperationException("custom format isn't supported"); + } return java.lang.Long.valueOf(value); } @@ -295,7 +300,10 @@ public abstract class FieldStats> implements Streamable, } @Override - protected java.lang.Float valueOf(String value) { + protected java.lang.Float valueOf(String value, String optionalFormat) { + if (optionalFormat != null) { + throw new UnsupportedOperationException("custom format isn't supported"); + } return java.lang.Float.valueOf(value); } @@ -345,7 +353,10 @@ public abstract class FieldStats> implements Streamable, } @Override - protected java.lang.Double valueOf(String value) { + protected java.lang.Double valueOf(String value, String optionalFormat) { + if (optionalFormat != null) { + throw new UnsupportedOperationException("custom format isn't supported"); + } return java.lang.Double.valueOf(value); } @@ -399,7 +410,10 @@ public abstract class FieldStats> implements Streamable, } @Override - protected BytesRef valueOf(String value) { + protected BytesRef valueOf(String value, String optionalFormat) { + if (optionalFormat != null) { + throw new UnsupportedOperationException("custom format isn't supported"); + } return new BytesRef(value); } @@ -448,7 +462,11 @@ public abstract class FieldStats> implements Streamable, } @Override - protected java.lang.Long valueOf(String value) { + protected java.lang.Long valueOf(String value, String optionalFormat) { + FormatDateTimeFormatter dateFormatter = this.dateFormatter; + if (optionalFormat != null) { + dateFormatter = Joda.forPattern(optionalFormat); + } return dateFormatter.parser().parseMillis(value); } diff --git a/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStatsRequest.java b/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStatsRequest.java index aa107518110..09411b56e25 100644 --- a/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStatsRequest.java +++ b/core/src/main/java/org/elasticsearch/action/fieldstats/FieldStatsRequest.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.fieldstats; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ValidateActions; import org.elasticsearch.action.support.broadcast.BroadcastRequest; @@ -121,22 +122,24 @@ public class FieldStatsRequest extends BroadcastRequest { currentName = parser.currentName(); } else if (fieldToken == Token.START_OBJECT) { IndexConstraint.Property property = IndexConstraint.Property.parse(currentName); - Token propertyToken = parser.nextToken(); - if (propertyToken != Token.FIELD_NAME) { - throw new IllegalArgumentException("unexpected token [" + propertyToken + "]"); - } - IndexConstraint.Comparison comparison = IndexConstraint.Comparison.parse(parser.currentName()); - propertyToken = parser.nextToken(); - if (propertyToken.isValue() == false) { - throw new IllegalArgumentException("unexpected token [" + propertyToken + "]"); - } - String value = parser.text(); - indexConstraints.add(new IndexConstraint(field, property, comparison, value)); - - propertyToken = parser.nextToken(); - if (propertyToken != Token.END_OBJECT) { - throw new IllegalArgumentException("unexpected token [" + propertyToken + "]"); + String value = null; + String optionalFormat = null; + IndexConstraint.Comparison comparison = null; + for (Token propertyToken = parser.nextToken(); propertyToken != Token.END_OBJECT; propertyToken = parser.nextToken()) { + if (propertyToken.isValue()) { + if ("format".equals(parser.currentName())) { + optionalFormat = parser.text(); + } else { + comparison = IndexConstraint.Comparison.parse(parser.currentName()); + value = parser.text(); + } + } else { + if (propertyToken != Token.FIELD_NAME) { + throw new IllegalArgumentException("unexpected token [" + propertyToken + "]"); + } + } } + indexConstraints.add(new IndexConstraint(field, property, comparison, value, optionalFormat)); } else { throw new IllegalArgumentException("unexpected token [" + fieldToken + "]"); } @@ -189,6 +192,9 @@ public class FieldStatsRequest extends BroadcastRequest { out.writeByte(indexConstraint.getProperty().getId()); out.writeByte(indexConstraint.getComparison().getId()); out.writeString(indexConstraint.getValue()); + if (out.getVersion().onOrAfter(Version.V_2_0_1)) { + out.writeOptionalString(indexConstraint.getOptionalFormat()); + } } out.writeString(level); } diff --git a/core/src/main/java/org/elasticsearch/action/fieldstats/IndexConstraint.java b/core/src/main/java/org/elasticsearch/action/fieldstats/IndexConstraint.java index 2493e34204d..19e274e785c 100644 --- a/core/src/main/java/org/elasticsearch/action/fieldstats/IndexConstraint.java +++ b/core/src/main/java/org/elasticsearch/action/fieldstats/IndexConstraint.java @@ -19,10 +19,12 @@ package org.elasticsearch.action.fieldstats; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import java.io.IOException; import java.util.Locale; +import java.util.Objects; public class IndexConstraint { @@ -30,37 +32,68 @@ public class IndexConstraint { private final Property property; private final Comparison comparison; private final String value; + private final String optionalFormat; IndexConstraint(StreamInput input) throws IOException { this.field = input.readString(); this.property = Property.read(input.readByte()); this.comparison = Comparison.read(input.readByte()); this.value = input.readString(); + if (input.getVersion().onOrAfter(Version.V_2_0_1)) { + this.optionalFormat = input.readOptionalString(); + } else { + this.optionalFormat = null; + } } public IndexConstraint(String field, Property property, Comparison comparison, String value) { - this.field = field; - this.property = property; - this.comparison = comparison; - this.value = value; + this(field, property, comparison, value, null); } + public IndexConstraint(String field, Property property, Comparison comparison, String value, String optionalFormat) { + this.field = Objects.requireNonNull(field); + this.property = Objects.requireNonNull(property); + this.comparison = Objects.requireNonNull(comparison); + this.value = Objects.requireNonNull(value); + this.optionalFormat = optionalFormat; + } + + /** + * @return On what field the constraint is going to be applied on + */ public String getField() { return field; } + /** + * @return How to compare the specified value against the field property (lt, lte, gt and gte) + */ public Comparison getComparison() { return comparison; } + /** + * @return On what property of a field the contraint is going to be applied on (min or max value) + */ public Property getProperty() { return property; } + /** + * @return The value to compare against + */ public String getValue() { return value; } + /** + * @return An optional format, that specifies how the value string is converted in the native value of the field. + * Not all field types support this and right now only date field supports this option. + */ + public String getOptionalFormat() { + return optionalFormat; + } + public enum Property { MIN((byte) 0), diff --git a/core/src/test/java/org/elasticsearch/action/fieldstats/FieldStatsRequestTests.java b/core/src/test/java/org/elasticsearch/action/fieldstats/FieldStatsRequestTests.java index e33fba69b8b..937cfb7b948 100644 --- a/core/src/test/java/org/elasticsearch/action/fieldstats/FieldStatsRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/fieldstats/FieldStatsRequestTests.java @@ -42,7 +42,7 @@ public class FieldStatsRequestTests extends ESTestCase { assertThat(request.getFields()[3], equalTo("field4")); assertThat(request.getFields()[4], equalTo("field5")); - assertThat(request.getIndexConstraints().length, equalTo(6)); + assertThat(request.getIndexConstraints().length, equalTo(8)); assertThat(request.getIndexConstraints()[0].getField(), equalTo("field2")); assertThat(request.getIndexConstraints()[0].getValue(), equalTo("9")); assertThat(request.getIndexConstraints()[0].getProperty(), equalTo(MAX)); @@ -67,6 +67,16 @@ public class FieldStatsRequestTests extends ESTestCase { assertThat(request.getIndexConstraints()[5].getValue(), equalTo("9")); assertThat(request.getIndexConstraints()[5].getProperty(), equalTo(MAX)); assertThat(request.getIndexConstraints()[5].getComparison(), equalTo(LT)); + assertThat(request.getIndexConstraints()[6].getField(), equalTo("field1")); + assertThat(request.getIndexConstraints()[6].getValue(), equalTo("2014-01-01")); + assertThat(request.getIndexConstraints()[6].getProperty(), equalTo(MIN)); + assertThat(request.getIndexConstraints()[6].getComparison(), equalTo(GTE)); + assertThat(request.getIndexConstraints()[6].getOptionalFormat(), equalTo("date_optional_time")); + assertThat(request.getIndexConstraints()[7].getField(), equalTo("field1")); + assertThat(request.getIndexConstraints()[7].getValue(), equalTo("2015-01-01")); + assertThat(request.getIndexConstraints()[7].getProperty(), equalTo(MAX)); + assertThat(request.getIndexConstraints()[7].getComparison(), equalTo(LT)); + assertThat(request.getIndexConstraints()[7].getOptionalFormat(), equalTo("date_optional_time")); } } diff --git a/core/src/test/java/org/elasticsearch/fieldstats/FieldStatsTests.java b/core/src/test/java/org/elasticsearch/fieldstats/FieldStatsTests.java index 5db3130913c..0f36bdcfcc1 100644 --- a/core/src/test/java/org/elasticsearch/fieldstats/FieldStatsTests.java +++ b/core/src/test/java/org/elasticsearch/fieldstats/FieldStatsTests.java @@ -24,6 +24,8 @@ import org.elasticsearch.action.fieldstats.FieldStatsResponse; import org.elasticsearch.action.fieldstats.IndexConstraint; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import java.util.ArrayList; import java.util.List; @@ -359,4 +361,33 @@ public class FieldStatsTests extends ESSingleNodeTestCase { assertThat(response.getIndicesMergedFieldStats().get("test2").get("value").getMinValue(), equalTo("2014-01-02T00:00:00.000Z")); } + public void testDateFiltering_optionalFormat() { + createIndex("test1", Settings.EMPTY, "type", "value", "type=date,format=strict_date_optional_time"); + client().prepareIndex("test1", "type").setSource("value", "2014-01-01T00:00:00.000Z").get(); + createIndex("test2", Settings.EMPTY, "type", "value", "type=date,format=strict_date_optional_time"); + client().prepareIndex("test2", "type").setSource("value", "2014-01-02T00:00:00.000Z").get(); + client().admin().indices().prepareRefresh().get(); + + DateTime dateTime1 = new DateTime(2014, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC); + DateTime dateTime2 = new DateTime(2014, 1, 2, 0, 0, 0, 0, DateTimeZone.UTC); + FieldStatsResponse response = client().prepareFieldStats() + .setFields("value") + .setIndexContraints(new IndexConstraint("value", MIN, GT, String.valueOf(dateTime1.getMillis()), "epoch_millis"), new IndexConstraint("value", MAX, LTE, String.valueOf(dateTime2.getMillis()), "epoch_millis")) + .setLevel("indices") + .get(); + assertThat(response.getIndicesMergedFieldStats().size(), equalTo(1)); + assertThat(response.getIndicesMergedFieldStats().get("test2").get("value").getMinValue(), equalTo("2014-01-02T00:00:00.000Z")); + + try { + client().prepareFieldStats() + .setFields("value") + .setIndexContraints(new IndexConstraint("value", MIN, GT, String.valueOf(dateTime1.getMillis()), "xyz")) + .setLevel("indices") + .get(); + fail("IllegalArgumentException should have been thrown"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("Invalid format")); + } + } + } \ No newline at end of file diff --git a/core/src/test/resources/org/elasticsearch/action/fieldstats/fieldstats-index-constraints-request.json b/core/src/test/resources/org/elasticsearch/action/fieldstats/fieldstats-index-constraints-request.json index 525a5692122..8f3cc9c5044 100644 --- a/core/src/test/resources/org/elasticsearch/action/fieldstats/fieldstats-index-constraints-request.json +++ b/core/src/test/resources/org/elasticsearch/action/fieldstats/fieldstats-index-constraints-request.json @@ -28,6 +28,16 @@ "max_value" : { "lt": 9 } + }, + "field1": { + "min_value" : { + "gte": "2014-01-01", + "format" : "date_optional_time" + }, + "max_value" : { + "lt": "2015-01-01", + "format" : "date_optional_time" + } } } } \ No newline at end of file diff --git a/docs/reference/search/field-stats.asciidoc b/docs/reference/search/field-stats.asciidoc index fb29903ebeb..a99e8a00b1e 100644 --- a/docs/reference/search/field-stats.asciidoc +++ b/docs/reference/search/field-stats.asciidoc @@ -240,7 +240,7 @@ curl -XPOST "http://localhost:9200/_field_stats?level=indices" -d '{ "index_constraints" : { <2> "creation_date" : { <3> "min_value" : { <4> - "gte" : "2014-01-01T00:00:00.000Z", + "gte" : "2014-01-01T00:00:00.000Z" }, "max_value" : { "lt" : "2015-01-01T00:00:00.000Z" @@ -263,3 +263,25 @@ Each index constraint support the following comparisons: `gt`:: Greater-than `lte`:: Less-than or equal to `lt`:: Less-than + +Field stats index constraints on date fields optionally accept a `format` option, used to parse the constraint's value. +If missing, the format configured in the field's mapping is used. + +[source,js] +-------------------------------------------------- +curl -XPOST "http://localhost:9200/_field_stats?level=indices" -d '{ + "fields" : ["answer_count"] <1> + "index_constraints" : { <2> + "creation_date" : { <3> + "min_value" : { <4> + "gte" : "2014-01-01", + "format" : "date_optional_time" + }, + "max_value" : { + "lt" : "2015-01-01", + "format" : "date_optional_time" + } + } + } +}' +-------------------------------------------------- \ No newline at end of file