[OPENJPA-3050] Return null for arithmetic with a null operand evaluated in memory

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreQuery.java

@@ -1368,6 +1368,9 @@ private Object handleAbsVal(Object value, Object ob, Object[] params,
         org.apache.openjpa.jdbc.kernel.exps.Abs absVal =
             (org.apache.openjpa.jdbc.kernel.exps.Abs) value;
         Object val = getValue(absVal.getValue(), ob, params, sm);
+        if (val == null) {
+            return null;
+        }
         Class c = val.getClass();
         if (c == Integer.class)
             return Math.abs(((Integer) val).intValue());
@@ -1385,6 +1388,9 @@ private Object handleSqrtVal(Object value, Object ob, Object[] params,
         org.apache.openjpa.jdbc.kernel.exps.Sqrt sqrtVal =
             (org.apache.openjpa.jdbc.kernel.exps.Sqrt) value;
         Object val = getValue(sqrtVal.getValue(), ob, params, sm);
+        if (val == null) {
+            return null;
+        }
         Class c = val.getClass();
         if (c == Integer.class)
             return Math.sqrt(((Integer) val).doubleValue());

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java

@@ -57,9 +57,6 @@
  */
 public class Filters {
 
-    private static final BigDecimal ZERO_BIGDECIMAL = new BigDecimal(0D);
-    private static final BigInteger ZERO_BIGINTEGER = new BigInteger("0");
-
     private static final int OP_ADD = 0;
     private static final int OP_SUBTRACT = 1;
     private static final int OP_MULTIPLY = 2;
@@ -495,48 +492,34 @@ public static Object round(Object o1, Class<?> c1, Object o2, Class<?> c2) {
     }
 
     /**
-     * Perform the given operation on two numbers.
+     * Perform the given operation on two numbers. As in SQL, the result is
+     * null if either number is null.
      */
     private static Object op(Object o1, Class<?> c1, Object o2, Class<?> c2, int op) {
+        if (o1 == null || o2 == null) {
+            return null;
+        }
         Class<?> promote = promote(c1, c2);
         if (promote == int.class) {
-            int n1 = (o1 == null) ? 0 : ((Number) o1).intValue();
-            int n2 = (o2 == null) ? 0 : ((Number) o2).intValue();
-            return op(n1, n2, op);
+            return op(((Number) o1).intValue(), ((Number) o2).intValue(), op);
         }
         if (promote == float.class) {
-            float n1 = (o1 == null) ? 0F : ((Number) o1).floatValue();
-            float n2 = (o2 == null) ? 0F : ((Number) o2).floatValue();
-            return op(n1, n2, op);
+            return op(((Number) o1).floatValue(), ((Number) o2).floatValue(), op);
         }
         if (promote == double.class) {
-            double n1 = (o1 == null) ? 0D : ((Number) o1).doubleValue();
-            double n2 = (o2 == null) ? 0D : ((Number) o2).doubleValue();
-            return op(n1, n2, op);
+            return op(((Number) o1).doubleValue(), ((Number) o2).doubleValue(), op);
         }
         if (promote == long.class) {
-            long n1 = (o1 == null) ? 0L : ((Number) o1).longValue();
-            long n2 = (o2 == null) ? 0L : ((Number) o2).longValue();
-            return op(n1, n2, op);
+            return op(((Number) o1).longValue(), ((Number) o2).longValue(), op);
         }
         if (promote == BigDecimal.class) {
-            BigDecimal n1 = (o1 == null) ? ZERO_BIGDECIMAL
-                : (BigDecimal) convert(o1, promote);
-            BigDecimal n2 = (o2 == null) ? ZERO_BIGDECIMAL
-                : (BigDecimal) convert(o2, promote);
-            return op(n1, n2, op);
+            return op((BigDecimal) convert(o1, promote), (BigDecimal) convert(o2, promote), op);
         }
         if (promote == BigInteger.class) {
-            BigInteger n1 = (o1 == null) ? ZERO_BIGINTEGER
-                : (BigInteger) convert(o1, promote);
-            BigInteger n2 = (o2 == null) ? ZERO_BIGINTEGER
-                : (BigInteger) convert(o2, promote);
-            return op(n1, n2, op);
+            return op((BigInteger) convert(o1, promote), (BigInteger) convert(o2, promote), op);
         }
         // default to int
-        int n1 = (o1 == null) ? 0 : ((Number) o1).intValue();
-        int n2 = (o2 == null) ? 0 : ((Number) o2).intValue();
-        return op(n1, n2, op);
+        return op(((Number) o1).intValue(), ((Number) o2).intValue(), op);
     }
 
     /**

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Abs.java

@@ -54,6 +54,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         c = Filters.wrap(c);
         if (c == Integer.class)
             return Math.abs(((Number) o).intValue());

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Ceiling.java

@@ -56,6 +56,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         c = Filters.wrap(c);
         if (c == Integer.class)
             return Math.ceil(((Number) o).intValue());

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Exponential.java

@@ -42,6 +42,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         return Math.exp(((Number) o).doubleValue());
     }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Floor.java

@@ -56,6 +56,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         c = Filters.wrap(c);
         if (c == Integer.class)
             return Math.floor(((Number) o).intValue());

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/NaturalLogarithm.java

@@ -42,6 +42,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         return Math.log(((Number) o).doubleValue());
     }
 }

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sign.java

@@ -46,6 +46,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         c = Filters.wrap(c);
         float value = 0;
         if (c == Integer.class || c == Float.class || c == Double.class || c == Long.class)

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sqrt.java

@@ -43,6 +43,9 @@ protected Class getType(Class c) {
 
     @Override
     protected Object operate(Object o, Class c) {
+        if (o == null) {
+            return null;
+        }
         return Math.sqrt(((Number) o).doubleValue());
     }
 }

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestNullArithmeticInMemory.java

@@ -0,0 +1,197 @@
+/*
+ * 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.openjpa.persistence.jpql.expressions;
+
+import java.math.BigDecimal;
+import java.util.Arrays;
+import java.util.List;
+
+import jakarta.persistence.EntityManager;
+
+import org.apache.openjpa.jdbc.sql.DBDictionary;
+import org.apache.openjpa.kernel.QueryImpl;
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+/**
+ * Arithmetic with a null operand evaluated in memory yields null, as it does
+ * in the database, instead of treating the null operand as zero.
+ *
+ * @see <A HREF="https://issues.apache.org/jira/browse/OPENJPA-3050">OPENJPA-3050</A>
+ */
+public class TestNullArithmeticInMemory extends SingleEMFTestCase {
+
+    private static final String ARITHMETIC = "SELECT -n.intB, 0 - n.intB, n.intA + n.intB, "
+        + "n.intA * n.intB, n.intA / n.intB, MOD(n.intA, n.intB), n.longA - n.intB, "
+        + "-n.decA, n.decA + 1, n.doubleA * n.decA FROM SignedOperands n WHERE n.id = 1";
+
+    @Override
+    public void setUp() {
+        super.setUp(CLEAR_TABLES, SignedOperands.class);
+        try (EntityManager em = emf.createEntityManager()) {
+            em.getTransaction().begin();
+            em.persist(new SignedOperands(1, 7, null, (short) 2, 10L, null, 1.5));
+            em.persist(new SignedOperands(2, 7, 0, (short) 2, 10L, BigDecimal.ONE, 1.5));
+            em.getTransaction().commit();
+        }
+    }
+
+    public void testArithmeticWithNullOperandInDatabase() {
+        try (EntityManager em = emf.createEntityManager()) {
+            Object[] row = (Object[]) em.createQuery(ARITHMETIC).getSingleResult();
+            assertAllNull(row);
+        }
+    }
+
+    public void testArithmeticWithNullOperandInMemory() {
+        try (EntityManager em = emf.createEntityManager()) {
+            Object[] row = (Object[]) inMemory(em, ARITHMETIC).get(0);
+            assertAllNull(row);
+        }
+    }
+
+    public void testNumericFunctionsWithNullOperandInMemory() {
+        try (EntityManager em = emf.createEntityManager()) {
+            Object[] row = (Object[]) inMemory(em, "SELECT ABS(n.intB), SQRT(n.intB), "
+                + "SIGN(n.intB), CEILING(n.decA), FLOOR(n.decA), EXP(n.decA), LN(n.decA), "
+                + "POWER(n.intA, n.intB), ROUND(n.decA, 1) FROM SignedOperands n "
+                + "WHERE n.id = 1").get(0);
+            assertAllNull(row);
+        }
+    }
+
+    public void testNullResultInsideCoalesceInMemory() {
+        try (EntityManager em = emf.createEntityManager()) {
+            // a function nested in another expression must return null itself,
+            // at the top level a failure on null is turned into null anyway
+            Object[] row = (Object[]) inMemory(em, "SELECT COALESCE(n.intB + 1, 5), "
+                + "COALESCE(-n.intB, 6), COALESCE(ABS(n.intB), 7), COALESCE(SQRT(n.decA), 8), "
+                + "COALESCE(SIGN(n.intB), 9), COALESCE(CEILING(n.decA), 10), "
+                + "COALESCE(FLOOR(n.decA), 11), COALESCE(EXP(n.decA), 12), "
+                + "COALESCE(LN(n.decA), 13) FROM SignedOperands n WHERE n.id = 1").get(0);
+            for (int i = 0; i < row.length; i++) {
+                String column = "column " + i + " of " + Arrays.toString(row);
+                assertNotNull(column, row[i]);
+                assertEquals(column, i + 5, ((Number) row[i]).intValue());
+            }
+        }
+    }
+
+    public void testNullOperandDoesNotMatchInMemory() {
+        try (EntityManager em = emf.createEntityManager()) {
+            // with the null operand taken as zero, the row with intB = null
+            // matched just like the row with intB = 0
+            assertEquals(List.of(2L), em.createQuery("SELECT m.id FROM SignedOperands m "
+                + "WHERE m.intA + m.intB = 7").getResultList());
+            assertEquals(List.of(2L), inMemory(em, "SELECT n.id FROM SignedOperands n "
+                + "WHERE n.intA + n.intB = 7"));
+            assertEquals(List.of(2L), inMemory(em, "SELECT n.id FROM SignedOperands n "
+                + "WHERE -n.intB = 0"));
+            assertEquals(List.of(2L), inMemory(em, "SELECT n.id FROM SignedOperands n "
+                + "WHERE n.decA * 2 > 1"));
+        }
+    }
+
+    public void testAggregatesSkipNullInMemory() {
+        try (EntityManager em = emf.createEntityManager()) {
+            Object[] row = (Object[]) inMemory(em, "SELECT SUM(n.intB), AVG(n.decA), "
+                + "SUM(n.intA) FROM SignedOperands n").get(0);
+            assertEquals(0L, ((Number) row[0]).longValue());
+            assertEquals(0, BigDecimal.ONE.compareTo(new BigDecimal(row[1].toString())));
+            assertEquals(14L, ((Number) row[2]).longValue());
+        }
+    }
+
+    /**
+     * An update over a candidate collection evaluates the new values with the
+     * in-memory expressions.
+     */
+    public void testBulkUpdateOverCandidatesKeepsNull() {
+        try (EntityManager em = emf.createEntityManager()) {
+            em.getTransaction().begin();
+            List<?> candidates = em.createQuery("SELECT n FROM SignedOperands n").getResultList();
+            org.apache.openjpa.persistence.QueryImpl<?> q = query(em, candidates,
+                "UPDATE SignedOperands n SET n.intB = n.intB - 1, n.decA = -n.decA");
+            assertEquals(2, q.executeUpdate());
+            em.getTransaction().commit();
+        }
+        assertStored(1, null, null);
+        assertStored(2, -1, new BigDecimal("-1"));
+    }
+
+    /**
+     * A bulk update the database dictionary cannot express is executed in
+     * memory by the JDBC store query.
+     */
+    public void testBulkUpdateInMemoryByStoreKeepsNull() {
+        DBDictionary dict = getDBDictionary();
+        boolean supportsSubselect = dict.supportsSubselect;
+        dict.supportsSubselect = false;
+        try (EntityManager em = emf.createEntityManager()) {
+            em.getTransaction().begin();
+            assertEquals(2, em.createQuery("UPDATE SignedOperands n "
+                + "SET n.intB = n.intB + 1, n.decA = n.decA * 2 WHERE n.intA = 7")
+                .executeUpdate());
+            assertEquals(2, em.createQuery("UPDATE SignedOperands n "
+                + "SET n.intB = ABS(n.intB) WHERE n.intA = 7").executeUpdate());
+            // the primitive field cannot hold null and gets 0
+            assertEquals(1, em.createQuery("UPDATE SignedOperands n "
+                + "SET n.doubleA = SQRT(n.intB) WHERE n.id = 1").executeUpdate());
+            em.getTransaction().commit();
+        } finally {
+            dict.supportsSubselect = supportsSubselect;
+        }
+        assertStored(1, null, null);
+        assertStored(2, 1, new BigDecimal("2"));
+        try (EntityManager em = emf.createEntityManager()) {
+            assertEquals(0D, em.find(SignedOperands.class, 1L).getDoubleA());
+        }
+    }
+
+    private void assertStored(long id, Integer intB, BigDecimal decA) {
+        try (EntityManager em = emf.createEntityManager()) {
+            SignedOperands n = em.find(SignedOperands.class, id);
+            assertEquals(intB, n.getIntB());
+            if (decA == null) {
+                assertNull(n.getDecA());
+            } else {
+                assertEquals(0, decA.compareTo(n.getDecA()));
+            }
+        }
+    }
+
+    private static void assertAllNull(Object[] row) {
+        for (int i = 0; i < row.length; i++) {
+            assertNull("column " + i + " of " + Arrays.toString(row), row[i]);
+        }
+    }
+
+    private static List<?> inMemory(EntityManager em, String jpql) {
+        List<?> candidates = em.createQuery("SELECT n FROM SignedOperands n ORDER BY n.id")
+            .getResultList();
+        return query(em, candidates, jpql).getResultList();
+    }
+
+    private static org.apache.openjpa.persistence.QueryImpl<?> query(EntityManager em,
+        List<?> candidates, String jpql) {
+        org.apache.openjpa.persistence.QueryImpl<?> q =
+            (org.apache.openjpa.persistence.QueryImpl<?>) em.createQuery(jpql);
+        ((QueryImpl) q.getDelegate()).setCandidateCollection(candidates);
+        return q;
+    }
+}

openjpa-project/src/docs/asciidoc/migration_considerations.adoc

@@ -104,6 +104,16 @@ Relative precedence of the additive and the multiplicative operators is unchange
 Queries that relied on the previous grouping have to be parenthesized explicitly.
 No compatibility option exists.
 
+[[jpa_4.2_InMemoryNullArithmetic]]
+==== Arithmetic with null is null in memory
+
+A query evaluated in memory, for example over a candidate collection, or a bulk update executed in memory took a null operand of `+`, `-`, `*`, `/`, `MOD`, `POWER` and `ROUND` as zero, so `-e.intField` or `e.intField + 1` returned a number for a null field and a division by a null operand failed.
+As in the database, the result is now null, and `ABS`, `SQRT`, `SIGN`, `CEILING`, `FLOOR`, `EXP` and `LN` of null are null as well.
+A bulk update stores null instead of a number.
+A comparison with such a result is evaluated like a comparison with a null field: `=`, `<`, `>` and `BETWEEN` do not match, while `<>` matches, unlike in the database, because in memory `<>` is true when exactly one side is null.
+The public methods [method]``Filters.add``, [method]``subtract``, [method]``multiply``, [method]``divide``, [method]``mod``, [method]``power`` and [method]``round`` return null for a null argument.
+No compatibility option exists.
+
 [[jpa_4.2_QueryValueConversion]]
 ==== Query value conversion