Skip to content

Commit 9a3fa42

Browse files
authored
Standardize script field's rejection error (#60029)
This standardizes the exception type thrown when attempting to build a query on a script field that it doesn't support. It also replaces the field type `[script]` in the error message with whatever the runtime type is.
1 parent 9adf29c commit 9a3fa42

File tree

6 files changed

+148
-8
lines changed

6 files changed

+148
-8
lines changed

x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/AbstractScriptMappedFieldType.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66

77
package org.elasticsearch.xpack.runtimefields.mapper;
88

9+
import org.apache.lucene.analysis.TokenStream;
10+
import org.apache.lucene.search.MultiTermQuery;
11+
import org.apache.lucene.search.Query;
12+
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
13+
import org.apache.lucene.search.spans.SpanQuery;
914
import org.elasticsearch.ElasticsearchException;
15+
import org.elasticsearch.common.geo.ShapeRelation;
16+
import org.elasticsearch.common.time.DateMathParser;
17+
import org.elasticsearch.common.unit.Fuzziness;
1018
import org.elasticsearch.index.mapper.MappedFieldType;
1119
import org.elasticsearch.index.mapper.TextSearchInfo;
1220
import org.elasticsearch.index.query.QueryShardContext;
1321
import org.elasticsearch.script.Script;
1422

23+
import java.io.IOException;
24+
import java.time.ZoneId;
25+
import java.util.List;
1526
import java.util.Map;
1627

1728
import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
@@ -49,6 +60,71 @@ public final boolean isAggregatable() {
4960
return true;
5061
}
5162

