Skip to content

Check field range with doc values skipper for date field #20287

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.

Sign up for GitHub

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

Open
bugmakerrrrrr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check field range with doc values skipper for date field #20287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 47 additions & 15 deletions server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.SortedNumericDocValuesField;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.DocValuesSkipper;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.PointValues;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.IndexOrDocValuesQuery;
Expand Down Expand Up @@ -375,6 +377,7 @@ public DateFieldMapper build(BuilderContext context) {
index.getValue(),
store.getValue(),
docValues.getValue(),
skiplist.getValue(),
buildFormatter(),
resolution,
nullValue.getValue(),
Expand Down Expand Up @@ -411,38 +414,41 @@ public static final class DateFieldType extends MappedFieldType implements Numer
protected final DateMathParser dateMathParser;
protected final Resolution resolution;
protected final String nullValue;
private final boolean hasSkiplist;

public DateFieldType(
String name,
boolean isSearchable,
boolean isStored,
boolean hasDocValues,
boolean hasSkiplist,
DateFormatter dateTimeFormatter,
Resolution resolution,
String nullValue,
Map<String, String> meta
) {
super(name, isSearchable, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_ONLY, meta);
this.hasSkiplist = hasSkiplist;
this.dateTimeFormatter = dateTimeFormatter;
this.dateMathParser = dateTimeFormatter.toDateMathParser();
this.resolution = resolution;
this.nullValue = nullValue;
}

public DateFieldType(String name) {
this(name, true, false, true, getDefaultDateTimeFormatter(), Resolution.MILLISECONDS, null, Collections.emptyMap());
this(name, true, false, true, false, getDefaultDateTimeFormatter(), Resolution.MILLISECONDS, null, Collections.emptyMap());
}

public DateFieldType(String name, DateFormatter dateFormatter) {
this(name, true, false, true, dateFormatter, Resolution.MILLISECONDS, null, Collections.emptyMap());
this(name, true, false, true, false, dateFormatter, Resolution.MILLISECONDS, null, Collections.emptyMap());
}

public DateFieldType(String name, Resolution resolution) {
this(name, true, false, true, getDefaultDateTimeFormatter(), resolution, null, Collections.emptyMap());
this(name, true, false, true, false, getDefaultDateTimeFormatter(), resolution, null, Collections.emptyMap());
}

public DateFieldType(String name, Resolution resolution, DateFormatter dateFormatter) {
this(name, true, false, true, dateFormatter, resolution, null, Collections.emptyMap());
this(name, true, false, true, false, dateFormatter, resolution, null, Collections.emptyMap());
}

@Override
Expand Down Expand Up @@ -664,10 +670,44 @@ public Relation isFieldWithinQuery(
DateMathParser dateParser,
QueryRewriteContext context
) throws IOException {
// if we have only doc_values enabled we do not look at the BKD so we return an INTERSECTS by default
if (isSearchable() == false && hasDocValues()) {
return Relation.INTERSECTS;
final long minValue, maxValue;
if (isSearchable()) {
if (PointValues.size(reader, name()) == 0) {
// no points, so nothing matches
return Relation.DISJOINT;
}

minValue = LongPoint.decodeDimension(PointValues.getMinPackedValue(reader, name()), 0);
maxValue = LongPoint.decodeDimension(PointValues.getMaxPackedValue(reader, name()), 0);
} else if (hasDocValues()) {
if (hasSkiplist == false) {
return Relation.INTERSECTS;
}

long globalMin = Long.MAX_VALUE;
long globalMax = Long.MIN_VALUE;
for (LeafReaderContext ctx : reader.leaves()) {
if (ctx.reader().getFieldInfos().fieldInfo(name()) == null) {
continue; // no field values in this segment, so we can ignore it
}
DocValuesSkipper skipper = ctx.reader().getDocValuesSkipper(name());
if (skipper == null) {
// cannot be computed correctly since skipper is not enabled for some leaf
return Relation.INTERSECTS;
} else {
globalMin = Math.min(globalMin, skipper.minValue());
globalMax = Math.max(globalMax, skipper.maxValue());
}
}
if (globalMin > globalMax) {
return Relation.DISJOINT;
}
minValue = globalMin;
maxValue = globalMax;
} else {
return Relation.DISJOINT;
}

if (dateParser == null) {
dateParser = this.dateMathParser;
}
Expand All @@ -694,14 +734,6 @@ public Relation isFieldWithinQuery(
}
}

if (PointValues.size(reader, name()) == 0) {
// no points, so nothing matches
return Relation.DISJOINT;
}

long minValue = LongPoint.decodeDimension(PointValues.getMinPackedValue(reader, name()), 0);
long maxValue = LongPoint.decodeDimension(PointValues.getMaxPackedValue(reader, name()), 0);

if (minValue >= fromInclusive && maxValue <= toInclusive) {
return Relation.WITHIN;
} else if (maxValue < fromInclusive || minValue > toInclusive) {
Expand Down
79 changes: 58 additions & 21 deletions server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.function.BiFunction;

import static org.opensearch.index.mapper.DateFieldMapper.getDefaultDateTimeFormatter;
import static org.apache.lucene.document.LongPoint.pack;

public class DateFieldTypeTests extends FieldTypeTestCase {
Expand All @@ -107,29 +109,61 @@ public void testIsFieldWithinRangeEmptyReader() throws IOException {
);
}

public void testIsFieldWithinQueryDateMillis() throws IOException {
public void testIsFieldWithinQueryDateMillisBasedPointIndex() throws IOException {
DateFieldType ft = new DateFieldType("my_date", Resolution.MILLISECONDS);
isFieldWithinRangeTestCase(ft);
isFieldWithinRangeTestCase(ft, LongPoint::new);
}

public void testIsFieldWithinQueryDateNanos() throws IOException {
public void testIsFieldWithinQueryDateNanosBasedPointIndex() throws IOException {
DateFieldType ft = new DateFieldType("my_date", Resolution.NANOSECONDS);
isFieldWithinRangeTestCase(ft);
isFieldWithinRangeTestCase(ft, LongPoint::new);
}

public void isFieldWithinRangeTestCase(DateFieldType ft) throws IOException {
public void testIsFieldWithinQueryDateMillisBasedDVSkipper() throws IOException {
DateFieldType ft = new DateFieldType(
"my_date",
false,
false,
true,
true,
getDefaultDateTimeFormatter(),
Resolution.MILLISECONDS,
null,
Collections.emptyMap()
);
isFieldWithinRangeTestCase(ft, SortedNumericDocValuesField::indexedField);
}

public void testIsFieldWithinQueryDateNanosBasedDVSkipper() throws IOException {
DateFieldType ft = new DateFieldType(
"my_date",
false,
false,
true,
true,
getDefaultDateTimeFormatter(),
Resolution.NANOSECONDS,
null,
Collections.emptyMap()
);
isFieldWithinRangeTestCase(ft, SortedNumericDocValuesField::indexedField);
}

public void isFieldWithinRangeTestCase(DateFieldType ft, BiFunction<String, Long, IndexableField> fieldBuilder) throws IOException {

Directory dir = newDirectory();
IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));
Document doc = new Document();
LongPoint field = new LongPoint("my_date", ft.parse("2015-10-12"));
IndexableField field = fieldBuilder.apply("my_date", ft.parse("2015-10-12"));
doc.add(field);
w.addDocument(doc);
field.setLongValue(ft.parse("2016-04-03"));
doc = new Document();
field = fieldBuilder.apply("my_date", ft.parse("2016-04-03"));
doc.add(field);
w.addDocument(doc);
DirectoryReader reader = DirectoryReader.open(w);

DateMathParser alternateFormat = DateFieldMapper.getDefaultDateTimeFormatter().toDateMathParser();
DateMathParser alternateFormat = getDefaultDateTimeFormatter().toDateMathParser();
doTestIsFieldWithinQuery(ft, reader, null, null);
doTestIsFieldWithinQuery(ft, reader, null, alternateFormat);
doTestIsFieldWithinQuery(ft, reader, DateTimeZone.UTC, null);
Expand Down Expand Up @@ -180,23 +214,21 @@ private void doTestIsFieldWithinQuery(DateFieldType ft, DirectoryReader reader,

public void testValueFormat() {
MappedFieldType ft = new DateFieldType("field");
long instant = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse("2015-10-12T14:10:55"))
.toInstant()
.toEpochMilli();
long instant = DateFormatters.from(getDefaultDateTimeFormatter().parse("2015-10-12T14:10:55")).toInstant().toEpochMilli();

assertEquals("2015-10-12T14:10:55.000Z", ft.docValueFormat(null, ZoneOffset.UTC).format(instant));
assertEquals("2015-10-12T15:10:55.000+01:00", ft.docValueFormat(null, ZoneOffset.ofHours(1)).format(instant));
assertEquals("2015", new DateFieldType("field").docValueFormat("YYYY", ZoneOffset.UTC).format(instant));
assertEquals(instant, ft.docValueFormat(null, ZoneOffset.UTC).parseLong("2015-10-12T14:10:55", false, null));
assertEquals(instant + 999, ft.docValueFormat(null, ZoneOffset.UTC).parseLong("2015-10-12T14:10:55", true, null));
long i = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse("2015-10-13")).toInstant().toEpochMilli();
long i = DateFormatters.from(getDefaultDateTimeFormatter().parse("2015-10-13")).toInstant().toEpochMilli();
assertEquals(i - 1, ft.docValueFormat(null, ZoneOffset.UTC).parseLong("2015-10-12||/d", true, null));
}

public void testValueForSearch() {
MappedFieldType ft = new DateFieldType("field");
String date = "2015-10-12T12:09:55.000Z";
long instant = DateFieldMapper.getDefaultDateTimeFormatter().parseMillis(date);
long instant = getDefaultDateTimeFormatter().parseMillis(date);
assertEquals(date, ft.valueForDisplay(instant));
}

Expand Down Expand Up @@ -227,7 +259,7 @@ public void testTermQuery() {
);
MappedFieldType ft = new DateFieldType("field");
String date = "2015-10-12T14:10:55";
long instant = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date)).toInstant().toEpochMilli();
long instant = DateFormatters.from(getDefaultDateTimeFormatter().parse(date)).toInstant().toEpochMilli();
Query expected = new ApproximateScoreQuery(
new IndexOrDocValuesQuery(
LongPoint.newRangeQuery("field", instant, instant + 999),
Expand All @@ -248,7 +280,8 @@ public void testTermQuery() {
false,
false,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
false,
getDefaultDateTimeFormatter(),
Resolution.MILLISECONDS,
null,
Collections.emptyMap()
Expand Down Expand Up @@ -285,8 +318,8 @@ public void testRangeQuery() throws IOException {
MappedFieldType ft = new DateFieldType("field");
String date1 = "2015-10-12T14:10:55";
String date2 = "2016-04-28T11:33:52";
long instant1 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli();
long instant2 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999;
long instant1 = DateFormatters.from(getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli();
long instant2 = DateFormatters.from(getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999;
ApproximatePointRangeQuery approximatePointRangeQuery = new ApproximatePointRangeQuery(
"field",
pack(new long[] { instant1 }).bytes,
Expand Down Expand Up @@ -331,7 +364,8 @@ public void testRangeQuery() throws IOException {
false,
false,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
false,
getDefaultDateTimeFormatter(),
Resolution.MILLISECONDS,
null,
Collections.emptyMap()
Expand Down Expand Up @@ -377,8 +411,8 @@ public void testRangeQueryWithIndexSort() {
MappedFieldType ft = new DateFieldType("field");
String date1 = "2015-10-12T14:10:55";
String date2 = "2016-04-28T11:33:52";
long instant1 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli();
long instant2 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999;
long instant1 = DateFormatters.from(getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli();
long instant2 = DateFormatters.from(getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999;

Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2);
Query expected = new ApproximateScoreQuery(
Expand Down Expand Up @@ -429,7 +463,7 @@ public void testDateNanoDocValues() throws IOException {

private static DateFieldType fieldType(Resolution resolution, String format, String nullValue) {
DateFormatter formatter = DateFormatter.forPattern(format);
return new DateFieldType("field", true, false, true, formatter, resolution, nullValue, Collections.emptyMap());
return new DateFieldType("field", true, false, true, false, formatter, resolution, nullValue, Collections.emptyMap());
}

public void testFetchSourceValue() throws IOException {
Expand Down Expand Up @@ -481,6 +515,7 @@ public void testDateResolutionForOverflow() throws IOException {
true,
true,
true,
false,
DateFormatter.forPattern("yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis||strict_date_optional_time"),
Resolution.MILLISECONDS,
null,
Expand Down Expand Up @@ -579,6 +614,7 @@ public void testDateResolutionForOverflow() throws IOException {
true,
true,
true,
false,
DateFormatter.forPattern("yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis||strict_date_optional_time"),
Resolution.MILLISECONDS,
"2020-01-01T00:00:00Z",
Expand All @@ -598,6 +634,7 @@ public void testDateFieldTypeWithNulls() throws IOException {
true,
true,
true,
false,
DateFormatter.forPattern("yyyy-MM-dd HH:mm:ss||yyyy-MM-dd||epoch_millis||date_optional_time"),
Resolution.MILLISECONDS,
null,
Expand Down
1 change: 1 addition & 0 deletions server/src/test/java/org/opensearch/search/aggregations/AggregatorBaseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ private ValuesSourceConfig getVSConfig(
indexed,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
resolution,
null,
Expand Down
1 change: 1 addition & 0 deletions ...ava/org/opensearch/search/aggregations/bucket/filterrewrite/FilterRewriteSubAggTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ protected final DateFieldMapper.DateFieldType aggregableDateFieldType(boolean us
isSearchable,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
useNanosecondResolution ? DateFieldMapper.Resolution.NANOSECONDS : DateFieldMapper.Resolution.MILLISECONDS,
null,
Expand Down
1 change: 1 addition & 0 deletions .../org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregatorTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ protected final DateFieldMapper.DateFieldType aggregableDateFieldType(boolean us
isSearchable,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
useNanosecondResolution ? DateFieldMapper.Resolution.NANOSECONDS : DateFieldMapper.Resolution.MILLISECONDS,
null,
Expand Down
1 change: 1 addition & 0 deletions ...c/test/java/org/opensearch/search/aggregations/bucket/range/DateRangeAggregatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ private void testCase(
true,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
resolution,
null,
Expand Down
2 changes: 2 additions & 0 deletions ...r/src/test/java/org/opensearch/search/aggregations/bucket/range/RangeAggregatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public void testDateFieldNanosecondResolution() throws IOException {
true,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
DateFieldMapper.Resolution.NANOSECONDS,
null,
Expand Down Expand Up @@ -191,6 +192,7 @@ public void testMissingDateWithDateField() throws IOException {
true,
false,
true,
false,
DateFieldMapper.getDefaultDateTimeFormatter(),
DateFieldMapper.Resolution.NANOSECONDS,
null,
Expand Down
1 change: 1 addition & 0 deletions .../test/java/org/opensearch/search/aggregations/bucket/terms/MultiTermsAggregatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,7 @@ private static DateFieldMapper.DateFieldType dateFieldType(String name) {
true,
false,
true,
false,
DateFormatter.forPattern("date"),
DateFieldMapper.Resolution.MILLISECONDS,
null,
Expand Down
Loading