Remove LateParsingQuery to prevent timestamp access after context is frozen (#21328)

Today we still have a leftover from older percolators where lucene
query instances where created ahead of time and rewritten later.
This `LateParsingQuery` was resolving `now()` when it's really used which we
don't need anymore. As a side-effect this failed to execute some highlighting
queries when they get rewritten since at that point `now` access it not permitted
anymore to prevent bugs when queries get cached.

Closes #21295
This commit is contained in:
Simon Willnauer 2016-11-04 13:53:03 +01:00 committed by GitHub
parent 6666fb1614
commit ed2865863c
7 changed files with 93 additions and 158 deletions

View File

@ -794,7 +794,6 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]MoreLikeThisQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]MultiMatchQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]RandomQueryBuilder.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]RangeQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]SpanMultiTermQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]SpanNotQueryBuilderTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]query[/\\]support[/\\]QueryInnerHitsTests.java" checks="LineLength" />

View File

@ -161,71 +161,6 @@ public class DateFieldMapper extends FieldMapper {
}
public static final class DateFieldType extends MappedFieldType {
final class LateParsingQuery extends Query {
final Object lowerTerm;
final Object upperTerm;
final boolean includeLower;
final boolean includeUpper;
final DateTimeZone timeZone;
final DateMathParser forcedDateParser;
private QueryShardContext queryShardContext;
public LateParsingQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
DateTimeZone timeZone, DateMathParser forcedDateParser, QueryShardContext queryShardContext) {
this.lowerTerm = lowerTerm;
this.upperTerm = upperTerm;
this.includeLower = includeLower;
this.includeUpper = includeUpper;
this.timeZone = timeZone;
this.forcedDateParser = forcedDateParser;
this.queryShardContext = queryShardContext;
}
@Override
public Query rewrite(IndexReader reader) throws IOException {
Query rewritten = super.rewrite(reader);
if (rewritten != this) {
return rewritten;
}
return innerRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, queryShardContext);
}
// Even though we only cache rewritten queries it is good to let all queries implement hashCode() and equals():
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (sameClassAs(o) == false) return false;
LateParsingQuery that = (LateParsingQuery) o;
if (includeLower != that.includeLower) return false;
if (includeUpper != that.includeUpper) return false;
if (lowerTerm != null ? !lowerTerm.equals(that.lowerTerm) : that.lowerTerm != null) return false;
if (upperTerm != null ? !upperTerm.equals(that.upperTerm) : that.upperTerm != null) return false;
if (timeZone != null ? !timeZone.equals(that.timeZone) : that.timeZone != null) return false;
return true;
}
@Override
public int hashCode() {
return Objects.hash(classHash(), lowerTerm, upperTerm, includeLower, includeUpper, timeZone);
}
@Override
public String toString(String s) {
final StringBuilder sb = new StringBuilder();
return sb.append(name()).append(':')
.append(includeLower ? '[' : '{')
.append((lowerTerm == null) ? "*" : lowerTerm.toString())
.append(" TO ")
.append((upperTerm == null) ? "*" : upperTerm.toString())
.append(includeUpper ? ']' : '}')
.toString();
}
}
protected FormatDateTimeFormatter dateTimeFormatter;
protected DateMathParser dateMathParser;
@ -317,7 +252,7 @@ public class DateFieldMapper extends FieldMapper {
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
@Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser, QueryShardContext context) {
failIfNotIndexed();
return new LateParsingQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, context);
return innerRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, context);
}
Query innerRangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,

View File

