Skip to content

Commit

Permalink
Disable request cache for non-deterministic runtime fields (#79631)
Browse files Browse the repository at this point in the history 
This change ensures that a non-deterministic script defined in the runtime mapping is not eligible
to the request cache.
It also moves the scripts that extract values from source to be considered deterministic.
Currently a source-only runtime field defined in a search request is not eligible to the request cache.
This commit fixes this discrepancy.
  • Loading branch information
@jimczi
jimczi committed Oct 26, 2021
1 parent 4631c84 commit 5b7211f
Show file tree
Hide file tree
Showing 30 changed files with 260 additions and 163 deletions.
3 changes: 0 additions & 3 deletions ...-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.apache.lucene.search.spans.FieldMaskingSpanQuery;
import org.apache.lucene.search.spans.SpanNearQuery;
import org.apache.lucene.search.spans.SpanTermQuery;
import org.elasticsearch.Version;
import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.index.IndexSettings;
Expand Down Expand Up @@ -67,7 +66,6 @@
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.mockito.Mockito.when;

public class SearchAsYouTypeFieldMapperTests extends MapperTestCase {

Expand Down Expand Up @@ -578,7 +576,6 @@ public void testNestedExistsQuery() throws IOException, ParseException {
b.endObject();
}));
SearchExecutionContext qsc = createSearchExecutionContext(ms);
when(qsc.indexVersionCreated()).thenReturn(Version.CURRENT);
QueryStringQueryParser parser = new QueryStringQueryParser(qsc, "f");
Query q = parser.parse("foo:*");
assertEquals(new ConstantScoreQuery(new BooleanQuery.Builder()
Expand Down
8 changes: 7 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,19 @@ abstract class AbstractScriptFieldType<LeafFactory> extends MappedFieldType {

protected final Script script;
private final Function<SearchLookup, LeafFactory> factory;
private final boolean isResultDeterministic;

AbstractScriptFieldType(
String name,
Function<SearchLookup, LeafFactory> factory,
Script script,
boolean isResultDeterministic,
Map<String, String> meta
) {
super(name, false, false, false, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
this.factory = factory;
this.script = Objects.requireNonNull(script);
this.isResultDeterministic = isResultDeterministic;
}

@Override
Expand Down Expand Up @@ -156,12 +159,15 @@ private String unsupported(String query, String supported) {
);
}

protected final void checkAllowExpensiveQueries(SearchExecutionContext context) {
protected final void applyScriptContext(SearchExecutionContext context) {
if (context.allowExpensiveQueries() == false) {
throw new ElasticsearchException(
"queries cannot be executed against runtime fields while [" + ALLOW_EXPENSIVE_QUERIES.getKey() + "] is set to [false]."
);
}
if (isResultDeterministic == false) {
context.disableCache();
}
}

@Override
Expand Down
13 changes: 7 additions & 6 deletions server/src/main/java/org/elasticsearch/index/mapper/BooleanScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public static RuntimeField sourceOnly(String name) {
Script script,
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta);
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script,
scriptFactory.isResultDeterministic(), meta);
}

@Override
Expand Down Expand Up @@ -107,7 +108,7 @@ public BooleanScriptFieldData.Builder fielddataBuilder(String fullyQualifiedInde

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new BooleanScriptFieldExistsQuery(script, leafFactory(context), name());
}

Expand Down Expand Up @@ -178,13 +179,13 @@ public Query rangeQuery(

@Override
public Query termQueryCaseInsensitive(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new BooleanScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), toBoolean(value, true));
}

@Override
public Query termQuery(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), toBoolean(value, false));
}

Expand All @@ -211,11 +212,11 @@ private Query termsQuery(boolean trueAllowed, boolean falseAllowed, SearchExecut
// Either true or false
return existsQuery(context);
}
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), true);
}
if (falseAllowed) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), false);
}
return new MatchNoDocsQuery("neither true nor false allowed");
Expand Down
12 changes: 6 additions & 6 deletions server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static RuntimeField sourceOnly(String name, DateFormatter dateTimeFormatt
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup, dateTimeFormatter),
script, meta);
script, scriptFactory.isResultDeterministic(), meta);
this.dateTimeFormatter = dateTimeFormatter;
}

Expand Down Expand Up @@ -156,7 +156,7 @@ public DateScriptFieldData.Builder fielddataBuilder(String fullyQualifiedIndexNa

@Override
public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return DateFieldType.handleNow(context, now -> {
long originLong = DateFieldType.parseToLong(
origin,
Expand All @@ -179,7 +179,7 @@ public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionCo

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new LongScriptFieldExistsQuery(script, leafFactory(context)::newInstance, name());
}

Expand All @@ -194,7 +194,7 @@ public Query rangeQuery(
SearchExecutionContext context
) {
parser = parser == null ? dateTimeFormatter.toDateMathParser() : parser;
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return DateFieldType.dateRangeQuery(
lowerTerm,
upperTerm,
Expand All @@ -219,7 +219,7 @@ public Query termQuery(Object value, SearchExecutionContext context) {
now,
DateFieldMapper.Resolution.MILLISECONDS
);
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new LongScriptFieldTermQuery(script, leafFactory(context)::newInstance, name(), l);
});
}
Expand All @@ -243,7 +243,7 @@ public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
)
);
}
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new LongScriptFieldTermsQuery(script, leafFactory(context)::newInstance, name(), terms);
});
}
Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public static RuntimeField sourceOnly(String name) {
Script script,
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta);
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup),
script, scriptFactory.isResultDeterministic(), meta);
}

