Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize script field's rejection error #60029

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,23 @@

package org.elasticsearch.xpack.runtimefields.mapper;

import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.search.MultiTermQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
import org.apache.lucene.search.spans.SpanQuery;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.TextSearchInfo;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.script.Script;

import java.io.IOException;
import java.time.ZoneId;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
Expand Down Expand Up @@ -49,6 +60,71 @@ public final boolean isAggregatable() {
return true;
}

public abstract Query termsQuery(List<?> values, QueryShardContext context);

public abstract Query rangeQuery(
Object lowerTerm,
Object upperTerm,
boolean includeLower,
boolean includeUpper,
ShapeRelation relation,
ZoneId timeZone,
DateMathParser parser,
QueryShardContext context
);

public Query fuzzyQuery(
Object value,
Fuzziness fuzziness,
int prefixLength,
int maxExpansions,
boolean transpositions,
QueryShardContext context
) {
throw new IllegalArgumentException(unsupported("fuzzy", "keyword and text"));
}

public Query prefixQuery(String value, MultiTermQuery.RewriteMethod method, QueryShardContext context) {
throw new IllegalArgumentException(unsupported("prefix", "keyword, text and wildcard"));
}

public Query wildcardQuery(String value, MultiTermQuery.RewriteMethod method, QueryShardContext context) {
throw new IllegalArgumentException(unsupported("wildcard", "keyword, text and wildcard"));
}

public Query regexpQuery(
String value,
int flags,
int maxDeterminizedStates,
MultiTermQuery.RewriteMethod method,
QueryShardContext context
) {
throw new IllegalArgumentException(unsupported("regexp", "keyword and text"));
}

public abstract Query existsQuery(QueryShardContext context);

public Query phraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException {
throw new IllegalArgumentException(unsupported("phrase", "text"));
}

public Query multiPhraseQuery(TokenStream stream, int slop, boolean enablePositionIncrements) throws IOException {
throw new IllegalArgumentException(unsupported("phrase", "text"));
}

public Query phrasePrefixQuery(TokenStream stream, int slop, int maxExpansions) throws IOException {
throw new IllegalArgumentException(unsupported("phrase prefix", "text"));
}

public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRewriteMethod method, QueryShardContext context) {
throw new IllegalArgumentException(unsupported("span prefix", "text"));
}

private String unsupported(String query, String supported) {
String thisField = "[" + name() + "] which is of type [script] with runtime_type [" + runtimeType() + "]";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't hardcode script :) we are going to rename it and if you use the existing constant renaming will be easier

return "Can only use " + query + " queries on " + supported + " fields - not on " + thisField;
}

protected final void checkAllowExpensiveQueries(QueryShardContext context) {
if (context.allowExpensiveQueries() == false) {
throw new ElasticsearchException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
import static org.hamcrest.Matchers.equalTo;

abstract class AbstractNonTextScriptMappedFieldTypeTestCase extends AbstractScriptMappedFieldTypeTestCase {
protected abstract AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException;

public void testFuzzyQueryIsError() throws IOException {
assertQueryOnlyOnTextAndKeyword(
"fuzzy",
Expand All @@ -39,20 +37,30 @@ public void testWildcardQueryIsError() throws IOException {
}

private void assertQueryOnlyOnTextAndKeyword(String queryName, ThrowingRunnable buildQuery) {
// TODO use runtime type in the error message and a consistent exception type
Exception e = expectThrows(Exception.class, buildQuery);
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
assertThat(
e.getMessage(),
equalTo("Can only use " + queryName + " queries on keyword and text fields - not on [test] which is of type [script]")
equalTo(
"Can only use "
+ queryName
+ " queries on keyword and text fields - not on [test] which is of type [script] with runtime_type ["
+ runtimeType()
+ "]"
)
);
}

private void assertQueryOnlyOnTextKeywordAndWildcard(String queryName, ThrowingRunnable buildQuery) {
// TODO use runtime type in the error message and a consistent exception type
Exception e = expectThrows(Exception.class, buildQuery);
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
assertThat(
e.getMessage(),
equalTo("Can only use " + queryName + " queries on keyword, text and wildcard fields - not on [test] which is of type [script]")
equalTo(
"Can only use "
+ queryName
+ " queries on keyword, text and wildcard fields - not on [test] which is of type [script] with runtime_type ["
+ runtimeType()
+ "]"
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@

import java.io.IOException;

import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

abstract class AbstractScriptMappedFieldTypeTestCase extends ESTestCase {
protected abstract AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException;

protected abstract String runtimeType();

public abstract void testDocValues() throws IOException;

public abstract void testSort() throws IOException;
Expand Down Expand Up @@ -71,4 +76,35 @@ protected static QueryShardContext mockContext(boolean allowExpensiveQueries, Ab
when(context.lookup()).thenReturn(lookup);
return context;
}

public void testPhraseQueryIsError() throws IOException {
assertQueryOnlyOnText("phrase", () -> simpleMappedFieldType().phraseQuery(null, 1, false));
}

public void testPhrasePrefixQueryIsError() throws IOException {
assertQueryOnlyOnText("phrase prefix", () -> simpleMappedFieldType().phrasePrefixQuery(null, 1, 1));
}

public void testMultiPhraseQueryIsError() throws IOException {
assertQueryOnlyOnText("phrase", () -> simpleMappedFieldType().multiPhraseQuery(null, 1, false));
}

public void testSpanPrefixQueryIsError() throws IOException {
assertQueryOnlyOnText("span prefix", () -> simpleMappedFieldType().spanPrefixQuery(null, null, null));
}

private void assertQueryOnlyOnText(String queryName, ThrowingRunnable buildQuery) {
Exception e = expectThrows(IllegalArgumentException.class, buildQuery);
assertThat(
e.getMessage(),
equalTo(
"Can only use "
+ queryName
+ " queries on text fields - not on [test] which is of type [script] with runtime_type ["
+ runtimeType()
+ "]"
)
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,11 @@ protected ScriptDoubleMappedFieldType simpleMappedFieldType() throws IOException
return build("value(source.foo)");
}

@Override
protected String runtimeType() {
return "double";
}

private static ScriptDoubleMappedFieldType build(String code) throws IOException {
return build(new Script(code));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,16 @@ public void testMatchQuery() throws IOException {
}
}

@Override
protected AbstractScriptMappedFieldType simpleMappedFieldType() throws IOException {
return build("value(source.foo)");
}

@Override
protected String runtimeType() {
return "keyword";
}

private static ScriptKeywordMappedFieldType build(String code) throws IOException {
return build(new Script(code));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,11 @@ protected ScriptLongMappedFieldType simpleMappedFieldType() throws IOException {
return build("value(source.foo)");
}

@Override
protected String runtimeType() {
return "long";
}

private static ScriptLongMappedFieldType build(String code) throws IOException {
return build(new Script(code));
}
Expand Down