You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by rz...@apache.org on 2026/08/29 17:53:21 UTC
(openjpa) branch OPENJPA-2967 created (now cff73f127)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a change to branch OPENJPA-2967
in repository https://gitbox.apache.org/repos/asf/openjpa.git
at cff73f127 [OPENJPA-2967] Make VERSION() work for surrogate versions
This branch includes the following new commits:
new cff73f127 [OPENJPA-2967] Make VERSION() work for surrogate versions
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(openjpa) 01/01: [OPENJPA-2967] Make VERSION() work for surrogate versions
Posted by rz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch OPENJPA-2967
in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit cff73f127a835f453bf3f261019eb20ff6ec125c
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Sat Aug 29 19:52:14 2026 +0200
[OPENJPA-2967] Make VERSION() work for surrogate versions
VersionVal.getType() returned null for a type whose version is mapped by
the version strategy rather than by a @Version field. initialize() admits
such a type on purpose, since its version columns exist, so the null
escaped into JPQLExpressionBuilder (type.isPrimitive() for SELECT
VERSION(e)) and into Filters.wrap (for WHERE VERSION(e) = :v), both of
which failed with a NullPointerException. getType() now falls back to
Object.class, the same answer the in-memory VersionVal gives, and
toDataStoreValue() converts through the version column's java type.
initialize() also reported an invalid VERSION() argument with the
bad-getobjectid message, which describes an object id failure and whose
argument is null for a bare identification variable. It now uses a
dedicated message naming the path.
Known limitations, each worth its own issue: a multi-column surrogate
version still compares and loads through the first column only, and a
subclass of a versioned entity is rejected because SuperclassVersionStrategy
maps no columns of its own.
---
.../openjpa/jdbc/kernel/exps/VersionVal.java | 53 ++++++++++++++----
.../openjpa/jdbc/kernel/exps/localizer.properties | 2 +
.../jpql/functions/SurrogateVersionEntity.java | 64 ++++++++++++++++++++++
.../jpql/functions/TestVersionFunctionOrderBy.java | 63 ++++++++++++++++++++-
4 files changed, 169 insertions(+), 13 deletions(-)
diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java
index 2b9e9516b..f4d471a6d 100644
--- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java
+++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java
@@ -31,6 +31,7 @@ import org.apache.openjpa.kernel.exps.ExpressionVisitor;
import org.apache.openjpa.lib.util.Localizer;
import org.apache.openjpa.meta.ClassMetaData;
import org.apache.openjpa.meta.FieldMetaData;
+import org.apache.openjpa.meta.JavaTypes;
import org.apache.openjpa.util.UserException;
/**
@@ -78,11 +79,19 @@ class VersionVal
@Override
public Class getType() {
- FieldMetaData versionField = _path.getMetaData().getVersionField();
- if (versionField != null) {
- return versionField.getType();
- }
- return null;
+ // note: getColumns()/initialize() resolve the target type from the
+ // ExpState; getType() has none, so it uses the path's class.
+ ClassMetaData meta = _path.getMetaData();
+ FieldMetaData versionField = (meta == null) ? null : meta.getVersionField();
+ if (versionField != null)
+ return versionField.getType();
+
+ // surrogate version: the version strategy maps column(s) but there is
+ // no version field, so no java type is declared here. Type the value
+ // loosely, exactly as the in-memory VersionVal does, rather than
+ // returning null: callers dereference this (result shapes, comparison
+ // type checks, Filters.convert).
+ return Object.class;
}
@Override
@@ -98,23 +107,43 @@ class VersionVal
// non-pc fields at all
ClassMapping cls = _path.getClassMapping(state);
if (cls == null || cls.getEmbeddingMapping() != null)
- throw new UserException(_loc.get("bad-getobjectid", _path.getFieldMapping(state)));
+ throw new UserException(_loc.get("bad-version-path", pathDescription()));
// types that are not versioned have no version columns to select,
// group, order or compare by; fail with a meaningful message rather
// than a NullPointerException further down the line
- if (cls.getVersion() == null || cls.getVersion().getColumns().length == 0)
+ if (cls.getVersion().getColumns().length == 0)
throw new UserException(_loc.get("no-version-field", cls));
return state;
}
+ /**
+ * A user-recognizable description of the VERSION() argument, for error
+ * messages.
+ */
+ private String pathDescription() {
+ String desc = _path.getPCPathString();
+ if (desc != null && desc.endsWith("."))
+ desc = desc.substring(0, desc.length() - 1);
+ if (desc != null && desc.length() > 0)
+ return desc;
+ String alias = _path.getSchemaAlias();
+ return (alias != null) ? alias : String.valueOf(_path.getMetaData());
+ }
+
@Override
public Object toDataStoreValue(Select sel, ExpContext ctx, ExpState state, Object val) {
- ClassMapping mapping = _path.getClassMapping(state);
- if (mapping.getVersion() != null) {
- return Filters.convert(val, getType());
- }
- return null;
+ ClassMapping cls = _path.getClassMapping(state);
+ FieldMetaData versionField = cls.getVersionField();
+ if (versionField != null)
+ return Filters.convert(val, versionField.getType());
+
+ // surrogate version: convert using the version column's java type,
+ // which the version strategy stamped onto the column
+ Column[] cols = cls.getVersion().getColumns();
+ if (cols.length == 1)
+ return JavaTypes.convert(val, cols[0].getJavaType());
+ return val;
}
@Override
diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties
index cdf5b2070..d61ae128b 100644
--- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties
+++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties
@@ -17,6 +17,8 @@
bad-getobjectid: Attempt to get the object id of a non-persistent or \
embedded object field "{0}".
+bad-version-path: Attempt to obtain the version of a non-persistent or \
+ embedded object "{0}".
no-version-field: Attempt to obtain the version of type "{0}", which does \
not have a version field or version column(s).
non-pers-field: Field "{0}" is not persistent, and thus cannot be queried.
diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/SurrogateVersionEntity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/SurrogateVersionEntity.java
new file mode 100644
index 000000000..e79e9b41b
--- /dev/null
+++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/SurrogateVersionEntity.java
@@ -0,0 +1,64 @@
+/*
+ * 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.functions;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Table;
+
+import org.apache.openjpa.persistence.jdbc.VersionColumn;
+
+/**
+ * An entity with a surrogate version: the version strategy maps a version
+ * column, but the type has no version field.
+ */
+@Entity
+@Table(name = "SURVERENT")
+@VersionColumn
+public class SurrogateVersionEntity {
+
+ @Id
+ private int id;
+
+ private String name;
+
+ public SurrogateVersionEntity() {
+ }
+
+ public SurrogateVersionEntity(int id, String name) {
+ this.id = id;
+ this.name = name;
+ }
+
+ public int getId() {
+ return id;
+ }
+
+ public void setId(int id) {
+ this.id = id;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+}
diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestVersionFunctionOrderBy.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestVersionFunctionOrderBy.java
index bcd86a86b..dd03b8865 100644
--- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestVersionFunctionOrderBy.java
+++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jpql/functions/TestVersionFunctionOrderBy.java
@@ -36,13 +36,15 @@ public class TestVersionFunctionOrderBy extends SQLListenerTestCase {
@Override
public void setUp() {
- setUp(CLEAR_TABLES, CompUser.class, CompVerUser.class, Address.class);
+ setUp(CLEAR_TABLES, CompUser.class, CompVerUser.class, Address.class,
+ SurrogateVersionEntity.class);
EntityManager em = emf.createEntityManager();
em.getTransaction().begin();
em.persist(new CompVerUser("Bob", "n/a", null, 21));
em.persist(new CompVerUser("Alice", "n/a", null, 22));
em.persist(new CompUser("Ugo", "n/a", null, 23));
+ em.persist(new SurrogateVersionEntity(1, "Surro"));
em.getTransaction().commit();
em.close();
}
@@ -107,6 +109,65 @@ public class TestVersionFunctionOrderBy extends SQLListenerTestCase {
msg.contains(CompUser.class.getName()));
assertFalse("Should not fail with a NullPointerException: " + msg,
hasNPE(e));
+ assertFalse("VERSION() must not report an object-id failure: " + msg,
+ msg.contains("object id"));
+ } finally {
+ em.close();
+ }
+ }
+
+ public void testProjectVersionOfSurrogateVersionedType() {
+ EntityManager em = emf.createEntityManager();
+ resetSQL();
+
+ List<?> result = em.createQuery(
+ "SELECT VERSION(e) FROM SurrogateVersionEntity e").getResultList();
+
+ assertEquals(1, result.size());
+ assertNotNull(result.get(0));
+ assertContainsSQL("versn");
+ em.close();
+ }
+
+ public void testWhereVersionOnSurrogateVersionedType() {
+ EntityManager em = emf.createEntityManager();
+ resetSQL();
+
+ List<?> result = em.createQuery(
+ "SELECT e FROM SurrogateVersionEntity e WHERE VERSION(e) = :v")
+ .setParameter("v", 1).getResultList();
+
+ assertEquals(1, result.size());
+ assertContainsSQL("versn = ?");
+ em.close();
+ }
+
+ public void testOrderByVersionOnSurrogateVersionedType() {
+ EntityManager em = emf.createEntityManager();
+ resetSQL();
+
+ List<?> result = em.createQuery(
+ "SELECT e FROM SurrogateVersionEntity e ORDER BY VERSION(e)").getResultList();
+
+ // the version column is selected with the entity anyway, so assert on
+ // the ORDER BY clause itself rather than on the column name alone
+ assertEquals(1, result.size());
+ assertContainsSQL("ORDER BY");
+ assertContainsSQL("ORDER BY t0.versn");
+ em.close();
+ }
+
+ public void testProjectVersionOnUnversionedTypeThrowsUserException() {
+ EntityManager em = emf.createEntityManager();
+ try {
+ em.createQuery("SELECT VERSION(u) FROM CompUser u").getResultList();
+ fail("VERSION() on an unversioned type should be rejected");
+ } catch (RuntimeException e) {
+ String msg = getNestedMessages(e);
+ assertTrue("Unexpected failure: " + msg,
+ msg.contains("does not") && msg.contains("version"));
+ assertFalse("Should not fail with a NullPointerException: " + msg,
+ hasNPE(e));
} finally {
em.close();
}