@ -176,70 +176,6 @@ public class LegacyDateFieldMapper extends LegacyNumberFieldMapper {
public static class DateFieldType extends NumberFieldType {
final class LateParsingQuery extends Query {
final Object lowerTerm;
final Object upperTerm;
final boolean includeLower;
final boolean includeUpper;
final DateTimeZone timeZone;
final DateMathParser forcedDateParser;
private QueryShardContext context;
public LateParsingQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, DateTimeZone timeZone,
DateMathParser forcedDateParser, QueryShardContext context) {
this.lowerTerm = lowerTerm;
this.upperTerm = upperTerm;
this.includeLower = includeLower;
this.includeUpper = includeUpper;
this.timeZone = timeZone;
this.forcedDateParser = forcedDateParser;
this.context = context;
}
@Override
public Query rewrite(IndexReader reader) throws IOException {
Query rewritten = super.rewrite(reader);
if (rewritten != this) {
return rewritten;
}
return innerRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, context);
}
// Even though we only cache rewritten queries it is good to let all queries implement hashCode() and equals():
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (sameClassAs(o) == false) return false;
LateParsingQuery that = (LateParsingQuery) o;
if (includeLower != that.includeLower) return false;
if (includeUpper != that.includeUpper) return false;
if (lowerTerm != null ? !lowerTerm.equals(that.lowerTerm) : that.lowerTerm != null) return false;
if (upperTerm != null ? !upperTerm.equals(that.upperTerm) : that.upperTerm != null) return false;
if (timeZone != null ? !timeZone.equals(that.timeZone) : that.timeZone != null) return false;
return true;
}
@Override
public int hashCode() {
return Objects.hash(classHash(), lowerTerm, upperTerm, includeLower, includeUpper, timeZone);
}
@Override
public String toString(String s) {
final StringBuilder sb = new StringBuilder();
return sb.append(name()).append(':')
.append(includeLower ? '[' : '{')
.append((lowerTerm == null) ? "*" : lowerTerm.toString())
.append(" TO ")
.append((upperTerm == null) ? "*" : upperTerm.toString())
.append(includeUpper ? ']' : '}')
.toString();
}
}
protected FormatDateTimeFormatter dateTimeFormatter = Defaults.DATE_TIME_FORMATTER;
protected TimeUnit timeUnit = Defaults.TIME_UNIT;
protected DateMathParser dateMathParser = new DateMathParser(dateTimeFormatter);
@ -371,7 +307,7 @@ public class LegacyDateFieldMapper extends LegacyNumberFieldMapper {
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
@Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser, QueryShardContext context) {
return new LateParsingQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, context);
return innerRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, forcedDateParser, context);
}
private Query innerRangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,

View File

@ -260,6 +260,10 @@ public class RangeQueryBuilder extends AbstractQueryBuilder<RangeQueryBuilder> i
return this.timeZone == null ? null : this.timeZone.getID();
}
DateTimeZone getDateTimeZone() { // for testing
return timeZone;
}
/**
* In case of format field, we can parse the from/to fields using this time format
*/
@ -278,6 +282,13 @@ public class RangeQueryBuilder extends AbstractQueryBuilder<RangeQueryBuilder> i
return this.format == null ? null : this.format.format();
}
DateMathParser getForceDateParser() { // pkg private for testing
if (this.format != null) {
return new DateMathParser(this.format);
}
return null;
}
@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
@ -440,19 +451,13 @@ public class RangeQueryBuilder extends AbstractQueryBuilder<RangeQueryBuilder> i
MappedFieldType mapper = context.fieldMapper(this.fieldName);
if (mapper != null) {
if (mapper instanceof LegacyDateFieldMapper.DateFieldType) {
DateMathParser forcedDateParser = null;
if (this.format != null) {
forcedDateParser = new DateMathParser(this.format);
}
query = ((LegacyDateFieldMapper.DateFieldType) mapper).rangeQuery(from, to, includeLower, includeUpper,
timeZone, forcedDateParser, context);
timeZone, getForceDateParser(), context);
} else if (mapper instanceof DateFieldMapper.DateFieldType) {
DateMathParser forcedDateParser = null;
if (this.format != null) {
forcedDateParser = new DateMathParser(this.format);
}
query = ((DateFieldMapper.DateFieldType) mapper).rangeQuery(from, to, includeLower, includeUpper,
timeZone, forcedDateParser, context);
timeZone, getForceDateParser(), context);
} else {
if (timeZone != null) {
throw new QueryShardException(context, "[range] time_zone can not be applied to non date field ["

View File

@ -256,7 +256,7 @@ public class LegacyDateFieldMapperTests extends ESSingleNodeTestCase {
assertThat(((LegacyLongFieldMapper.CustomLongNumericField) doc.rootDoc().getField("date_field")).numericAsString(), equalTo(Long.toString(new DateTime(TimeValue.timeValueHours(10).millis(), DateTimeZone.UTC).getMillis())));
LegacyNumericRangeQuery<Long> rangeQuery = (LegacyNumericRangeQuery<Long>) defaultMapper.mappers().smartNameFieldMapper("date_field").fieldType()
.rangeQuery("10:00:00", "11:00:00", true, true, context).rewrite(null);
.rangeQuery("10:00:00", "11:00:00", true, true, context);
assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(11).millis(), DateTimeZone.UTC).getMillis() + 999));
assertThat(rangeQuery.getMin(), equalTo(new DateTime(TimeValue.timeValueHours(10).millis(), DateTimeZone.UTC).getMillis()));
}
@ -283,7 +283,7 @@ public class LegacyDateFieldMapperTests extends ESSingleNodeTestCase {
assertThat(((LegacyLongFieldMapper.CustomLongNumericField) doc.rootDoc().getField("date_field")).numericAsString(), equalTo(Long.toString(new DateTime(TimeValue.timeValueHours(34).millis(), DateTimeZone.UTC).getMillis())));
LegacyNumericRangeQuery<Long> rangeQuery = (LegacyNumericRangeQuery<Long>) defaultMapper.mappers().smartNameFieldMapper("date_field").fieldType()
.rangeQuery("Jan 02 10:00:00", "Jan 02 11:00:00", true, true, context).rewrite(null);
.rangeQuery("Jan 02 10:00:00", "Jan 02 11:00:00", true, true, context);
assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(35).millis() + 999, DateTimeZone.UTC).getMillis()));
assertThat(rangeQuery.getMin(), equalTo(new DateTime(TimeValue.timeValueHours(34).millis(), DateTimeZone.UTC).getMillis()));
}

