diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java index e5ba55de7bf..1ee2b6f059a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java @@ -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 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 > 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 > 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 > Query createQuery(String field, T from, T to, boolean includeFrom, boolean includeTo, + BiFunction 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 createFields(ParseContext context, String name, Range range, boolean indexed, boolean docValued, boolean stored) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java index 6ca98fb4db6..a26999fa3a6 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java @@ -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(); @@ -104,7 +231,7 @@ public class RangeFieldTypeTests extends FieldTypeTestCase { return new QueryShardContext(0, idxSettings, null, null, null, null, null, xContentRegistry(), writableRegistry(), null, null, () -> nowInMillis, null); } - + public void testDateRangeQueryUsingMappingFormat() { QueryShardContext context = createContext(); RangeFieldType fieldType = new RangeFieldType(RangeType.DATE);