You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ep...@apache.org on 2022/09/16 19:45:23 UTC
[solr] branch main updated: SOLR-16361 'mod' function query casts to float, returns wrong modulus for large ints (#989)
This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new b480bc81ac8 SOLR-16361 'mod' function query casts to float, returns wrong modulus for large ints (#989)
b480bc81ac8 is described below
commit b480bc81ac82094bc5051ace3308a34f85d7604d
Author: Dan Rosher <ro...@gmail.com>
AuthorDate: Fri Sep 16 20:45:17 2022 +0100
SOLR-16361 'mod' function query casts to float, returns wrong modulus for large ints (#989)
* use double instead of float
Co-authored-by: Dan Rosher <d....@cv-library.co.uk>
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/search/ValueSourceParser.java | 7 +-
.../solr/search/function/DualDoubleFunction.java | 103 +++++++++++++++++++++
.../apache/solr/search/TestModulusFunctions.java | 80 ++++++++++++++++
4 files changed, 189 insertions(+), 3 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 68ed82e8a37..2ed45819bcd 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -84,6 +84,8 @@ Improvements
* SOLR-16230: JWT nested roles support (Marco Descher, janhoy)
+* SOLR-16361: mod() is now accurate for all integers, floats, doubles and longs upto 2^52 (Dan Rosher via Eric Pugh)
+
Optimizations
---------------------
* SOLR-16120: Optimise hl.fl expansion. (Christine Poerschke, David Smiley, Mike Drob)
diff --git a/solr/core/src/java/org/apache/solr/search/ValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ValueSourceParser.java
index 90bfeed9fb2..89309db4ce8 100644
--- a/solr/core/src/java/org/apache/solr/search/ValueSourceParser.java
+++ b/solr/core/src/java/org/apache/solr/search/ValueSourceParser.java
@@ -102,6 +102,7 @@ import org.apache.solr.search.facet.UniqueBlockQueryAgg;
import org.apache.solr.search.facet.VarianceAgg;
import org.apache.solr.search.function.CollapseScoreFunction;
import org.apache.solr.search.function.ConcatStringFunction;
+import org.apache.solr.search.function.DualDoubleFunction;
import org.apache.solr.search.function.EqualFunction;
import org.apache.solr.search.function.OrdFieldSource;
import org.apache.solr.search.function.ReverseOrdFieldSource;
@@ -274,16 +275,16 @@ public abstract class ValueSourceParser implements NamedListInitializedPlugin {
public ValueSource parse(FunctionQParser fp) throws SyntaxError {
ValueSource a = fp.parseValueSource();
ValueSource b = fp.parseValueSource();
- return new DualFloatFunction(a, b) {
+ return new DualDoubleFunction(a, b) {
@Override
protected String name() {
return "mod";
}
@Override
- protected float func(int doc, FunctionValues aVals, FunctionValues bVals)
+ protected double func(int doc, FunctionValues aVals, FunctionValues bVals)
throws IOException {
- return aVals.floatVal(doc) % bVals.floatVal(doc);
+ return aVals.doubleVal(doc) % bVals.doubleVal(doc);
}
};
}
diff --git a/solr/core/src/java/org/apache/solr/search/function/DualDoubleFunction.java b/solr/core/src/java/org/apache/solr/search/function/DualDoubleFunction.java
new file mode 100644
index 00000000000..d92ea9820a1
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/search/function/DualDoubleFunction.java
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.function;
+
+import java.io.IOException;
+import java.util.Map;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.queries.function.FunctionValues;
+import org.apache.lucene.queries.function.ValueSource;
+import org.apache.lucene.queries.function.docvalues.DoubleDocValues;
+import org.apache.lucene.queries.function.valuesource.MultiFunction;
+import org.apache.lucene.search.IndexSearcher;
+
+/**
+ * Abstract {@link ValueSource} implementation which wraps two ValueSources and applies an
+ * extendible double function to their values.
+ */
+public abstract class DualDoubleFunction extends ValueSource {
+ protected final ValueSource a;
+ protected final ValueSource b;
+
+ /**
+ * @param a the base.
+ * @param b the exponent.
+ */
+ public DualDoubleFunction(ValueSource a, ValueSource b) {
+ this.a = a;
+ this.b = b;
+ }
+
+ protected abstract String name();
+
+ protected abstract double func(int doc, FunctionValues aVals, FunctionValues bVals)
+ throws IOException;
+
+ @Override
+ public String description() {
+ return name() + "(" + a.description() + "," + b.description() + ")";
+ }
+
+ @Override
+ public FunctionValues getValues(Map<Object, Object> context, LeafReaderContext readerContext)
+ throws IOException {
+ final FunctionValues aVals = a.getValues(context, readerContext);
+ final FunctionValues bVals = b.getValues(context, readerContext);
+ return new DoubleDocValues(this) {
+ @Override
+ public double doubleVal(int doc) throws IOException {
+ return func(doc, aVals, bVals);
+ }
+ /**
+ * True if and only if <em>all</em> of the wrapped {@link FunctionValues} <code>exists</code>
+ * for the specified doc
+ */
+ @Override
+ public boolean exists(int doc) throws IOException {
+ return MultiFunction.allExists(doc, aVals, bVals);
+ }
+
+ @Override
+ public String toString(int doc) throws IOException {
+ return name() + '(' + aVals.toString(doc) + ',' + bVals.toString(doc) + ')';
+ }
+ };
+ }
+
+ @Override
+ public void createWeight(Map<Object, Object> context, IndexSearcher searcher) throws IOException {
+ a.createWeight(context, searcher);
+ b.createWeight(context, searcher);
+ }
+
+ @Override
+ public int hashCode() {
+ int h = a.hashCode();
+ h ^= (h << 13) | (h >>> 20);
+ h += b.hashCode();
+ h ^= (h << 23) | (h >>> 10);
+ h += name().hashCode();
+ return h;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this.getClass() != o.getClass()) return false;
+ DualDoubleFunction other = (DualDoubleFunction) o;
+ return this.a.equals(other.a) && this.b.equals(other.b);
+ }
+}
diff --git a/solr/core/src/test/org/apache/solr/search/TestModulusFunctions.java b/solr/core/src/test/org/apache/solr/search/TestModulusFunctions.java
new file mode 100644
index 00000000000..2a46c674d71
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/search/TestModulusFunctions.java
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TestModulusFunctions extends SolrTestCaseJ4 {
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ initCore("solrconfig.xml", "schema15.xml");
+ }
+
+ @After
+ public void cleanup() {
+ clearIndex();
+ assertU(optimize());
+ }
+
+ @Test
+ public void testModulus() {
+ int l = 1 << 24;
+ int[] a = new int[] {l - 3, l - 2, l - 1, l, l + 1, l + 2};
+
+ // The existing implementation casts int to float, with 24bit mantissa
+ float[] f = new float[] {1, 2, 0, 1, 1, 0}; // these...
+
+ // The newer implementation casts int to double, with 52bit mantissa
+ double[] d = new double[] {1, 2, 0, 1, 2, 0}; // ...differ
+
+ for (int i = 0; i < a.length; i++) {
+ assertEquals("int -> float modulus", f[i], (float) a[i] % 3, 0.0);
+ assertEquals("int -> double modulus", d[i], (double) a[i] % 3, 0.0);
+ }
+ }
+
+ @Test
+ public void testMod() throws Exception {
+ int a = 1 << 24;
+ for (int i = 0, b = a - 3; b < a + 3; b++, i++) {
+ assertU(adoc("id", Integer.toString(i), "foo_i", Integer.toString(b)));
+ }
+
+ assertU(commit());
+ assertJQ(
+ req(
+ "defType", "lucene",
+ "q", "*:*",
+ "fl", "id,foo_i,m:mod(foo_i,3)"),
+ "/response/docs/[0]/foo_i==16777213",
+ "/response/docs/[0]/m==1.0",
+ "/response/docs/[1]/foo_i==16777214",
+ "/response/docs/[1]/m==2.0",
+ "/response/docs/[2]/foo_i==16777215",
+ "/response/docs/[2]/m==0.0",
+ "/response/docs/[3]/foo_i==16777216",
+ "/response/docs/[3]/m==1.0",
+ "/response/docs/[4]/foo_i==16777217",
+ "/response/docs/[4]/m==2.0",
+ "/response/docs/[5]/foo_i==16777218",
+ "/response/docs/[5]/m==0.0");
+ }
+}