Skip to content

Commit

Permalink
do not silently round down in {term,terms}Query
Browse files Browse the repository at this point in the history
  • Loading branch information
@scampi
scampi committed Dec 7, 2016
1 parent 21db4b4 commit 19cdc1b
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 67 deletions.
114 changes: 79 additions & 35 deletions core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.lucene.util.NumericUtils;
import org.elasticsearch.action.fieldstats.FieldStats;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -144,7 +145,7 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
if (propNode == null) {
throw new MapperParsingException("Property [null_value] cannot be null.");
}
builder.nullValue(type.parse(propNode));
builder.nullValue(type.parse(propNode, false));
iterator.remove();
} else if (propName.equals("ignore_malformed")) {
builder.ignoreMalformed(TypeParsers.nodeBooleanValue("ignore_malformed", propNode, parserContext));
Expand All @@ -161,8 +162,8 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
public enum NumberType {
HALF_FLOAT("half_float", NumericType.HALF_FLOAT) {
@Override
Float parse(Object value) {
return (Float) FLOAT.parse(value);
Float parse(Object value, boolean coerce) {
return (Float) FLOAT.parse(value, false);
}

@Override
Expand All @@ -172,15 +173,15 @@ Float parse(XContentParser parser, boolean coerce) throws IOException {

@Override
Query termQuery(String field, Object value) {
float v = parse(value);
float v = parse(value, false);
return HalfFloatPoint.newExactQuery(field, v);
}

@Override
Query termsQuery(String field, List<Object> values) {
float[] v = new float[values.size()];
for (int i = 0; i < values.size(); ++i) {
v[i] = parse(values.get(i));
v[i] = (float) parse(values.get(i), false);
}
return HalfFloatPoint.newSetQuery(field, v);
}
Expand All @@ -191,14 +192,14 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
float l = Float.NEGATIVE_INFINITY;
float u = Float.POSITIVE_INFINITY;
if (lowerTerm != null) {
l = parse(lowerTerm);
l = parse(lowerTerm, false);
if (includeLower) {
l = Math.nextDown(l);
}
l = HalfFloatPoint.nextUp(l);
}
if (upperTerm != null) {
u = parse(upperTerm);
u = parse(upperTerm, false);
if (includeUpper) {
u = Math.nextUp(u);
}
Expand Down Expand Up @@ -241,7 +242,7 @@ FieldStats.Double stats(IndexReader reader, String fieldName,
},
FLOAT("float", NumericType.FLOAT) {
@Override
Float parse(Object value) {
Float parse(Object value, boolean coerce) {
if (value instanceof Number) {
return ((Number) value).floatValue();
}
Expand All @@ -258,15 +259,15 @@ Float parse(XContentParser parser, boolean coerce) throws IOException {

@Override
Query termQuery(String field, Object value) {
float v = parse(value);
float v = parse(value, false);
return FloatPoint.newExactQuery(field, v);
}

@Override
Query termsQuery(String field, List<Object> values) {
float[] v = new float[values.size()];
for (int i = 0; i < values.size(); ++i) {
v[i] = parse(values.get(i));
v[i] = parse(values.get(i), false);
}
return FloatPoint.newSetQuery(field, v);
}
Expand All @@ -277,13 +278,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
float l = Float.NEGATIVE_INFINITY;
float u = Float.POSITIVE_INFINITY;
if (lowerTerm != null) {
l = parse(lowerTerm);
l = parse(lowerTerm, false);
if (includeLower == false) {
l = Math.nextUp(l);
}
}
if (upperTerm != null) {
u = parse(upperTerm);
u = parse(upperTerm, false);
if (includeUpper == false) {
u = Math.nextDown(u);
}
Expand Down Expand Up @@ -325,7 +326,7 @@ FieldStats.Double stats(IndexReader reader, String fieldName,
},
DOUBLE("double", NumericType.DOUBLE) {
@Override
Double parse(Object value) {
Double parse(Object value, boolean coerce) {
if (value instanceof Number) {
return ((Number) value).doubleValue();
}
Expand All @@ -342,15 +343,15 @@ Double parse(XContentParser parser, boolean coerce) throws IOException {

@Override
Query termQuery(String field, Object value) {
double v = parse(value);
double v = parse(value, false);
return DoublePoint.newExactQuery(field, v);
}

@Override
Query termsQuery(String field, List<Object> values) {
double[] v = new double[values.size()];
for (int i = 0; i < values.size(); ++i) {
v[i] = parse(values.get(i));
v[i] = parse(values.get(i), false);
}
return DoublePoint.newSetQuery(field, v);
}
Expand All @@ -361,13 +362,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
double l = Double.NEGATIVE_INFINITY;
double u = Double.POSITIVE_INFINITY;
if (lowerTerm != null) {
l = parse(lowerTerm);
l = parse(lowerTerm, false);
if (includeLower == false) {
l = Math.nextUp(l);
}
}
if (upperTerm != null) {
u = parse(upperTerm);
u = parse(upperTerm, false);
if (includeUpper == false) {
u = Math.nextDown(u);
}
Expand Down Expand Up @@ -409,12 +410,15 @@ FieldStats.Double stats(IndexReader reader, String fieldName,
},
BYTE("byte", NumericType.BYTE) {
@Override
Byte parse(Object value) {
Byte parse(Object value, boolean coerce) {
if (value instanceof Number) {
double doubleValue = ((Number) value).doubleValue();
if (doubleValue < Byte.MIN_VALUE || doubleValue > Byte.MAX_VALUE) {
throw new IllegalArgumentException("Value [" + value + "] is out of range for a byte");
}
if (!coerce && doubleValue % 1 != 0) {
throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
}
return ((Number) value).byteValue();
}
if (value instanceof BytesRef) {
Expand Down Expand Up @@ -467,12 +471,15 @@ Number valueForSearch(Number value) {
},
SHORT("short", NumericType.SHORT) {
@Override
Short parse(Object value) {
Short parse(Object value, boolean coerce) {
if (value instanceof Number) {
double doubleValue = ((Number) value).doubleValue();
if (doubleValue < Short.MIN_VALUE || doubleValue > Short.MAX_VALUE) {
throw new IllegalArgumentException("Value [" + value + "] is out of range for a short");
}
if (!coerce && doubleValue % 1 != 0) {
throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
}
return ((Number) value).shortValue();
}
if (value instanceof BytesRef) {
Expand Down Expand Up @@ -525,12 +532,15 @@ Number valueForSearch(Number value) {
},
INTEGER("integer", NumericType.INT) {
@Override
Integer parse(Object value) {
Integer parse(Object value, boolean coerce) {
if (value instanceof Number) {
double doubleValue = ((Number) value).doubleValue();
if (doubleValue < Integer.MIN_VALUE || doubleValue > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Value [" + value + "] is out of range for an integer");
}
if (!coerce && doubleValue % 1 != 0) {
throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
}
return ((Number) value).intValue();
}
if (value instanceof BytesRef) {
Expand All @@ -546,15 +556,29 @@ Integer parse(XContentParser parser, boolean coerce) throws IOException {

@Override
Query termQuery(String field, Object value) {
int v = parse(value);
if (hasDecimalPart(value)) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] has a decimal part");
}
int v = parse(value, true);
return IntPoint.newExactQuery(field, v);
}

@Override
Query termsQuery(String field, List<Object> values) {
int[] v = new int[values.size()];
for (int i = 0; i < values.size(); ++i) {
v[i] = parse(values.get(i));
final List<Object> nonDecimalValues = new ArrayList<>();

values.forEach(value -> {
if (!hasDecimalPart(value)) {
nonDecimalValues.add(value);
}
});
if (nonDecimalValues.isEmpty()) {
return Queries.newMatchNoDocsQuery("All values have a decimal part");
}

int[] v = new int[nonDecimalValues.size()];
for (int i = 0; i < nonDecimalValues.size(); ++i) {
v[i] = parse(nonDecimalValues.get(i), true);
}
return IntPoint.newSetQuery(field, v);
}
Expand All @@ -565,7 +589,7 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
int l = Integer.MIN_VALUE;
int u = Integer.MAX_VALUE;
if (lowerTerm != null) {
l = parse(lowerTerm);
l = parse(lowerTerm, true);
if (includeLower == false || hasDecimalPart(lowerTerm)) {
if (l == Integer.MAX_VALUE) {
return new MatchNoDocsQuery();
Expand All @@ -574,7 +598,7 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
}
}
if (upperTerm != null) {
u = parse(upperTerm);
u = parse(upperTerm, true);
if (includeUpper == false && !hasDecimalPart(upperTerm)) {
if (u == Integer.MIN_VALUE) {
return new MatchNoDocsQuery();
Expand Down Expand Up @@ -618,12 +642,15 @@ FieldStats.Long stats(IndexReader reader, String fieldName,
},
LONG("long", NumericType.LONG) {
@Override
Long parse(Object value) {
Long parse(Object value, boolean coerce) {
if (value instanceof Number) {
double doubleValue = ((Number) value).doubleValue();
if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) {
throw new IllegalArgumentException("Value [" + value + "] is out of range for a long");
}
if (!coerce && doubleValue % 1 != 0) {
throw new IllegalArgumentException("Value [" + value + "] has a decimal part");
}
return ((Number) value).longValue();
}
if (value instanceof BytesRef) {
Expand All @@ -639,15 +666,29 @@ Long parse(XContentParser parser, boolean coerce) throws IOException {

@Override
Query termQuery(String field, Object value) {
long v = parse(value);
if (hasDecimalPart(value)) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] has a decimal part");
}
long v = parse(value, true);
return LongPoint.newExactQuery(field, v);
}

@Override
Query termsQuery(String field, List<Object> values) {
long[] v = new long[values.size()];
for (int i = 0; i < values.size(); ++i) {
v[i] = parse(values.get(i));
final List<Object> nonDecimalValues = new ArrayList<>();

values.forEach(value -> {
if (!hasDecimalPart(value)) {
nonDecimalValues.add(value);
}
});
if (nonDecimalValues.isEmpty()) {
return Queries.newMatchNoDocsQuery("All values have a decimal part");
}

long[] v = new long[nonDecimalValues.size()];
for (int i = 0; i < nonDecimalValues.size(); ++i) {
v[i] = parse(nonDecimalValues.get(i), true);
}
return LongPoint.newSetQuery(field, v);
}
Expand All @@ -658,7 +699,7 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
long l = Long.MIN_VALUE;
long u = Long.MAX_VALUE;
if (lowerTerm != null) {
l = parse(lowerTerm);
l = parse(lowerTerm, true);
if (includeLower == false || hasDecimalPart(lowerTerm)) {
if (l == Long.MAX_VALUE) {
return new MatchNoDocsQuery();
Expand All @@ -667,7 +708,7 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
}
}
if (upperTerm != null) {
u = parse(upperTerm);
u = parse(upperTerm, true);
if (includeUpper == false && !hasDecimalPart(upperTerm)) {
if (u == Long.MIN_VALUE) {
return new MatchNoDocsQuery();
Expand Down Expand Up @@ -731,7 +772,7 @@ final NumericType numericType() {
abstract Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
boolean includeLower, boolean includeUpper);
abstract Number parse(XContentParser parser, boolean coerce) throws IOException;
abstract Number parse(Object value);
abstract Number parse(Object value, boolean coerce);
public abstract List<Field> createFields(String name, Number value, boolean indexed,
boolean docValued, boolean stored);
abstract FieldStats<? extends Number> stats(IndexReader reader, String fieldName,
Expand All @@ -748,6 +789,9 @@ protected boolean hasDecimalPart(Object number) {
double doubleValue = ((Number) number).doubleValue();
return doubleValue % 1 != 0;
}
if (number instanceof String) {
return Double.parseDouble((String) number) % 1 != 0;
}
return false;
}

Expand Down Expand Up @@ -921,7 +965,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
}

if (numericValue == null) {
numericValue = fieldType().type.parse(value);
numericValue = fieldType().type.parse(value, coerce.value());
}

if (includeInAll) {
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,8 +728,8 @@ public Number parseTo(RangeFieldType fieldType, XContentParser parser, boolean c
public Query rangeQuery(String field, Object from, Object to, boolean includeFrom, boolean includeTo,
ShapeRelation relation, @Nullable DateTimeZone timeZone, @Nullable DateMathParser dateMathParser,
QueryShardContext context) {
Number lower = from == null ? minValue() : numberType.parse(from);
Number upper = to == null ? maxValue() : numberType.parse(to);
Number lower = from == null ? minValue() : numberType.parse(from, false);
Number upper = to == null ? maxValue() : numberType.parse(to, false);
if (relation == ShapeRelation.WITHIN) {
return withinQuery(field, lower, upper, includeFrom, includeTo);
} else if (relation == ShapeRelation.CONTAINS) {
Expand Down
Loading

0 comments on commit 19cdc1b

Please sign in to comment.