Fix error massage for unknown value type (#55821) (#55825)

Fixes confusing error message when unknown value type is specified in a terms
aggregation. Adds support for parsing "numeric" and "number" value types.

Fixes #55727
This commit is contained in:
Igor Motov 2020-04-27 18:34:43 -04:00 committed by GitHub
parent 08d328333a
commit 2ff858b290
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 1 deletions

View File

@ -109,6 +109,8 @@ public enum ValueType implements Writeable {
case "string": return STRING; case "string": return STRING;
case "double": case "double":
case "float": return DOUBLE; case "float": return DOUBLE;
case "number":
case "numeric":
case "long": case "long":
case "integer": case "integer":
case "short": case "short":

View File

@ -54,7 +54,13 @@ public abstract class ValuesSourceAggregationBuilder<AB extends ValuesSourceAggr
objectParser.declareField(ValuesSourceAggregationBuilder::missing, XContentParser::objectText, objectParser.declareField(ValuesSourceAggregationBuilder::missing, XContentParser::objectText,
ParseField.CommonFields.MISSING, ObjectParser.ValueType.VALUE); ParseField.CommonFields.MISSING, ObjectParser.ValueType.VALUE);
objectParser.declareField(ValuesSourceAggregationBuilder::userValueTypeHint, p -> ValueType.lenientParse(p.text()), objectParser.declareField(ValuesSourceAggregationBuilder::userValueTypeHint, p -> {
ValueType type = ValueType.lenientParse(p.text());
if (type == null) {
throw new IllegalArgumentException("Unknown value type [" + p.text() + "]");
}
return type;
},
ValueType.VALUE_TYPE, ObjectParser.ValueType.STRING); ValueType.VALUE_TYPE, ObjectParser.ValueType.STRING);
if (formattable) { if (formattable) {

View File

@ -24,6 +24,9 @@ import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.Strings; import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParseException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.mapper.IndexFieldMapper; import org.elasticsearch.index.mapper.IndexFieldMapper;
import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryBuilders;
@ -41,6 +44,8 @@ import org.elasticsearch.search.aggregations.metrics.Avg;
import org.elasticsearch.search.aggregations.metrics.ExtendedStats; import org.elasticsearch.search.aggregations.metrics.ExtendedStats;
import org.elasticsearch.search.aggregations.metrics.Stats; import org.elasticsearch.search.aggregations.metrics.Stats;
import org.elasticsearch.search.aggregations.metrics.Sum; import org.elasticsearch.search.aggregations.metrics.Sum;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
@ -70,6 +75,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.Matchers.startsWith;
import static org.hamcrest.core.IsNull.notNullValue; import static org.hamcrest.core.IsNull.notNullValue;
@ -117,6 +123,8 @@ public class StringTermsIT extends AbstractTermsTestCase {
return value.getValue(); return value.getValue();
}); });
scripts.put("42", vars -> 42);
return scripts; return scripts;
} }
@ -1175,4 +1183,39 @@ public class StringTermsIT extends AbstractTermsTestCase {
assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache()
.getMissCount(), equalTo(2L)); .getMissCount(), equalTo(2L));
} }
public void testScriptWithValueType() throws Exception {
SearchSourceBuilder builder = new SearchSourceBuilder()
.size(0)
.aggregation(terms("terms")
.script(new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "42", Collections.emptyMap()))
.userValueTypeHint(randomFrom(ValueType.NUMERIC, ValueType.NUMBER)));
String source = builder.toString();
try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
SearchResponse response = client()
.prepareSearch("idx")
.setSource(SearchSourceBuilder.fromXContent(parser))
.get();
assertSearchResponse(response);
Terms terms = response.getAggregations().get("terms");
assertThat(terms, notNullValue());
assertThat(terms.getName(), equalTo("terms"));
assertThat(terms.getBuckets().size(), equalTo(1));
}
String invalidValueType = source.replaceAll("\"value_type\":\"n.*\"", "\"value_type\":\"foobar\"");
try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidValueType)) {
XContentParseException ex = expectThrows(XContentParseException.class, () -> client()
.prepareSearch("idx")
.setSource(SearchSourceBuilder.fromXContent(parser))
.get());
assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
assertThat(ex.getCause().getMessage(), containsString("Unknown value type [foobar]"));
}
}
} }