From a938bd57a946f7eb8d687af19394cacc603d141f Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Tue, 6 Aug 2013 16:21:59 +0200 Subject: [PATCH] add assertion for cast double->float MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScoreFunction scoring might result in under or overflow, for example if a user decides to use the timestamp as a boost in the script scorer. Therefore, check if cast causes a huge precision loss. Note that this does not always detect casting issues. For example in ScriptFunction.score() the function SearchScript.runAsDouble() is called. AbstractFloatSearchScript implements it as follows: @Override
 public double runAsDouble() {
 return runAsFloat();
 } In this case the cast happens before the assertion and therfore precision lossor over/underflows cannot be detected by the assertion. --- .../search/function/BoostScoreFunction.java | 12 ++++++---- .../function/FiltersFunctionScoreQuery.java | 23 +++++++++++-------- .../search/function/FunctionScoreQuery.java | 12 +++++++--- .../lucene/search/function/ScoreFunction.java | 2 +- .../search/function/ScriptScoreFunction.java | 4 ++-- .../functionscore/DecayFunctionParser.java | 4 ++-- 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java index 209e7d44849..c62605ec1f5 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java @@ -33,7 +33,6 @@ public class BoostScoreFunction implements ScoreFunction { this.boost = boost; } - public float getBoost() { return boost; } @@ -44,7 +43,7 @@ public class BoostScoreFunction implements ScoreFunction { } @Override - public float score(int docId, float subQueryScore) { + public double score(int docId, float subQueryScore) { return subQueryScore * boost; } @@ -68,12 +67,15 @@ public class BoostScoreFunction implements ScoreFunction { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; BoostScoreFunction that = (BoostScoreFunction) o; - if (Float.compare(that.boost, boost) != 0) return false; + if (Float.compare(that.boost, boost) != 0) + return false; return true; } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java index 1fdcb3823c0..a89f6a27b5b 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java @@ -164,11 +164,11 @@ public class FiltersFunctionScoreQuery extends Query { if (docSet.get(doc)) { filterFunction.function.setNextReader(context); Explanation functionExplanation = filterFunction.function.explainFactor(doc); - float factor = functionExplanation.getValue(); + double factor = functionExplanation.getValue(); if (factor > maxBoost) { factor = maxBoost; } - float sc = getBoost() * factor; + float sc = toFloat(getBoost() * factor); Explanation filterExplanation = new ComplexExplanation(true, sc, "custom score, product of:"); filterExplanation.addDetail(new Explanation(1.0f, "match filter: " + filterFunction.filter.toString())); filterExplanation.addDetail(functionExplanation); @@ -186,21 +186,21 @@ public class FiltersFunctionScoreQuery extends Query { int count = 0; float total = 0; float multiply = 1; - float max = Float.NEGATIVE_INFINITY; - float min = Float.POSITIVE_INFINITY; + double max = Double.NEGATIVE_INFINITY; + double min = Double.POSITIVE_INFINITY; ArrayList filtersExplanations = new ArrayList(); for (FilterFunction filterFunction : filterFunctions) { Bits docSet = DocIdSets.toSafeBits(context.reader(), filterFunction.filter.getDocIdSet(context, context.reader().getLiveDocs())); if (docSet.get(doc)) { filterFunction.function.setNextReader(context); Explanation functionExplanation = filterFunction.function.explainFactor(doc); - float factor = functionExplanation.getValue(); + double factor = functionExplanation.getValue(); count++; total += factor; multiply *= factor; max = Math.max(factor, max); min = Math.min(factor, min); - Explanation res = new ComplexExplanation(true, factor, "custom score, product of:"); + Explanation res = new ComplexExplanation(true, toFloat(factor), "custom score, product of:"); res.addDetail(new Explanation(1.0f, "match filter: " + filterFunction.filter.toString())); res.addDetail(functionExplanation); res.addDetail(new Explanation(getBoost(), "queryBoost")); @@ -208,7 +208,7 @@ public class FiltersFunctionScoreQuery extends Query { } } if (count > 0) { - float factor = 0; + double factor = 0; switch (scoreMode) { case Avg: factor = total / count; @@ -230,7 +230,7 @@ public class FiltersFunctionScoreQuery extends Query { if (factor > maxBoost) { factor = maxBoost; } - float sc = factor * subQueryExpl.getValue() * getBoost(); + float sc = toFloat(factor * subQueryExpl.getValue() * getBoost()); Explanation res = new ComplexExplanation(true, sc, "custom score, score mode [" + scoreMode.toString().toLowerCase(Locale.ROOT) + "]"); res.addDetail(subQueryExpl); for (Explanation explanation : filtersExplanations) { @@ -341,7 +341,7 @@ public class FiltersFunctionScoreQuery extends Query { factor = maxBoost; } float score = scorer.score(); - return (float)(subQueryBoost * score * factor); + return toFloat(subQueryBoost * score * factor); } @Override @@ -381,5 +381,10 @@ public class FiltersFunctionScoreQuery extends Query { public int hashCode() { return subQuery.hashCode() + 31 * Arrays.hashCode(filterFunctions) ^ Float.floatToIntBits(getBoost()); } + + public static float toFloat(double input) { + assert Double.compare(((float) input), input) == 0 || (Math.abs(((float) input) - input) <= 0.001); + return (float) input; + } } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java index 3069e9198d8..ec4f356f333 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java @@ -62,7 +62,7 @@ public class FunctionScoreQuery extends Query { @Override public Query rewrite(IndexReader reader) throws IOException { Query newQ = subQuery.rewrite(reader); - if (newQ == subQuery){ + if (newQ == subQuery) { return this; } FunctionScoreQuery bq = (FunctionScoreQuery) this.clone(); @@ -165,8 +165,8 @@ public class FunctionScoreQuery extends Query { @Override public float score() throws IOException { - float factor = (float)function.score(scorer.docID(), scorer.score()); - return subQueryBoost * Math.min(maxBoost, factor); + double factor = function.score(scorer.docID(), scorer.score()); + return toFloat(subQueryBoost * Math.min(maxBoost, factor)); } @Override @@ -198,4 +198,10 @@ public class FunctionScoreQuery extends Query { public int hashCode() { return subQuery.hashCode() + 31 * function.hashCode() ^ Float.floatToIntBits(getBoost()); } + + public static float toFloat(double input) { + assert Double.compare(((float) input), input) == 0 || (Math.abs(((float) input) - input) <= 0.001); + return (float) input; + } + } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java index 1555135c94b..b420213a14d 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java @@ -29,7 +29,7 @@ public interface ScoreFunction { void setNextReader(AtomicReaderContext context); - float score(int docId, float subQueryScore); + double score(int docId, float subQueryScore); double factor(int docId); diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java index 46c652a7074..7fd53205820 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java @@ -46,10 +46,10 @@ public class ScriptScoreFunction implements ScoreFunction { } @Override - public float score(int docId, float subQueryScore) { + public double score(int docId, float subQueryScore) { script.setNextDocId(docId); script.setNextScore(subQueryScore); - return script.runAsFloat(); + return script.runAsDouble(); } @Override diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java index fe385e1328a..53c21476eb5 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java @@ -359,8 +359,8 @@ public abstract class DecayFunctionParser implements ScoreFunctionParser { } @Override - public float score(int docId, float subQueryScore) { - return (float) (subQueryScore * factor(docId)); + public double score(int docId, float subQueryScore) { + return (subQueryScore * factor(docId)); } @Override