@Override
Expand Down Expand Up @@ -100,7 +101,7 @@ public DoubleScriptFieldData.Builder fielddataBuilder(String fullyQualifiedIndex

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new DoubleScriptFieldExistsQuery(script, leafFactory(context), name());
}

Expand All @@ -114,7 +115,7 @@ public Query rangeQuery(
DateMathParser parser,
SearchExecutionContext context
) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return NumberType.doubleRangeQuery(
lowerTerm,
upperTerm,
Expand All @@ -126,7 +127,7 @@ public Query rangeQuery(

@Override
public Query termQuery(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new DoubleScriptFieldTermQuery(script, leafFactory(context), name(), NumberType.objectToDouble(value));
}

Expand All @@ -139,7 +140,7 @@ public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
for (Object value : values) {
terms.add(Double.doubleToLongBits(NumberType.objectToDouble(value)));
}
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new DoubleScriptFieldTermsQuery(script, leafFactory(context), name(), terms);
}
}
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ GeoPointFieldScript.Factory getCompositeLeafFactory(
Script script,
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta);
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup),
script, scriptFactory.isResultDeterministic(), meta);
}

@Override
Expand Down Expand Up @@ -100,7 +101,7 @@ public GeoPointScriptFieldData.Builder fielddataBuilder(String fullyQualifiedInd

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new GeoPointScriptFieldExistsQuery(script, leafFactory(context), name());
}

Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/IpScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ IpFieldScript.Factory getCompositeLeafFactory(Function<SearchLookup, CompositeFi
Script script,
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta);
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup),
script, scriptFactory.isResultDeterministic(), meta);
}

@Override
Expand Down Expand Up @@ -99,7 +100,7 @@ public IpScriptFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new IpScriptFieldExistsQuery(script, leafFactory(context), name());
}

Expand All @@ -113,7 +114,7 @@ public Query rangeQuery(
DateMathParser parser,
SearchExecutionContext context
) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return IpFieldMapper.IpFieldType.rangeQuery(
lowerTerm,
upperTerm,
Expand All @@ -131,7 +132,7 @@ public Query rangeQuery(

@Override
public Query termQuery(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
if (value instanceof InetAddress) {
return inetAddressQuery((InetAddress) value, context);
}
Expand All @@ -149,7 +150,7 @@ private Query inetAddressQuery(InetAddress address, SearchExecutionContext conte

@Override
public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
BytesRefHash terms = new BytesRefHash(values.size(), BigArrays.NON_RECYCLING_INSTANCE);
List<Query> cidrQueries = null;
for (Object value : values) {
Expand Down
23 changes: 12 additions & 11 deletions server/src/main/java/org/elasticsearch/index/mapper/KeywordScriptFieldType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public KeywordScriptFieldType(
Script script,
Map<String, String> meta
) {
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup), script, meta);
super(name, searchLookup -> scriptFactory.newFactory(name, script.getParams(), searchLookup),
script, scriptFactory.isResultDeterministic(), meta);
}

@Override
Expand All @@ -102,7 +103,7 @@ public StringScriptFieldData.Builder fielddataBuilder(String fullyQualifiedIndex

@Override
public Query existsQuery(SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldExistsQuery(script, leafFactory(context), name());
}

Expand All @@ -115,7 +116,7 @@ public Query fuzzyQuery(
boolean transpositions,
SearchExecutionContext context
) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return StringScriptFieldFuzzyQuery.build(
script,
leafFactory(context),
Expand All @@ -129,7 +130,7 @@ public Query fuzzyQuery(

@Override
public Query prefixQuery(String value, RewriteMethod method, boolean caseInsensitive, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldPrefixQuery(script, leafFactory(context), name(), value, caseInsensitive);
}

Expand All @@ -143,7 +144,7 @@ public Query rangeQuery(
DateMathParser parser,
SearchExecutionContext context
) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldRangeQuery(
script,
leafFactory(context),
Expand All @@ -164,7 +165,7 @@ public Query regexpQuery(
RewriteMethod method,
SearchExecutionContext context
) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
if (matchFlags != 0) {
throw new IllegalArgumentException("Match flags not yet implemented [" + matchFlags + "]");
}
Expand All @@ -181,7 +182,7 @@ public Query regexpQuery(

@Override
public Query termQueryCaseInsensitive(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldTermQuery(
script,
leafFactory(context),
Expand All @@ -193,7 +194,7 @@ public Query termQueryCaseInsensitive(Object value, SearchExecutionContext conte

@Override
public Query termQuery(Object value, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldTermQuery(
script,
leafFactory(context),
Expand All @@ -205,20 +206,20 @@ public Query termQuery(Object value, SearchExecutionContext context) {

@Override
public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
Set<String> terms = values.stream().map(v -> BytesRefs.toString(Objects.requireNonNull(v))).collect(toSet());
return new StringScriptFieldTermsQuery(script, leafFactory(context), name(), terms);
}

@Override
public Query wildcardQuery(String value, RewriteMethod method, boolean caseInsensitive, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldWildcardQuery(script, leafFactory(context), name(), value, caseInsensitive);
}

@Override
public Query normalizedWildcardQuery(String value, RewriteMethod method, SearchExecutionContext context) {
checkAllowExpensiveQueries(context);
applyScriptContext(context);
return new StringScriptFieldWildcardQuery(script, leafFactory(context), name(), value, false);
}
}

0 comments on commit 5b7211f

Please sign in to comment.