63+
public abstract Query termsQuery(List<?> values, QueryShardContext context);
64+
65+
public abstract Query rangeQuery(
66+
Object lowerTerm,
67+
Object upperTerm,
68+
boolean includeLower,
69+
boolean includeUpper,
70+
ShapeRelation relation,
71+
ZoneId timeZone,
72+
DateMathParser parser,
73+
QueryShardContext context
74+
);
75+
76+
public Query fuzzyQuery(
77+
Object value,
78+
Fuzziness fuzziness,
79+
int prefixLength,
80+
int maxExpansions,
81+
boolean transpositions,
82+
QueryShardContext context
83+
) {
84+
throw new IllegalArgumentException(unsupported("fuzzy", "keyword and text"));
85+
}
86+
87+
public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, QueryShardContext context) {
88+
throw new IllegalArgumentException(unsupported("prefix", "keyword, text and wildcard"));
89+
}
90+
91+
public Query wildcardQuery(String value, MultiTermQuery.RewriteMethod method, QueryShardContext context) {
92+
throw new IllegalArgumentException(unsupported("wildcard", "keyword, text and wildcard"));
93+
}
94+
95+
public Query regexpQuery(
96+
String value,
97+
int flags,
98+
int maxDeterminizedStates,
99+
MultiTermQuery.RewriteMethod method,
100+
QueryShardContext context
101+
) {
102+
throw new IllegalArgumentException(unsupported("regexp", "keyword and text"));
103+
}
104+
105+
public abstract Query existsQuery(QueryShardContext context);
106+
107+
public Query phraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException {
108+
throw new IllegalArgumentException(unsupported("phrase", "text"));
109+
}
110+
111+
public Query multiPhraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException {
112+
throw new IllegalArgumentException(unsupported("phrase", "text"));
113+
}
114+
115+
public Query phrasePrefixQuery(TokenStream stream, int slop, int maxExpansions) throws IOException {
116+
throw new IllegalArgumentException(unsupported("phrase prefix", "text"));
117+
}
118+
119+
public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRewriteMethod method, QueryShardContext context) {
120+
throw new IllegalArgumentException(unsupported("span prefix", "text"));
121+
}
122+
123+
private String unsupported(String query, String supported) {
124+
String thisField = "[" + name() + "] which is of type [script] with runtime_type [" + runtimeType() + "]";
125+
return "Can only use " + query + " queries on " + supported + " fields - not on " + thisField;
126+
}
127+
52128
protected final void checkAllowExpensiveQueries(QueryShardContext context) {
53129
if (context.allowExpensiveQueries() == false) {
54130
throw new ElasticsearchException(

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/AbstractNonTextScriptMappedFieldTypeTestCase.java

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
import static org.hamcrest.Matchers.equalTo;
1515

1616
abstract class AbstractNonTextScriptMappedFieldTypeTestCase extends AbstractScriptMappedFieldTypeTestCase {
17-
protected abstract AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException;
18-
1917
public void testFuzzyQueryIsError() throws IOException {
2018
assertQueryOnlyOnTextAndKeyword(
2119
"fuzzy",
@@ -39,20 +37,30 @@ public void testWildcardQueryIsError() throws IOException {
3937
}
4038

4139
private void assertQueryOnlyOnTextAndKeyword(String queryName, ThrowingRunnable buildQuery) {
42-
// TODO use runtime type in the error message and a consistent exception type
43-
Exception e = expectThrows(Exception.class, buildQuery);
40+
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
4441
assertThat(
4542
e.getMessage(),
46-
equalTo("Can only use " + queryName + " queries on keyword and text fields - not on [test] which is of type [script]")
43+
equalTo(
44+
"Can only use "
45+
+ queryName
46+
+ " queries on keyword and text fields - not on [test] which is of type [script] with runtime_type ["
47+
+ runtimeType()
48+
+ "]"
49+
)
4750
);
4851
}
4952

5053
private void assertQueryOnlyOnTextKeywordAndWildcard(String queryName, ThrowingRunnable buildQuery) {
51-
// TODO use runtime type in the error message and a consistent exception type
52-
Exception e = expectThrows(Exception.class, buildQuery);
54+
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
5355
assertThat(
5456
e.getMessage(),
55-
equalTo("Can only use " + queryName + " queries on keyword, text and wildcard fields - not on [test] which is of type [script]")
57+
equalTo(
58+
"Can only use "
59+
+ queryName
60+
+ " queries on keyword, text and wildcard fields - not on [test] which is of type [script] with runtime_type ["
61+
+ runtimeType()
62+
+ "]"
63+
)
5664
);
5765
}
5866
}

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/AbstractScriptMappedFieldTypeTestCase.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@
1515

1616
import java.io.IOException;
1717

18+
import static org.hamcrest.Matchers.equalTo;
1819
import static org.mockito.Matchers.any;
1920
import static org.mockito.Matchers.anyString;
2021
import static org.mockito.Mockito.mock;
2122
import static org.mockito.Mockito.when;
2223

2324
abstract class AbstractScriptMappedFieldTypeTestCase extends ESTestCase {
25+
protected abstract AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException;
26+
27+
protected abstract String runtimeType();
28+
2429
public abstract void testDocValues() throws IOException;
2530

2631
public abstract void testSort() throws IOException;
@@ -71,4 +76,35 @@ protected static QueryShardContext mockContext(boolean allowExpensiveQueries, Ab
7176
when(context.lookup()).thenReturn(lookup);
7277
return context;
7378
}
79+
80+
public void testPhraseQueryIsError() throws IOException {
81+
assertQueryOnlyOnText("phrase", () -> simpleMappedFieldType().phraseQuery(null, 1, false));
82+
}
83+
84+
public void testPhrasePrefixQueryIsError() throws IOException {
85+
assertQueryOnlyOnText("phrase prefix", () -> simpleMappedFieldType().phrasePrefixQuery(null, 1, 1));
86+
}
87+
88+
public void testMultiPhraseQueryIsError() throws IOException {
89+
assertQueryOnlyOnText("phrase", () -> simpleMappedFieldType().multiPhraseQuery(null, 1, false));
90+
}
91+
92+
public void testSpanPrefixQueryIsError() throws IOException {
93+
assertQueryOnlyOnText("span prefix", () -> simpleMappedFieldType().spanPrefixQuery(null, null, null));
94+
}
95+
96+
private void assertQueryOnlyOnText(String queryName, ThrowingRunnable buildQuery) {
97+
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
98+
assertThat(
99+
e.getMessage(),
100+
equalTo(
101+
"Can only use "
102+
+ queryName
103+
+ " queries on text fields - not on [test] which is of type [script] with runtime_type ["
104+
+ runtimeType()
105+
+ "]"
106+
)
107+
);
108+
}
109+
74110
}

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptDoubleMappedFieldTypeTests.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ protected ScriptDoubleMappedFieldType simpleMappedFieldType() throws IOException
252252
return build("value(source.foo)");
253253
}
254254

255+
@Override
256+
protected String runtimeType() {
257+
return "double";
258+
}
259+
255260
private static ScriptDoubleMappedFieldType build(String code) throws IOException {
256261
return build(new Script(code));
257262
}

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptKeywordMappedFieldTypeTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,16 @@ public void testMatchQuery() throws IOException {
331331
}
332332
}
333333

334+
@Override
335+
protected AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException {
336+
return build("value(source.foo)");
337+
}
338+
339+
@Override
340+
protected String runtimeType() {
341+
return "keyword";
342+
}
343+
334344
private static ScriptKeywordMappedFieldType build(String code) throws IOException {
335345
return build(new Script(code));
336346
}

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptLongMappedFieldTypeTests.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,11 @@ protected ScriptLongMappedFieldType simpleMappedFieldType() throws IOException {
245245
return build("value(source.foo)");
246246
}
247247

248+
@Override
249+
protected String runtimeType() {
250+
return "long";
251+
}
252+
248253
private static ScriptLongMappedFieldType build(String code) throws IOException {
249254
return build(new Script(code));
250255
}

0 commit comments

Comments
 (0)