honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix some `range` query edge cases (#41160) 

Currently we throw an error when a range querys minimum value exceeds the
maximum value due to the fact that they are neighbouring values and both upper
and lower value are excluded from the interval.

Since this is a condition that the user usually doesn't specify conciously (at
least in the case of float and double values its difficult to see which values
are adjacent) we should ignore those "wrong" intervals and create a
MatchNoDocsQuery in those cases.

We should still throw errors with an actionable message if the user specifies
the query interval in a way that min value > max value. This PR adds those
checks and tests for those cases.

Closes #40937
This commit is contained in:
Christoph Büscher 2019-04-16 10:03:35 +02:00
parent 36a8c7aa0b
commit f8161ffa88
2 changed files with 209 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
View File

@ -35,10 +35,12 @@ import org.apache.lucene.queries.BinaryDocValuesRangeQuery.QueryType;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
import org.apache.lucene.search.IndexOrDocValuesQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.store.ByteArrayDataOutput;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.FutureArrays;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Nullable;
@ -70,6 +72,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;
import static org.elasticsearch.index.query.RangeQueryBuilder.GTE_FIELD;
import static org.elasticsearch.index.query.RangeQueryBuilder.GT_FIELD;
@ -516,25 +519,38 @@ public class RangeFieldMapper extends FieldMapper {
}
@Override
public Query withinQuery(String field, Object from, Object to, boolean includeLower, boolean includeUpper) {
InetAddress lower = (InetAddress)from;
InetAddress upper = (InetAddress)to;
return InetAddressRange.newWithinQuery(field,
includeLower ? lower : nextUp(lower), includeUpper ? upper : nextDown(upper));
public Query withinQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return createQuery(field, from, to, includeFrom, includeTo,
(f, t) -> InetAddressRange.newWithinQuery(field, f, t));
}
@Override
public Query containsQuery(String field, Object from, Object to, boolean includeLower, boolean includeUpper) {
InetAddress lower = (InetAddress)from;
InetAddress upper = (InetAddress)to;
return InetAddressRange.newContainsQuery(field,
includeLower ? lower : nextUp(lower), includeUpper ? upper : nextDown(upper));
public Query containsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return createQuery(field, from, to, includeFrom, includeTo,
(f, t) -> InetAddressRange.newContainsQuery(field, f, t ));
}
@Override
public Query intersectsQuery(String field, Object from, Object to, boolean includeLower, boolean includeUpper) {
InetAddress lower = (InetAddress)from;
InetAddress upper = (InetAddress)to;
return InetAddressRange.newIntersectsQuery(field,
includeLower ? lower : nextUp(lower), includeUpper ? upper : nextDown(upper));
public Query intersectsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return createQuery(field, from, to, includeFrom, includeTo,
(f, t) -> InetAddressRange.newIntersectsQuery(field, f ,t ));
}
private Query createQuery(String field, Object lower, Object upper, boolean includeLower, boolean includeUpper,
BiFunction<InetAddress, InetAddress, Query> querySupplier) {
byte[] lowerBytes = InetAddressPoint.encode((InetAddress) lower);
byte[] upperBytes = InetAddressPoint.encode((InetAddress) upper);
if (FutureArrays.compareUnsigned(lowerBytes, 0, lowerBytes.length, upperBytes, 0, upperBytes.length) > 0) {
throw new IllegalArgumentException(
"Range query `from` value (" + lower + ") is greater than `to` value (" + upper + ")");
}
InetAddress correctedFrom = includeLower ? (InetAddress) lower : nextUp(lower);
InetAddress correctedTo = includeUpper ? (InetAddress) upper : nextDown(upper);;
lowerBytes = InetAddressPoint.encode(correctedFrom);
upperBytes = InetAddressPoint.encode(correctedTo);
if (FutureArrays.compareUnsigned(lowerBytes, 0, lowerBytes.length, upperBytes, 0, upperBytes.length) > 0) {
return new MatchNoDocsQuery("float range didn't intersect anything");
} else {
return querySupplier.apply(correctedFrom, correctedTo);
}
}
},
DATE("date_range", NumberType.LONG) {
@ -662,21 +678,18 @@ public class RangeFieldMapper extends FieldMapper {
}
@Override
public Query withinQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return FloatRange.newWithinQuery(field,
new float[] {includeFrom ? (Float)from : Math.nextUp((Float)from)},
new float[] {includeTo ? (Float)to : Math.nextDown((Float)to)});
return createQuery(field, (Float) from, (Float) to, includeFrom, includeTo,
(f, t) -> FloatRange.newWithinQuery(field, new float[] { f }, new float[] { t }), RangeType.FLOAT);
}
@Override
public Query containsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return FloatRange.newContainsQuery(field,
new float[] {includeFrom ? (Float)from : Math.nextUp((Float)from)},
new float[] {includeTo ? (Float)to : Math.nextDown((Float)to)});
return createQuery(field, (Float) from, (Float) to, includeFrom, includeTo,
(f, t) -> FloatRange.newContainsQuery(field, new float[] { f }, new float[] { t }), RangeType.FLOAT);
}
@Override
public Query intersectsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return FloatRange.newIntersectsQuery(field,
new float[] {includeFrom ? (Float)from : Math.nextUp((Float)from)},
new float[] {includeTo ? (Float)to : Math.nextDown((Float)to)});
return createQuery(field, (Float) from, (Float) to, includeFrom, includeTo,
(f, t) -> FloatRange.newIntersectsQuery(field, new float[] { f }, new float[] { t }), RangeType.FLOAT);
}
},
DOUBLE("double_range", NumberType.DOUBLE) {
@ -724,22 +737,20 @@ public class RangeFieldMapper extends FieldMapper {
}
@Override
public Query withinQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return DoubleRange.newWithinQuery(field,
new double[] {includeFrom ? (Double)from : Math.nextUp((Double)from)},
new double[] {includeTo ? (Double)to : Math.nextDown((Double)to)});
return createQuery(field, (Double) from, (Double) to, includeFrom, includeTo,
(f, t) -> DoubleRange.newWithinQuery(field, new double[] { f }, new double[] { t }), RangeType.DOUBLE);
}
@Override
public Query containsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return DoubleRange.newContainsQuery(field,
new double[] {includeFrom ? (Double)from : Math.nextUp((Double)from)},
new double[] {includeTo ? (Double)to : Math.nextDown((Double)to)});
return createQuery(field, (Double) from, (Double) to, includeFrom, includeTo,
(f, t) -> DoubleRange.newContainsQuery(field, new double[] { f }, new double[] { t }), RangeType.DOUBLE);
}
@Override
public Query intersectsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return DoubleRange.newIntersectsQuery(field,
new double[] {includeFrom ? (Double)from : Math.nextUp((Double)from)},
new double[] {includeTo ? (Double)to : Math.nextDown((Double)to)});
return createQuery(field, (Double) from, (Double) to, includeFrom, includeTo,
(f, t) -> DoubleRange.newIntersectsQuery(field, new double[] { f }, new double[] { t }), RangeType.DOUBLE);
}
},
// todo add BYTE support
// todo add SHORT support
@ -777,18 +788,18 @@ public class RangeFieldMapper extends FieldMapper {
}
@Override
public Query withinQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return IntRange.newWithinQuery(field, new int[] {(Integer)from + (includeFrom ? 0 : 1)},
new int[] {(Integer)to - (includeTo ? 0 : 1)});
return createQuery(field, (Integer) from, (Integer) to, includeFrom, includeTo,
(f, t) -> IntRange.newWithinQuery(field, new int[] { f }, new int[] { t }), RangeType.INTEGER);
}
@Override
public Query containsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return IntRange.newContainsQuery(field, new int[] {(Integer)from + (includeFrom ? 0 : 1)},
new int[] {(Integer)to - (includeTo ? 0 : 1)});
return createQuery(field, (Integer) from, (Integer) to, includeFrom, includeTo,
(f, t) -> IntRange.newContainsQuery(field, new int[] { f }, new int[] { t }), RangeType.INTEGER);
}
@Override
public Query intersectsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return IntRange.newIntersectsQuery(field, new int[] {(Integer)from + (includeFrom ? 0 : 1)},
new int[] {(Integer)to - (includeTo ? 0 : 1)});
return createQuery(field, (Integer) from, (Integer) to, includeFrom, includeTo,
(f, t) -> IntRange.newIntersectsQuery(field, new int[] { f }, new int[] { t }), RangeType.INTEGER);
}
},
LONG("long_range", NumberType.LONG) {
@ -837,18 +848,18 @@ public class RangeFieldMapper extends FieldMapper {
}
@Override
public Query withinQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return LongRange.newWithinQuery(field, new long[] {(Long)from + (includeFrom ? 0 : 1)},
new long[] {(Long)to - (includeTo ? 0 : 1)});
return createQuery(field, (Long) from, (Long) to, includeFrom, includeTo,
(f, t) -> LongRange.newWithinQuery(field, new long[] { f }, new long[] { t }), RangeType.LONG);
}
@Override
public Query containsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return LongRange.newContainsQuery(field, new long[] {(Long)from + (includeFrom ? 0 : 1)},
new long[] {(Long)to - (includeTo ? 0 : 1)});
return createQuery(field, (Long) from, (Long) to, includeFrom, includeTo,
(f, t) -> LongRange.newContainsQuery(field, new long[] { f }, new long[] { t }), RangeType.LONG);
}
@Override
public Query intersectsQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo) {
return LongRange.newIntersectsQuery(field, new long[] {(Long)from + (includeFrom ? 0 : 1)},
new long[] {(Long)to - (includeTo ? 0 : 1)});
return createQuery(field, (Long) from, (Long) to, includeFrom, includeTo,
(f, t) -> LongRange.newIntersectsQuery(field, new long[] { f }, new long[] { t }), RangeType.LONG);
}
};
@ -867,6 +878,31 @@ public class RangeFieldMapper extends FieldMapper {
return name;
}
/**
* Internal helper to create the actual {@link Query} using the provided supplier function. Before creating the query we check if
* the intervals min &gt; max, in which case an {@link IllegalArgumentException} is raised. The method adapts the interval bounds
* based on whether the edges should be included or excluded. In case where after this correction the interval would be empty
* because min &gt; max, we simply return a {@link MatchNoDocsQuery}.
* This helper handles all {@link Number} cases and dates, the IP range type uses its own logic.
*/
private static <T extends Comparable<T>> Query createQuery(String field, T from, T to, boolean includeFrom, boolean includeTo,
BiFunction<T, T, Query> querySupplier, RangeType rangeType) {
if (from.compareTo(to) > 0) {
// wrong argument order, this is an error the user should fix
throw new IllegalArgumentException("Range query `from` value (" + from + ") is greater than `to` value (" + to + ")");
}
@SuppressWarnings("unchecked")
T correctedFrom = includeFrom ? from : (T) rangeType.nextUp(from);
@SuppressWarnings("unchecked")
T correctedTo = includeTo ? to : (T) rangeType.nextDown(to);
if (correctedFrom.compareTo(correctedTo) > 0) {
return new MatchNoDocsQuery("range didn't intersect anything");
} else {
return querySupplier.apply(correctedFrom, correctedTo);
}
}
public abstract Field getRangeField(String name, Range range);
public List<IndexableField> createFields(ParseContext context, String name, Range range, boolean indexed,
boolean docValued, boolean stored) {

127
server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java
View File

@ -28,6 +28,7 @@ import org.apache.lucene.document.LongRange;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.queries.BinaryDocValuesRangeQuery;
import org.apache.lucene.search.IndexOrDocValuesQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchParseException;
@ -49,6 +50,7 @@ import java.net.InetAddress;
import java.util.Locale;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
public class RangeFieldTypeTests extends FieldTypeTestCase {
RangeType type;
@ -92,11 +94,136 @@ public class RangeFieldTypeTests extends FieldTypeTestCase {
boolean includeUpper = randomBoolean();
Object from = nextFrom();
Object to = nextTo(from);
if (includeLower == false && includeUpper == false) {
// need to increase once more, otherwise interval is empty because edge values are exclusive
to = nextTo(to);
}
assertEquals(getExpectedRangeQuery(relation, from, to, includeLower, includeUpper),
ft.rangeQuery(from, to, includeLower, includeUpper, relation, null, null, context));
}
/**
* test the queries are correct if from/to are adjacent and the range is exclusive of those values
*/
public void testRangeQueryIntersectsAdjacentValues() throws Exception {
QueryShardContext context = createContext();
ShapeRelation relation = randomFrom(ShapeRelation.values());
RangeFieldType ft = new RangeFieldType(type);
ft.setName(FIELDNAME);
ft.setIndexOptions(IndexOptions.DOCS);
Object from = null;
Object to = null;
switch (type) {
case LONG: {
long fromValue = randomLong();
from = fromValue;
to = fromValue + 1;
break;
}
case DATE: {
long fromValue = randomInt();
from = new DateTime(fromValue);
to = new DateTime(fromValue + 1);
break;
}
case INTEGER: {
int fromValue = randomInt();
from = fromValue;
to = fromValue + 1;
break;
}
case DOUBLE: {
double fromValue = randomDoubleBetween(0, 100, true);
from = fromValue;
to = Math.nextUp(fromValue);
break;
}
case FLOAT: {
float fromValue = randomFloat();
from = fromValue;
to = Math.nextUp(fromValue);
break;
}
case IP: {
byte[] ipv4 = new byte[4];
random().nextBytes(ipv4);
InetAddress fromValue = InetAddress.getByAddress(ipv4);
from = fromValue;
to = InetAddressPoint.nextUp(fromValue);
break;
}
default:
from = nextFrom();
to = nextTo(from);
}
Query rangeQuery = ft.rangeQuery(from, to, false, false, relation, null, null, context);
assertThat(rangeQuery, instanceOf(IndexOrDocValuesQuery.class));
assertThat(((IndexOrDocValuesQuery) rangeQuery).getIndexQuery(), instanceOf(MatchNoDocsQuery.class));
}
/**
* check that we catch cases where the user specifies larger "from" than "to" value, not counting the include upper/lower settings
*/
public void testFromLargerToErrors() throws Exception {
QueryShardContext context = createContext();
RangeFieldType ft = new RangeFieldType(type);
ft.setName(FIELDNAME);
ft.setIndexOptions(IndexOptions.DOCS);
final Object from;
final Object to;
switch (type) {
case LONG: {
long fromValue = randomLong();
from = fromValue;
to = fromValue - 1L;
break;
}
case DATE: {
long fromValue = randomInt();
from = new DateTime(fromValue);
to = new DateTime(fromValue - 1);
break;
}
case INTEGER: {
int fromValue = randomInt();
from = fromValue;
to = fromValue - 1;
break;
}
case DOUBLE: {
double fromValue = randomDoubleBetween(0, 100, true);
from = fromValue;
to = fromValue - 1.0d;
break;
}
case FLOAT: {
float fromValue = randomFloat();
from = fromValue;
to = fromValue - 1.0f;
break;
}
case IP: {
byte[] ipv4 = new byte[4];
random().nextBytes(ipv4);
InetAddress fromValue = InetAddress.getByAddress(ipv4);
from = fromValue;
to = InetAddressPoint.nextDown(fromValue);
break;
}
default:
// quit test for other range types
return;
}
ShapeRelation relation = randomFrom(ShapeRelation.values());
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> ft.rangeQuery(from, to, true, true, relation, null, null, context));
assertTrue(ex.getMessage().contains("Range query `from` value"));
assertTrue(ex.getMessage().contains("is greater than `to` value"));
}
private QueryShardContext createContext() {
Settings indexSettings = Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build();