From 87cd28415d29fcc96f2638951957c10db700724d Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Mon, 2 Nov 2026 19:54:53 +0100 Subject: [PATCH] [OPENJPA-3050] Return null for arithmetic with a null operand evaluated in memory --- .../openjpa/jdbc/kernel/JDBCStoreQuery.java | 6 + .../org/apache/openjpa/kernel/Filters.java | 41 ++-- .../org/apache/openjpa/kernel/exps/Abs.java | 3 + .../apache/openjpa/kernel/exps/Ceiling.java | 3 + .../openjpa/kernel/exps/Exponential.java | 3 + .../org/apache/openjpa/kernel/exps/Floor.java | 3 + .../openjpa/kernel/exps/NaturalLogarithm.java | 3 + .../org/apache/openjpa/kernel/exps/Sign.java | 3 + .../org/apache/openjpa/kernel/exps/Sqrt.java | 3 + .../TestNullArithmeticInMemory.java | 197 ++++++++++++++++++ .../asciidoc/migration_considerations.adoc | 10 + 11 files changed, 246 insertions(+), 29 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestNullArithmeticInMemory.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreQuery.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreQuery.java index 789357d614..18ce1c3b6e 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreQuery.java +++ b/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()); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java index 854c733e45..73575508fe 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java +++ b/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); } /** diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Abs.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Abs.java index bb6a634c9d..87ea7fd990 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Abs.java +++ b/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()); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Ceiling.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Ceiling.java index 19123b661c..f46e6e7aa5 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Ceiling.java +++ b/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()); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Exponential.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Exponential.java index ef34c7a784..a6f7fe854c 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Exponential.java +++ b/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()); } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Floor.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Floor.java index 95b1ffdf31..8d69d2b6c9 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Floor.java +++ b/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()); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/NaturalLogarithm.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/NaturalLogarithm.java index 38bb1e805c..1e241c2195 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/NaturalLogarithm.java +++ b/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()); } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sign.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sign.java index 0816bfc571..996ce7b3c1 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sign.java +++ b/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) diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sqrt.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sqrt.java index e9d207f4ba..a001465e73 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Sqrt.java +++ b/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()); } } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestNullArithmeticInMemory.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestNullArithmeticInMemory.java new file mode 100644 index 0000000000..37c4545c4e --- /dev/null +++ b/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 OPENJPA-3050 + */ +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; + } +} diff --git a/openjpa-project/src/docs/asciidoc/migration_considerations.adoc b/openjpa-project/src/docs/asciidoc/migration_considerations.adoc index 1d3af0affb..815d1472e4 100644 --- a/openjpa-project/src/docs/asciidoc/migration_considerations.adoc +++ b/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