Skip to content

Commit

Permalink
Fix some range query edge cases (#41160)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Christoph Büscher authored Apr 16, 2019
1 parent 01ab82f commit 778a1d0
Show file tree
Hide file tree
Showing 2 changed files with 209 additions and 46 deletions.
126 changes: 81 additions & 45 deletions server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand Down Expand Up @@ -63,13 +64,15 @@
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
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;
Expand Down Expand Up @@ -516,25 +519,38 @@ public Query dvRangeQuery(String field, QueryType queryType, Object from, Object
}

@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 (Arrays.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 (Arrays.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) {
Expand Down Expand Up @@ -662,21 +678,18 @@ public Field getRangeField(String name, Range r) {
}
@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) {
Expand Down Expand Up @@ -724,22 +737,20 @@ public Field getRangeField(String name, Range r) {
}
@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
Expand Down Expand Up @@ -777,18 +788,18 @@ public Field getRangeField(String name, Range r) {
}
@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) {
Expand Down Expand Up @@ -837,18 +848,18 @@ public Field getRangeField(String name, Range r) {
}
@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);
}
};

Expand All @@ -867,6 +878,31 @@ public final String typeName() {
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) {
Expand Down
Loading

0 comments on commit 778a1d0

Please sign in to comment.