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 1ae44bfe7..b21290ff4 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 @@ -28,6 +28,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Iterator; import org.apache.openjpa.event.LifecycleEventManager; import org.apache.openjpa.jdbc.kernel.exps.ExpContext; @@ -62,6 +63,7 @@ import org.apache.openjpa.lib.rop.ResultObjectProvider; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.meta.ClassMetaData; import org.apache.openjpa.meta.ValueMetaData; +import org.apache.openjpa.meta.FieldMetaData; import org.apache.openjpa.util.UserException; import serp.util.Numbers; @@ -437,7 +439,11 @@ public class JDBCStoreQuery // we cannot execute a bulk delete statement when have mappings in // multiple tables, so indicate we want to use in-memory with null ClassMapping[] mappings = (ClassMapping[]) metas; + + // specification of the "updates" map indicates that this is + // an update query; otherwise, this is a delete statement boolean isUpdate = updates != null && updates.size() > 0; + for (int i = 0; i < mappings.length; i++) { if (!isSingleTableMapping(mappings[i], subclasses) && !isUpdate) return null; @@ -471,13 +477,11 @@ public class JDBCStoreQuery subclasses, exps[i], state[i], JDBCFetchConfiguration.EAGER_NONE); - // specification of the "udpates" map indicates that this is - // an update query; otherwise, this is a delete statement // The bulk operation will return null to indicate that the database // does not support the request bulk delete operation; in // this case, we need to perform the query in-memory and // manually delete the instances - if (updates == null) + if (!isUpdate) sql[i] = dict.toDelete(mappings[i], sel, params); else sql[i] = dict.toUpdate(mappings[i], sel, _store, params, diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/FieldMapping.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/FieldMapping.java index e1976567d..14b0f3c56 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/FieldMapping.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/FieldMapping.java @@ -918,7 +918,14 @@ public class FieldMapping } public Column[] getColumns() { - return _val.getColumns(); + // pcl: 6 July 2007: this seems a bit hacky, but if the mapping is a + // version, it will have a NoneFieldMapping (since the version strategy + // for the class takes care of it's mapping), and NoneFieldStrategies + // do not have columns. + if (isVersion()) + return getDeclaringMapping().getVersion().getColumns(); + else + return _val.getColumns(); } public void setColumns(Column[] cols) { diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/QueryResultMapping.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/QueryResultMapping.java index 290e45747..3c07fc2fb 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/QueryResultMapping.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/QueryResultMapping.java @@ -413,8 +413,6 @@ public class QueryResultMapping throw new MetaDataException(_loc.get("untraversable-path", QueryResultMapping.this, _candidate, path)); Column[] cols = last.getColumns(); - if (last.isVersion()) - cols = candidate.getVersion().getColumns(); assertSingleColumn(cols, path); Column col = cols[0]; diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/Version.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/Version.java index 67bc467c9..3086daa5a 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/Version.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/Version.java @@ -19,6 +19,7 @@ package org.apache.openjpa.jdbc.meta; import java.sql.SQLException; +import java.util.Map; import org.apache.openjpa.jdbc.kernel.JDBCStore; import org.apache.openjpa.jdbc.schema.Column; @@ -365,4 +366,14 @@ public class Version public String toString() { return _mapping + ""; } + + /** + * @return a Map specifying how to update each version + * column in this instance during a bulk update. + * + * @since 1.0.0 + */ + public Map getBulkUpdateValues() { + return _strategy.getBulkUpdateValues(); + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/VersionStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/VersionStrategy.java index 7ca0f3bd8..60c80c2f1 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/VersionStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/VersionStrategy.java @@ -19,10 +19,12 @@ package org.apache.openjpa.jdbc.meta; import java.sql.SQLException; +import java.util.Map; import org.apache.openjpa.jdbc.kernel.JDBCStore; import org.apache.openjpa.jdbc.sql.Result; import org.apache.openjpa.jdbc.sql.Select; +import org.apache.openjpa.jdbc.schema.Column; import org.apache.openjpa.kernel.OpenJPAStateManager; import org.apache.openjpa.kernel.StoreManager; @@ -74,4 +76,12 @@ public interface VersionStrategy * @see StoreManager#compareVersion */ public int compareVersion(Object v1, Object v2); + + /** + * @return a Map specifying how to update each version + * column during a bulk update. + * + * @since 1.0.0 + */ + public Map getBulkUpdateValues(); } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/AbstractVersionStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/AbstractVersionStrategy.java index 60e7adce3..58bf9ad75 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/AbstractVersionStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/AbstractVersionStrategy.java @@ -19,6 +19,8 @@ package org.apache.openjpa.jdbc.meta.strats; import java.sql.SQLException; +import java.util.Map; +import java.util.Collections; import org.apache.openjpa.jdbc.kernel.JDBCStore; import org.apache.openjpa.jdbc.meta.ClassMapping; @@ -67,4 +69,8 @@ public abstract class AbstractVersionStrategy public int compareVersion(Object v1, Object v2) { return StoreManager.VERSION_SAME; } + + public Map getBulkUpdateValues() { + return Collections.EMPTY_MAP; + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/NumberVersionStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/NumberVersionStrategy.java index 7997be50f..2f2d8e224 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/NumberVersionStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/NumberVersionStrategy.java @@ -18,7 +18,11 @@ */ package org.apache.openjpa.jdbc.meta.strats; +import java.util.Map; +import java.util.HashMap; + import org.apache.openjpa.meta.JavaTypes; +import org.apache.openjpa.jdbc.schema.Column; import serp.util.Numbers; /** @@ -60,4 +64,12 @@ public class NumberVersionStrategy return _initial; return Numbers.valueOf(((Number) version).intValue() + 1); } + + public Map getBulkUpdateValues() { + Column[] cols = vers.getColumns(); + Map map = new HashMap(cols.length); + for (int i = 0; i < cols.length; i++) + map.put(cols[i], cols[i].getName() + " + 1"); + return map; + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SuperclassVersionStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SuperclassVersionStrategy.java index 908025c7c..b73e77bb8 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SuperclassVersionStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/SuperclassVersionStrategy.java @@ -19,6 +19,7 @@ package org.apache.openjpa.jdbc.meta.strats; import java.sql.SQLException; +import java.util.Map; import org.apache.openjpa.jdbc.kernel.JDBCStore; import org.apache.openjpa.kernel.OpenJPAStateManager; @@ -48,4 +49,9 @@ public class SuperclassVersionStrategy return vers.getClassMapping().getPCSuperclassMapping().getVersion(). compareVersion(v1, v2); } + + public Map getBulkUpdateValues() { + return vers.getClassMapping().getPCSuperclassMapping().getVersion() + .getBulkUpdateValues(); + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/TimestampVersionStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/TimestampVersionStrategy.java index a84b7b5c5..ed91d13a6 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/TimestampVersionStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/TimestampVersionStrategy.java @@ -19,8 +19,12 @@ package org.apache.openjpa.jdbc.meta.strats; import java.sql.Timestamp; +import java.util.Map; +import java.util.HashMap; +import java.util.Date; import org.apache.openjpa.jdbc.meta.JavaSQLTypes; +import org.apache.openjpa.jdbc.schema.Column; /** * Uses a timestamp for optimistic versioning. @@ -43,4 +47,13 @@ public class TimestampVersionStrategy protected Object nextVersion(Object version) { return new Timestamp(System.currentTimeMillis()); } + + public Map getBulkUpdateValues() { + Column[] cols = vers.getColumns(); + Map map = new HashMap(cols.length); + Date d = new Date(); + for (int i = 0; i < cols.length; i++) + map.put(cols[i], d); + return map; + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java index a6d3edcfe..e894987d3 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java @@ -58,6 +58,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.HashMap; import javax.sql.DataSource; import org.apache.commons.lang.StringUtils; @@ -1883,12 +1884,23 @@ public class DBDictionary sql.append(" SET "); ExpContext ctx = new ExpContext(store, params, store.getFetchConfiguration()); + + // If the updates map contains any version fields, assume that the + // optimistic lock version data is being handled properly by the + // caller. Otherwise, give the version indicator an opportunity to + // add more update clauses as needed. + boolean augmentUpdates = true; + for (Iterator i = updateParams.entrySet().iterator(); i.hasNext();) { Map.Entry next = (Map.Entry) i.next(); - FieldMetaData fmd = (FieldMetaData) next.getKey(); + FieldMapping fmd = (FieldMapping) next.getKey(); + + if (fmd.isVersion()) + augmentUpdates = false; + Val val = (Val) next.getValue(); - Column col = ((FieldMapping) fmd).getColumns()[0]; + Column col = fmd.getColumns()[0]; sql.append(col.getName()); sql.append(" = "); @@ -1904,6 +1916,21 @@ public class DBDictionary if (i.hasNext()) sql.append(", "); } + + if (augmentUpdates) { + ClassMapping meta = + ((FieldMapping) updateParams.keySet().iterator().next()) + .getDeclaringMapping(); + Map updates = meta.getVersion().getBulkUpdateValues(); + for (Iterator iter = updates.entrySet().iterator(); + iter.hasNext(); ) { + Map.Entry e = (Map.Entry) iter.next(); + Column col = (Column) e.getKey(); + String val = (String) e.getValue(); + sql.append(", ").append(col.getName()) + .append(" = ").append(val); + } + } } /** diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java index 995212414..20092367b 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/OptimisticLockInstance.java @@ -35,6 +35,7 @@ public class OptimisticLockInstance { private int oplock; private String str; + private int intField; protected OptimisticLockInstance() { } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkUpdatesAndVersionColumn.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkUpdatesAndVersionColumn.java new file mode 100644 index 000000000..4d702a011 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestBulkUpdatesAndVersionColumn.java @@ -0,0 +1,101 @@ +/* + * 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.datacache; + +import javax.persistence.EntityManager; +import javax.persistence.LockModeType; +import javax.persistence.OptimisticLockException; +import javax.persistence.RollbackException; + +import org.apache.openjpa.persistence.OpenJPAEntityManager; +import org.apache.openjpa.persistence.OpenJPAPersistence; +import org.apache.openjpa.persistence.test.SingleEMFTestCase; +import org.apache.openjpa.jdbc.meta.ClassMapping; +import org.apache.openjpa.jdbc.meta.FieldMapping; + +public class TestBulkUpdatesAndVersionColumn + extends SingleEMFTestCase { + + public void setUp() throws Exception { + setUp("openjpa.DataCache", "true", + "openjpa.RemoteCommitProvider", "sjvm", + "openjpa.Log", "SQL=TRACE", + OptimisticLockInstance.class, CLEAR_TABLES); + + OpenJPAEntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + OptimisticLockInstance pc = new OptimisticLockInstance("foo"); + em.persist(pc); + em.getTransaction().commit(); + em.close(); + } + + public void testSelectOnOplockField() { + EntityManager em = emf.createEntityManager(); + em.createQuery("select o from OptimisticLockInstance o " + + "where o.oplock = 0").getResultList(); + em.close(); + } + + public void testOplockFieldMapping() { + ClassMapping cm = (ClassMapping) OpenJPAPersistence.getMetaData( + emf, OptimisticLockInstance.class); + FieldMapping fm = cm.getFieldMapping("oplock"); + assertEquals(1, fm.getColumns().length); + } + + public void testBulkUpdateWithManualVersionIncrement() { + bulkUpdateHelper(true); + } + + public void testBulkUpdateWithoutManualVersionIncrement() { + bulkUpdateHelper(false); + } + + private void bulkUpdateHelper(boolean incrementVersionField) { + EntityManager em = emf.createEntityManager(); + + em.getTransaction().begin(); + OptimisticLockInstance oli = (OptimisticLockInstance) em.createQuery( + "SELECT o FROM OptimisticLockInstance o WHERE o.str = 'foo'") + .getSingleResult(); + assertNotNull(oli); + em.lock(oli, LockModeType.READ); + + EntityManager em2 = emf.createEntityManager(); + em2.getTransaction().begin(); + em2.createQuery("UPDATE OptimisticLockInstance o SET o.str = 'foo', " + + "o.intField = o.intField + 1" + + (incrementVersionField ? ", o.oplock = o.oplock + 1 " : "") + + "WHERE o.str = 'foo'") + .executeUpdate(); + em2.getTransaction().commit(); + em2.close(); + + try { + em.getTransaction().commit(); + fail("transaction should have failed"); + } catch (RollbackException re) { + assertTrue("nested exception must be an oplock exception", + re.getCause() instanceof OptimisticLockException); + } finally { + em.close(); + } + } +}