This is an automated email from the ASF dual-hosted git repository.
andy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/jena.git
The following commit(s) were added to refs/heads/main by this push:
new 96d3a4b804 GH-1934: Fix NaN and sameValue
new cf5a50a47e Merge pull request #1935 from afs/nan
96d3a4b804 is described below
commit 96d3a4b804f8bb10ec6a5b235b18937abf48962d
Author: Andy Seaborne <[email protected]>
AuthorDate: Thu Jun 29 14:11:26 2023 +0100
GH-1934: Fix NaN and sameValue
---
.../java/org/apache/jena/sparql/expr/ExprNode.java | 116 +++++++++++----------
.../org/apache/jena/sparql/expr/NodeValue.java | 26 ++---
.../org/apache/jena/sparql/expr/NodeValueCmp.java | 24 +++--
.../org/apache/jena/sparql/expr/LibTestExpr.java | 10 ++
.../jena/sparql/expr/TestExpressionsMath.java | 14 +--
.../jena/sparql/expr/TestLeviathanFunctions.java | 5 +-
.../org/apache/jena/sparql/expr/TestNodeValue.java | 45 ++++++++
.../function/library/TestFnFunctionsString.java | 2 +-
8 files changed, 154 insertions(+), 88 deletions(-)
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprNode.java
b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprNode.java
index bf70c052d5..26e89aca24 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprNode.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/ExprNode.java
@@ -18,103 +18,107 @@
package org.apache.jena.sparql.expr;
-import java.util.Set ;
+import java.util.Set;
-import org.apache.jena.sparql.algebra.Op ;
-import org.apache.jena.sparql.core.Var ;
-import org.apache.jena.sparql.engine.binding.Binding ;
-import org.apache.jena.sparql.expr.nodevalue.XSDFuncOp ;
-import org.apache.jena.sparql.function.FunctionEnv ;
+import org.apache.jena.sparql.algebra.Op;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.expr.nodevalue.XSDFuncOp;
+import org.apache.jena.sparql.function.FunctionEnv;
import org.apache.jena.sparql.graph.NodeTransform;
-import org.apache.jena.sparql.sse.writers.WriterExpr ;
+import org.apache.jena.sparql.sse.writers.WriterExpr;
-/**
+/**
* A node that is a constraint expression that can be evaluated
* An {@link Expr} is already a Constraint - ExprNode is the base
implementation
* of all {@link Expr} classes that provides the Constraint machinery.
*/
-
+
public abstract class ExprNode implements Expr
{
@Override
public boolean isSatisfied(Binding binding, FunctionEnv funcEnv) {
try {
- NodeValue v = eval(binding, funcEnv) ;
- boolean b = XSDFuncOp.booleanEffectiveValue(v) ;
- return b ;
+ NodeValue v = eval(binding, funcEnv);
+ boolean b = XSDFuncOp.booleanEffectiveValue(v);
+ return b;
}
- catch (ExprEvalException ex) {
- return false ;
+ catch (ExprEvalException ex) {
+ return false;
}
}
- public boolean isExpr() { return true ; }
- public final Expr getExpr() { return this ; }
-
+ public boolean isExpr() { return true; }
+ public final Expr getExpr() { return this; }
+
// --- interface Constraint
-
+
@Override
- public abstract NodeValue eval(Binding binding, FunctionEnv env) ;
-
+ public abstract NodeValue eval(Binding binding, FunctionEnv env);
+
@Override
- public final Set<Var> getVarsMentioned() { return
ExprVars.getVarsMentioned(this) ; }
+ public final Set<Var> getVarsMentioned() { return
ExprVars.getVarsMentioned(this); }
@Override
- public abstract int hashCode() ;
+ public abstract int hashCode();
+
@Override
public final boolean equals(Object other) {
- if ( other == null ) return false ;
- if ( this == other ) return true ;
- if ( ! ( other instanceof Expr ) ) return false ;
- return equals((Expr)other, false) ;
+ if ( other == null )
+ return false;
+ if ( this == other )
+ return true;
+ if ( !(other instanceof Expr) )
+ return false;
+ return equals((Expr)other, false);
}
-
+
@Override
public final boolean equalsBySyntax(Expr other) {
- if ( other == null ) return false ;
- if ( this == other ) return true ;
- return equals(other, true) ;
+ if ( other == null ) return false;
+ if ( this == other ) return true;
+ return equals(other, true);
}
-
+
@Override
- public abstract boolean equals(Expr other, boolean bySyntax) ;
-
- protected static NodeValue eval(Binding binding, FunctionEnv funcEnv, Expr
expr) {
- if ( expr == null ) return null ;
- return expr.eval(binding, funcEnv) ;
+ public abstract boolean equals(Expr other, boolean bySyntax);
+
+ protected static NodeValue eval(Binding binding, FunctionEnv funcEnv, Expr
expr) {
+ if ( expr == null ) return null;
+ return expr.eval(binding, funcEnv);
}
-
+
@Override
- final public Expr deepCopy() { return copySubstitute(null) ; }
-
+ final public Expr deepCopy() { return copySubstitute(null); }
+
@Override
- public abstract Expr copySubstitute(Binding binding) ;
-
+ public abstract Expr copySubstitute(Binding binding);
+
@Override
- public abstract Expr applyNodeTransform(NodeTransform transform) ;
-
+ public abstract Expr applyNodeTransform(NodeTransform transform);
+
// ---- Default implementations
@Override
- public boolean isVariable() { return false ; }
+ public boolean isVariable() { return false; }
@Override
- public String getVarName() { return null ; } //throw new
ExprException("Expr.getVarName called on non-variable") ; }
+ public String getVarName() { return null; } //throw new
ExprException("Expr.getVarName called on non-variable"); }
@Override
- public ExprVar getExprVar() { return null ; } //throw new
ExprException("Expr.getVar called on non-variable") ; }
+ public ExprVar getExprVar() { return null; } //throw new
ExprException("Expr.getVar called on non-variable"); }
@Override
- public Var asVar() { return null ; } //throw new
ExprException("Expr.getVar called on non-variable") ; }
-
+ public Var asVar() { return null; } //throw new
ExprException("Expr.getVar called on non-variable"); }
+
@Override
- public boolean isConstant() { return false ; }
+ public boolean isConstant() { return false; }
@Override
- public NodeValue getConstant() { return null ; } // throw new
ExprException("Expr.getConstant called on non-constant") ; }
-
+ public NodeValue getConstant() { return null; } // throw new
ExprException("Expr.getConstant called on non-constant"); }
+
@Override
- public boolean isFunction() { return false ; }
+ public boolean isFunction() { return false; }
@Override
- public ExprFunction getFunction() { return null ; }
+ public ExprFunction getFunction() { return null; }
- public boolean isGraphPattern() { return false ; }
- public Op getGraphPattern() { return null ; }
+ public boolean isGraphPattern() { return false; }
+ public Op getGraphPattern() { return null; }
@Override
- public String toString() { return WriterExpr.asString(this) ; }
+ public String toString() { return WriterExpr.asString(this); }
}
diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValue.java
b/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValue.java
index d323019e6c..826f96f8da 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValue.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValue.java
@@ -487,7 +487,7 @@ public abstract class NodeValue extends ExprNode
* the two NodeValues are known to be the same, else throw
ExprEvalException
*/
public static boolean notSameValueAs(NodeValue nv1, NodeValue nv2) {
- return !sameValueAs(nv1, nv2);
+ return NodeValueCmp.notSameValueAs(nv1, nv2);
}
/** @deprecated Use {@link #sameValueAs(NodeValue, NodeValue)}. */
@@ -844,28 +844,25 @@ public abstract class NodeValue extends ExprNode
public final String asQuotedString()
{ return asQuotedString(new SerializationContext()) ; }
- public final String asQuotedString(SerializationContext context)
- {
+ public final String asQuotedString(SerializationContext context) {
// If possible, make a node and use that as the formatted output.
if ( node == null )
- node = asNode() ;
+ node = asNode();
if ( node != null )
- return FmtUtils.stringForNode(node, context) ;
- return toString() ;
+ return FmtUtils.stringForNode(node, context);
+ return toString();
}
- // Convert to a string - usually overridden.
- public String asString()
- {
+ // Convert to a string - usually overridden.
+ public String asString() {
// Do not call .toString()
- forceToNode() ;
- return NodeFunctions.str(node) ;
+ forceToNode();
+ return NodeFunctions.str(node);
}
@Override
- public int hashCode()
- {
- return asNode().hashCode() ;
+ public int hashCode() {
+ return asNode().hashCode();
}
@Override
@@ -873,7 +870,6 @@ public abstract class NodeValue extends ExprNode
if ( other == null ) return false ;
if ( this == other ) return true ;
// This is the equality condition Jena uses - lang tags are different
by case.
-
if ( ! ( other instanceof NodeValue ) )
return false ;
NodeValue nv = (NodeValue)other ;
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValueCmp.java
b/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValueCmp.java
index 9cf3993dc1..cfa40d2036 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValueCmp.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/NodeValueCmp.java
@@ -55,12 +55,10 @@ public class NodeValueCmp {
ValueSpace compType = NodeValue.classifyValueOp(nv1, nv2);
- if ( nv1 == nv2 )
- return true;
- if ( nv1.hasNode() && nv2.hasNode() ) {
- // Fast path - same RDF term => sameValue
- if ( nv1.getNode().equals(nv2.getNode()) )
- return true;
+ if ( nv1.equals(nv2) ) {
+ // Fast path - same RDF term => sameValue (except NaN).
+ if ( nv1.getNode() != null && nv2.getNode() != null &&
nv1.getNode().equals(nv2.asNode()) )
+ return sameExceptNaN(nv1, nv2);
}
// Special case - date/dateTime comparison is affected by timezones
and may
@@ -152,6 +150,15 @@ public class NodeValueCmp {
throw new ARQInternalErrorException("sameValueAs failure " + nv1 + "
and " + nv2);
}
+ // When nv1 and nv1 are known to be the sameTerm, including java ==
+ private static boolean sameExceptNaN(NodeValue nv1, NodeValue nv2) {
+ if ( nv1.isDouble() && Double.isNaN(nv1.getDouble()) )
+ return false;
+ if ( nv1.isFloat() && Float.isNaN(nv1.getFloat()) )
+ return false;
+ return true;
+ }
+
/** Worker for sameAs. */
private static boolean nSameValueAs(Node n1, Node n2) {
NodeValue nv1 = NodeValue.makeNode(n1);
@@ -171,8 +178,9 @@ public class NodeValueCmp {
* Return true if the two NodeValues are known to be different, return
false if
* the two NodeValues are known to be the same, else throw
ExprEvalException
*/
- /*package*/private static boolean notSameValueAs(NodeValue nv1, NodeValue
nv2) {
- return !sameValueAs(nv1, nv2);
+ /*package*/ static boolean notSameValueAs(NodeValue nv1, NodeValue nv2) {
+ // Works for NaN as well.
+ return !sameValueAs(nv1, nv2);
}
// ==== Compare
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/LibTestExpr.java
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/LibTestExpr.java
index 7b01f8a38b..4a57eb62b0 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/expr/LibTestExpr.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/expr/LibTestExpr.java
@@ -92,6 +92,16 @@ public class LibTestExpr {
assertTrue("Expected = " + expected + " : Actual = " + actual,
sameValueSameDatatype(expected, actual));
}
+ public static void testIsNaN(String exprStr) {
+ testSameObject(exprStr, NodeValue.nvNaN);
+ }
+
+ public static void testSameObject(String exprStr, NodeValue expected) {
+ Expr expr = parse(exprStr);
+ NodeValue actual = expr.eval(null, LibTestExpr.createTest());
+ assertTrue("Expected = " + expected + " : Actual = " + actual,
expected.equals(actual));
+ }
+
private static boolean sameValueSameDatatype(NodeValue nv1, NodeValue nv2)
{
if ( ! NodeValue.sameValueAs(nv1, nv2) )
return false;
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestExpressionsMath.java
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestExpressionsMath.java
index c323bb4993..32cb43375c 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestExpressionsMath.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestExpressionsMath.java
@@ -36,7 +36,7 @@ public class TestExpressionsMath
@Test public void exp_04() { test("math:exp(1e0/0)",
"'INF'^^xsd:double") ; }
@Test public void exp_05() { test("math:exp('INF'^^xsd:double)",
"'INF'^^xsd:double") ; }
@Test public void exp_06() { test("math:exp('-INF'^^xsd:double)",
"'0.0e0'^^xsd:double") ; }
- @Test public void exp_07() { test("math:exp('NaN'^^xsd:double)",
"'NaN'^^xsd:double") ; }
+ @Test public void exp_07() {
testIsNaN("math:exp('NaN'^^xsd:double)") ; }
@Test public void exp10_01() { test("math:exp10(2)", "100") ; }
@Test public void exp10_02() { testDouble("math:exp10(-1)", 0.1,
0.00001) ; }
@@ -50,7 +50,7 @@ public class TestExpressionsMath
@Test public void log_04() { test("math:log(0)",
"'-INF'^^xsd:double") ; }
@Test public void log_05() { test("math:exp('INF'^^xsd:double)",
"'INF'^^xsd:double") ; }
@Test public void log_06() { test("math:exp('-INF'^^xsd:double)",
"'0.0e0'^^xsd:double") ; }
- @Test public void log_07() { test("math:exp('NaN'^^xsd:double)",
"'NaN'^^xsd:double") ; }
+ @Test public void log_07() {
testIsNaN("math:exp('NaN'^^xsd:double)") ; }
@Test public void pow_01() { test("math:pow(2,2)", "4") ; }
@Test public void pow_02() { testDouble("math:pow(2,-2)", 0.25,
0.00001) ; }
@@ -62,19 +62,19 @@ public class TestExpressionsMath
@Test public void pow_13() {
test("math:pow('INF'^^xsd:double,0)", "'1.0e0'^^xsd:double") ; }
@Test public void pow_14() { test("math:pow('-INF'^^xsd:double,
0)", "'1.0e0'^^xsd:double") ; }
- @Test public void pow_15() { test("math:pow('NaN'^^xsd:double,
1)", "'NaN'^^xsd:double") ; }
- @Test public void pow_16() { test("math:pow(1,
'NaN'^^xsd:double)", "'NaN'^^xsd:double") ; }
+ @Test public void pow_15() {
testIsNaN("math:pow('NaN'^^xsd:double, 1)") ; }
+ @Test public void pow_16() { testIsNaN("math:pow(1,
'NaN'^^xsd:double)") ; }
@Test public void sqrt_01() { test("math:sqrt(1)",
"'1.0e0'^^xsd:double") ; }
@Test public void sqrt_02() { testDouble("math:sqrt(2)",
Math.sqrt(2), 0.000001) ; }
- @Test public void sqrt_03() { test("math:sqrt(-2)",
"'NaN'^^xsd:double") ; }
+ @Test public void sqrt_03() { testIsNaN("math:sqrt(-2)"); }
@Test(expected=ARQException.class)
public void sqrt_04() { test("math:sqrt('TWO')", "'dummy'")
; }
@Test public void sqrt_10() { test("math:sqrt('INF'^^xsd:double)",
"'INF'^^xsd:double") ; }
- @Test public void sqrt_11() {
test("math:sqrt('-INF'^^xsd:double)", "'NaN'^^xsd:double") ; }
- @Test public void sqrt_12() { test("math:sqrt('NaN'^^xsd:double)",
"'NaN'^^xsd:double") ; }
+ @Test public void sqrt_11() {
testIsNaN("math:sqrt('-INF'^^xsd:double)") ; }
+ @Test public void sqrt_12() {
testIsNaN("math:sqrt('NaN'^^xsd:double)") ; }
// 4.8.7 math:sqrt
// 4.8.8 math:sin
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestLeviathanFunctions.java
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestLeviathanFunctions.java
index 6bbdda0556..0e3b6ed447 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestLeviathanFunctions.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestLeviathanFunctions.java
@@ -21,6 +21,7 @@ package org.apache.jena.sparql.expr;
import static org.apache.jena.sparql.expr.LibTestExpr.test ;
import static org.apache.jena.sparql.expr.LibTestExpr.testDouble ;
import static org.apache.jena.sparql.expr.LibTestExpr.testError ;
+import static org.junit.Assert.assertTrue;
import org.apache.jena.sparql.util.NodeFactoryExtra ;
import org.junit.AfterClass;
@@ -130,7 +131,9 @@ public class TestLeviathanFunctions {
@Test
public void log_03() {
- test("lfn:log(-1)", NodeFactoryExtra.doubleToNode(Double.NaN));
+ NodeValue actual = LibTestExpr.eval("lfn:log(-1)");
+ // Test the object, not the value.
+ assertTrue(NodeValue.nvNaN.equals(actual));
}
@Test
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestNodeValue.java
b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestNodeValue.java
index 8da966aa31..ff2134a1d4 100644
--- a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestNodeValue.java
+++ b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestNodeValue.java
@@ -1067,6 +1067,51 @@ public class TestNodeValue
assertFalse("Different values - notNotSame (" + nv1 + "," + nv3 + ")",
NodeValue.notSameValueAs(nv1, nv3));
}
+ @Test
+ public void testSameValueNaN_double_1() {
+ NodeValue nv1 = NodeValue.makeNode("NaN", XSDDatatype.XSDdouble);
+ NodeValue nv2 = NodeValue.makeNode("NaN", XSDDatatype.XSDdouble);
+ assertEquals(nv1, nv2);
+ assertFalse(NodeValue.sameValueAs(nv1, nv2));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv2));
+ }
+
+ @Test
+ public void testSameValueNaN_float_1() {
+ NodeValue nv1 = NodeValue.makeNode("NaN", XSDDatatype.XSDfloat);
+ NodeValue nv2 = NodeValue.makeNode("NaN", XSDDatatype.XSDfloat);
+ assertEquals(nv1, nv2);
+ assertFalse(NodeValue.sameValueAs(nv1, nv2));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv2));
+ // NaN is weird.
+ assertFalse(NodeValue.sameValueAs(nv1, nv1));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv1));
+ }
+
+ @Test
+ public void testSameValueNaN_double_2() {
+ NodeValue nv1 = NodeValue.makeDouble(Double.NaN);
+ NodeValue nv2 = NodeValue.makeDouble(Double.NaN);
+ assertEquals(nv1, nv2);
+ assertFalse(NodeValue.sameValueAs(nv1, nv2));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv2));
+
+ assertFalse(NodeValue.sameValueAs(nv1, nv1));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv1));
+ }
+
+ @Test
+ public void testSameValueNaN_float_2() {
+ NodeValue nv1 = NodeValue.makeFloat(Float.NaN);
+ NodeValue nv2 = NodeValue.makeFloat(Float.NaN);
+ assertEquals(nv1, nv2);
+ assertFalse(NodeValue.sameValueAs(nv1, nv2));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv2));
+
+ assertFalse(NodeValue.sameValueAs(nv1, nv1));
+ assertTrue(NodeValue.notSameValueAs(nv1, nv1));
+ }
+
@Test
public void testLang1() {
Node n1 = org.apache.jena.graph.NodeFactory.createLiteral("xyz", "en");
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/function/library/TestFnFunctionsString.java
b/jena-arq/src/test/java/org/apache/jena/sparql/function/library/TestFnFunctionsString.java
index 67dce3e37f..af91d6ab0f 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/function/library/TestFnFunctionsString.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/function/library/TestFnFunctionsString.java
@@ -92,7 +92,7 @@ public class TestFnFunctionsString {
@Test public void exprStrNormalizeSpace0() { test("fn:normalize-space('
The wealthy curled darlings of
our nation. ')",
NodeValue.makeString("The wealthy curled darlings of our
nation.")) ; }
- @Test public void exprStrNormalizeSpace1() {
test("fn:normalize-space('')",NodeValue.nvEmptyString) ; }
+ @Test public void exprStrNormalizeSpace1() {
test("fn:normalize-space('')", NodeValue.nvEmptyString) ; }
@Test public void exprStrNormalizeSpace2() { test("fn:normalize-space('
Aaa ')",NodeValue.makeString("Aaa")) ; }
@Test public void exprStrNormalizeSpace3() { test("fn:normalize-space('A a
a a a ')",NodeValue.makeString("A a a a a")) ; }
