From 6dcf26af7a353c6f156c711974bf5bdd48faf8dd Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Wed, 2 Sep 2026 21:16:55 +0200 Subject: [PATCH 1/2] [OPENJPA-2956] Unwrap ID(), honour null precedence and reject set operations in memory Three separate defects on the in-memory path, all reachable: the executor is selected whenever a candidate collection is supplied, when the store does not support datastore execution, or when dirty instances are queried with FlushBeforeQueries disabled. ID() returned the internal identity wrapper rather than the raw key, so a comparison against the plain key threw a ClassCastException out of Filters.convert for numeric ids, never matched for an @EmbeddedId, and matched only by accident for a String id. It now unwraps exactly as the JDBC projection does. The wrapper-returning getObjectId() is unchanged. NULLS FIRST and NULLS LAST were ignored: the comparator hard coded nulls last when ascending and first when descending, so two of the four combinations were right by chance and two were silently wrong. The requested precedence is now threaded through, falling back to the previous policy when none is given. Set operations produced a NullPointerException from a compound expression with no filter, or an empty result. They are now rejected with a message that says why the query is running in memory and how to avoid it: the executor is built for one candidate extent and has no multiset semantics, and a candidate collection has no defined meaning across operands. Note that the in-memory path cannot yet be exercised end to end from JPQL with an identification variable: JPQLExpressionBuilder casts the value from getThis() to Path, and the in-memory factory returns a Val, so it fails with a ClassCastException. That is an older, separate defect and wants its own issue. --- .../openjpa/kernel/ExpressionStoreQuery.java | 8 +++ .../kernel/exps/GetNativeObjectId.java | 50 +++++++++++++++++++ .../exps/InMemoryExpressionFactory.java | 30 ++++++++--- .../openjpa/kernel/localizer.properties | 9 ++++ 4 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/GetNativeObjectId.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java index 293611fd37..09c784ffe7 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java @@ -627,6 +627,14 @@ public InMemoryExecutor(ExpressionStoreQuery q, _exps = new QueryExpressions[] { parser.eval(parsed, q, _factory, _meta) }; + // set operations require a datastore query; the in-memory + // executor is built for a single candidate extent and has no + // multiset semantics + if (_exps[0].setOperationType != QueryExpressions.SET_OP_NONE) { + throw new UnsupportedException(_loc.get("inmem-set-op", + q.getContext().getCandidateType(), + q.getContext().getQueryString())); + } if (_exps[0].projections.length == 0) _projTypes = StoreQuery.EMPTY_CLASSES; else { diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/GetNativeObjectId.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/GetNativeObjectId.java new file mode 100644 index 0000000000..235441ddc7 --- /dev/null +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/GetNativeObjectId.java @@ -0,0 +1,50 @@ +/* + * 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.kernel.exps; + +import org.apache.openjpa.kernel.StoreContext; +import org.apache.openjpa.util.OpenJPAId; + +/** + * Get the native identity value of an object, i.e. the raw primary key value + * the application sees as the entity identifier, rather than the internal + * {@link OpenJPAId} wrapper. In-memory counterpart of + * org.apache.openjpa.jdbc.kernel.exps.GetNativeObjectId. + */ +class GetNativeObjectId + extends GetObjectId { + + + private static final long serialVersionUID = 1L; + + /** + * Constructor. Provide value whose native oid to extract. + */ + public GetNativeObjectId(Val val) { + super(val); + } + + @Override + protected Object eval(Object candidate, Object orig, + StoreContext ctx, Object[] params) { + Object oid = super.eval(candidate, orig, ctx, params); + return (oid != null && OpenJPAId.class.isAssignableFrom(oid.getClass())) + ? ((OpenJPAId) oid).getIdObject() : oid; + } +} diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/InMemoryExpressionFactory.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/InMemoryExpressionFactory.java index 479c308a07..2d70d810e2 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/InMemoryExpressionFactory.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/InMemoryExpressionFactory.java @@ -306,14 +306,17 @@ private List order(QueryExpressions exps, Value[] orderValues, int results = (projected) ? exps.projections.length : 0; boolean[] asc = (projected) ? exps.ascending : null; + int[] nulls = (projected) ? exps.nullPrecedence : null; int idx; for (int i = orderValues.length - 1; i >= 0; i--) { // if this is a projection, then in project() we must have selected // the ordering value already after the projection values idx = (results > 0) ? results + i : -1; + int nullPrec = (nulls == null || i >= nulls.length) + ? QueryExpressions.NULLS_DEFAULT : nulls[i]; Collections.sort(matches, new OrderValueComparator((Val) orderValues[i], - asc == null || asc[i], idx, ctx, params)); + asc == null || asc[i], nullPrec, idx, ctx, params)); } return matches; } @@ -818,7 +821,7 @@ public Value getObjectId(Value val) { @Override public Value getNativeObjectId(Value val) { - return new GetObjectId((Val) val); + return new GetNativeObjectId((Val) val); } @Override @@ -865,8 +868,9 @@ public boolean equals(Object other) { /** * Comparator that uses the result of eval'ing a Value to sort on. Null - * values are placed last if sorting in ascending order, first if - * descending. + * values are placed according to the given null precedence; when no null + * precedence is specified they are placed last if sorting in ascending + * order, first if descending. */ private static class OrderValueComparator implements Comparator { @@ -874,14 +878,16 @@ private static class OrderValueComparator private final StoreContext _ctx; private final Val _val; private final boolean _asc; + private final int _nullPrec; private final int _idx; private final Object[] _params; - private OrderValueComparator(Val val, boolean asc, int idx, - StoreContext ctx, Object[] params) { + private OrderValueComparator(Val val, boolean asc, int nullPrec, + int idx, StoreContext ctx, Object[] params) { _ctx = ctx; _val = val; _asc = asc; + _nullPrec = nullPrec; _idx = idx; _params = params; } @@ -898,10 +904,18 @@ public int compare(Object o1, Object o2) { if (o1 == null && o2 == null) return 0; + boolean nullsFirst; + if (_nullPrec == QueryExpressions.NULLS_FIRST) { + nullsFirst = true; + } else if (_nullPrec == QueryExpressions.NULLS_LAST) { + nullsFirst = false; + } else { + nullsFirst = !_asc; + } if (o1 == null) - return (_asc) ? 1 : -1; + return (nullsFirst) ? -1 : 1; if (o2 == null) - return (_asc) ? -1 : 1; + return (nullsFirst) ? 1 : -1; if (o1 instanceof Boolean && o2 instanceof Boolean) { int i1 = (Boolean) o1 ? 1 : 0; diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties index f5069ead58..692a5f59ac 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties @@ -245,6 +245,15 @@ inmem-agg-proj-var: Queries with aggregates or projections using variables \ set the openjpa.FlushBeforeQueries property to true, or execute the query \ before changing any instances in the transaction. The offending query was \ on type "{0}" with filter "{1}". +inmem-set-op: Set operations (UNION, INTERSECT and EXCEPT) cannot be evaluated \ + in-memory; they require a datastore query. OpenJPA is executing this \ + query in-memory because a candidate collection was supplied, because the \ + datastore does not support query execution, or because there are dirty \ + instances in the transaction and openjpa.FlushBeforeQueries is disabled. \ + Either remove the candidate collection, set IgnoreCache to true, set the \ + openjpa.FlushBeforeQueries property to true, or execute the query before \ + changing any instances in the transaction. The offending query on type \ + "{0}" was: {1}. merged-order-with-result: This query on candidate type "{0}" with filter "{1}" \ involves combining the results of multiple queries in memory. \ You have chosen to order the results on "{2}", but you have not selected \ From b31dd36eabf446c32c71473941579bad12b72037 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Thu, 3 Sep 2026 19:27:26 +0200 Subject: [PATCH 2/2] [OPENJPA-2956] Cover the in-memory null precedence and set operation rejection Review feedback. Both cases run through the in-memory executor by supplying a candidate collection, and both fail against the unchanged kernel: the null precedence test because the comparator ignored NULLS FIRST/LAST, the set operation test because the query failed with a NullPointerException instead of saying it cannot be evaluated in memory. ID() is deliberately not covered. It cannot be reached from JPQL in memory at all: JPQLExpressionBuilder casts the value from getThis() to Path, the in-memory factory returns a Val, and the query dies with a ClassCastException before the identity is evaluated. That is an older, separate defect; a test for ID() has to wait for it. --- .../TestInMemoryScalarExpressions.java | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestInMemoryScalarExpressions.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestInMemoryScalarExpressions.java index 115d9df565..4849812d0a 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestInMemoryScalarExpressions.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/expressions/TestInMemoryScalarExpressions.java @@ -91,6 +91,67 @@ public void setUp() { endEm(em); } + /** + * OPENJPA-2956: NULLS FIRST and NULLS LAST were ignored in memory, which + * left two of the four combinations with the wrong order. + */ + public void testNullPrecedenceInMemory() { + EntityManager em = currentEntityManager(); + List rsall = em.createQuery("SELECT e from CompUser e").getResultList(); + + // two users have a null country + assertNull(first(em, rsall, + "SELECT e.address.country FROM CompUser e" + + " ORDER BY e.address.country ASC NULLS FIRST")); + assertNotNull(first(em, rsall, + "SELECT e.address.country FROM CompUser e" + + " ORDER BY e.address.country ASC NULLS LAST")); + assertNull(first(em, rsall, + "SELECT e.address.country FROM CompUser e" + + " ORDER BY e.address.country DESC NULLS FIRST")); + assertNotNull(first(em, rsall, + "SELECT e.address.country FROM CompUser e" + + " ORDER BY e.address.country DESC NULLS LAST")); + + endEm(em); + } + + /** + * OPENJPA-2956: a set operation cannot be evaluated in memory, and must + * say so rather than return a wrong result. + */ + public void testSetOperationInMemoryIsRejected() { + EntityManager em = currentEntityManager(); + List rsall = em.createQuery("SELECT e from CompUser e").getResultList(); + + org.apache.openjpa.persistence.QueryImpl q1 = + (org.apache.openjpa.persistence.QueryImpl) em.createQuery( + "SELECT e.name FROM CompUser e WHERE e.age > 25" + + " UNION SELECT e.name FROM CompUser e WHERE e.age > 30"); + try { + ((QueryImpl) q1.getDelegate()).setCandidateCollection(rsall); + q1.getResultList(); + fail("a set operation must not be evaluated in memory"); + } catch (RuntimeException e) { + StringBuilder sb = new StringBuilder(); + for (Throwable t = e; t != null; t = t.getCause()) { + sb.append(t.getMessage()).append(' '); + } + String msg = sb.toString(); + assertTrue("must be rejected, not merely fail: " + msg, + msg.contains("cannot be evaluated in-memory")); + } finally { + endEm(em); + } + } + + private Object first(EntityManager em, List candidates, String jpql) { + org.apache.openjpa.persistence.QueryImpl q1 = + (org.apache.openjpa.persistence.QueryImpl) em.createQuery(jpql); + ((QueryImpl) q1.getDelegate()).setCandidateCollection(candidates); + return q1.getResultList().get(0); + } + public void testCoalesceExpressions() { EntityManager em = currentEntityManager(); List rsall = em.createQuery("SELECT e from CompUser e")