View File

@ -29,8 +29,11 @@ import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.index.mapper.DateFieldMapper;
import org.elasticsearch.index.mapper.LegacyDateFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MappedFieldType.Relation;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;
import org.joda.time.DateTime;
@ -118,7 +121,8 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
@Override
protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException {
if (getCurrentTypes().length == 0 || (queryBuilder.fieldName().equals(DATE_FIELD_NAME) == false && queryBuilder.fieldName().equals(INT_FIELD_NAME) == false)) {
if (getCurrentTypes().length == 0 || (queryBuilder.fieldName().equals(DATE_FIELD_NAME) == false
&& queryBuilder.fieldName().equals(INT_FIELD_NAME) == false)) {
assertThat(query, instanceOf(TermRangeQuery.class));
TermRangeQuery termRangeQuery = (TermRangeQuery) query;
assertThat(termRangeQuery.getField(), equalTo(queryBuilder.fieldName()));
@ -127,7 +131,68 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
assertThat(termRangeQuery.includesLower(), equalTo(queryBuilder.includeLower()));
assertThat(termRangeQuery.includesUpper(), equalTo(queryBuilder.includeUpper()));
} else if (queryBuilder.fieldName().equals(DATE_FIELD_NAME)) {
//we can't properly test unmapped dates because LateParsingQuery is package private
assertThat(query, either(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
MapperService mapperService = context.getQueryShardContext().getMapperService();
MappedFieldType mappedFieldType = mapperService.fullName(DATE_FIELD_NAME);
final Long fromInMillis;
final Long toInMillis;
// we have to normalize the incoming value into milliseconds since it could be literally anything
if (mappedFieldType instanceof LegacyDateFieldMapper.DateFieldType) {
fromInMillis = queryBuilder.from() == null ? null :
((LegacyDateFieldMapper.DateFieldType) mappedFieldType).parseToMilliseconds(queryBuilder.from(),
queryBuilder.includeLower(),
queryBuilder.getDateTimeZone(),
queryBuilder.getForceDateParser(), context.getQueryShardContext());
toInMillis = queryBuilder.to() == null ? null :
((LegacyDateFieldMapper.DateFieldType) mappedFieldType).parseToMilliseconds(queryBuilder.to(),
queryBuilder.includeUpper(),
queryBuilder.getDateTimeZone(),
queryBuilder.getForceDateParser(), context.getQueryShardContext());
} else if (mappedFieldType instanceof DateFieldMapper.DateFieldType) {
fromInMillis = queryBuilder.from() == null ? null :
((DateFieldMapper.DateFieldType) mappedFieldType).parseToMilliseconds(queryBuilder.from(),
queryBuilder.includeLower(),
queryBuilder.getDateTimeZone(),
queryBuilder.getForceDateParser(), context.getQueryShardContext());
toInMillis = queryBuilder.to() == null ? null :
((DateFieldMapper.DateFieldType) mappedFieldType).parseToMilliseconds(queryBuilder.to(),
queryBuilder.includeUpper(),
queryBuilder.getDateTimeZone(),
queryBuilder.getForceDateParser(), context.getQueryShardContext());
} else {
fromInMillis = toInMillis = null;
fail("unexpected mapped field type: [" + mappedFieldType.getClass() + "] " + mappedFieldType.toString());
}
if (query instanceof LegacyNumericRangeQuery) {
LegacyNumericRangeQuery numericRangeQuery = (LegacyNumericRangeQuery) query;
assertThat(numericRangeQuery.getField(), equalTo(queryBuilder.fieldName()));
assertThat(numericRangeQuery.getMin(), equalTo(fromInMillis));
assertThat(numericRangeQuery.getMax(), equalTo(toInMillis));
assertThat(numericRangeQuery.includesMin(), equalTo(queryBuilder.includeLower()));
assertThat(numericRangeQuery.includesMax(), equalTo(queryBuilder.includeUpper()));
} else {
Long min = fromInMillis;
Long max = toInMillis;
long minLong, maxLong;
if (min == null) {
minLong = Long.MIN_VALUE;
} else {
minLong = min.longValue();
if (queryBuilder.includeLower() == false && minLong != Long.MAX_VALUE) {
minLong++;
}
}
if (max == null) {
maxLong = Long.MAX_VALUE;
} else {
maxLong = max.longValue();
if (queryBuilder.includeUpper() == false && maxLong != Long.MIN_VALUE) {
maxLong--;
}
}
assertEquals(LongPoint.newRangeQuery(DATE_FIELD_NAME, minLong, maxLong), query);
}
} else if (queryBuilder.fieldName().equals(INT_FIELD_NAME)) {
assertThat(query, either(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
if (query instanceof LegacyNumericRangeQuery) {
@ -157,11 +222,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
maxInt--;
}
}
try {
assertEquals(IntPoint.newRangeQuery(INT_FIELD_NAME, minInt, maxInt), query);
}catch(AssertionError e) {
throw e;
}
}
} else {
throw new UnsupportedOperationException();
@ -228,7 +289,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
" }\n" +
" }\n" +
"}";
Query parsedQuery = parseQuery(query).toQuery(createShardContext()).rewrite(null);
Query parsedQuery = parseQuery(query).toQuery(createShardContext());
assertThat(parsedQuery, either(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
if (parsedQuery instanceof LegacyNumericRangeQuery) {
@ -256,8 +317,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
" }\n" +
" }\n" +
"}";
Query rewrittenQuery = parseQuery(invalidQuery).toQuery(createShardContext());
expectThrows(ElasticsearchParseException.class, () -> rewrittenQuery.rewrite(null));
expectThrows(ElasticsearchParseException.class, () -> parseQuery(invalidQuery).toQuery(createShardContext()));
}
public void testDateRangeBoundaries() throws IOException {
@ -270,7 +330,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
" }\n" +
" }\n" +
"}\n";
Query parsedQuery = parseQuery(query).toQuery(createShardContext()).rewrite(null);
Query parsedQuery = parseQuery(query).toQuery(createShardContext());
assertThat(parsedQuery, either(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
if (parsedQuery instanceof LegacyNumericRangeQuery) {
LegacyNumericRangeQuery rangeQuery = (LegacyNumericRangeQuery) parsedQuery;
@ -297,7 +357,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
" }\n" +
" }\n" +
"}";
parsedQuery = parseQuery(query).toQuery(createShardContext()).rewrite(null);
parsedQuery = parseQuery(query).toQuery(createShardContext());
assertThat(parsedQuery, either(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
if (parsedQuery instanceof LegacyNumericRangeQuery) {
LegacyNumericRangeQuery rangeQuery = (LegacyNumericRangeQuery) parsedQuery;
@ -330,7 +390,7 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase<RangeQueryBuil
" }\n" +
"}";
QueryShardContext context = createShardContext();
Query parsedQuery = parseQuery(query).toQuery(context).rewrite(null);
Query parsedQuery = parseQuery(query).toQuery(context);
if (parsedQuery instanceof PointRangeQuery) {
// TODO what can we assert
} else {

View File

@ -2974,12 +2974,12 @@ public class HighlighterSearchIT extends ESIntegTestCase {
.preTags("<x>")
.postTags("</x>")
).setQuery(QueryBuilders.boolQuery().must(
QueryBuilders.rangeQuery("d").gte("now-7d/d").lte("now").includeLower(true).includeUpper(true).boost(1.0f))
QueryBuilders.rangeQuery("d").gte("now-12h").lte("now").includeLower(true).includeUpper(true).boost(1.0f))
.should(QueryBuilders.termQuery("field", "hello")))
.get();
assertSearchResponse(r1);
assertThat(r1.getHits().getTotalHits(), equalTo(3L));
assertThat(r1.getHits().getTotalHits(), equalTo(1L));
assertHighlight(r1, 0, "field", 0, 1,
equalTo("<x>hello</x> world"));
}