You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by pc...@apache.org on 2007/04/06 00:55:54 UTC
svn commit: r525997 - in /incubator/openjpa/trunk:
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/
openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/
openjpa-jdbc/src/main/...
Author: pcl
Date: Thu Apr 5 15:55:52 2007
New Revision: 525997
URL: http://svn.apache.org/viewvc?view=rev&rev=525997
Log:
OPENJPA-182. Moved to API-based model. Query.setHint() can still be used via the query hint => fetch plan binding.
Removed the logic to override the forUpdate value, since the calling code already incorporates fetch configuration data into its decision about how to invoke toSelect(). Added a test case to assert this behavior.
Also cleaned up some minor whitespace issues, and reduced code duplication by moving a couple of concepts up into DBDictionary. Removed some seemingly-unnecessary overrides from H2Dictionary.
Added a test case for isolation level configuration. For non-DB2 dictionaries, it asserts that an exception is thrown during execution. Someone with DB2 knowledge / access should fill in the test case for the DB2 cases. As we add support for per-query isolation level configuration for other databases, we should change this test case.
Added:
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java (with props)
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java (with props)
Modified:
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java
incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java
incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java Thu Apr 5 15:55:52 2007
@@ -240,4 +240,21 @@
throw translate(re);
}
}
+
+ public int getIsolationLevel() {
+ try {
+ return getJDBCDelegate().getIsolationLevel();
+ } catch (RuntimeException re) {
+ throw translate(re);
+ }
+ }
+
+ public JDBCFetchConfiguration setIsolationLevel(int level) {
+ try {
+ getJDBCDelegate().setIsolationLevel(level);
+ return this;
+ } catch (RuntimeException re) {
+ throw translate(re);
+ }
+ }
}
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java Thu Apr 5 15:55:52 2007
@@ -16,6 +16,7 @@
package org.apache.openjpa.jdbc.kernel;
import java.sql.ResultSet;
+import java.sql.Connection;
import java.util.Collection;
import java.util.Set;
@@ -169,4 +170,38 @@
* Convenience method to cast traversal to store-specific type.
*/
public JDBCFetchConfiguration traverseJDBC(FieldMetaData fm);
+
+ /**
+ * <p>The isolation level for queries issued to the database. This overrides
+ * the persistence-unit-wide <code>openjpa.jdbc.TransactionIsolation</code>
+ * value.</p>
+ *
+ * <p>Must be one of {@link Connection#TRANSACTION_NONE},
+ * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
+ * {@link Connection#TRANSACTION_READ_COMMITTED},
+ * {@link Connection#TRANSACTION_REPEATABLE_READ},
+ * {@link Connection#TRANSACTION_SERIALIZABLE},
+ * or -1 for the default connection level specified by the context in
+ * which this fetch configuration is being used.</p>
+ *
+ * @since 0.9.7
+ */
+ public int getIsolationLevel();
+
+ /**
+ * <p>The isolation level for queries issued to the database. This overrides
+ * the persistence-unit-wide <code>openjpa.jdbc.TransactionIsolation</code>
+ * value.</p>
+ *
+ * <p>Must be one of {@link Connection#TRANSACTION_NONE},
+ * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
+ * {@link Connection#TRANSACTION_READ_COMMITTED},
+ * {@link Connection#TRANSACTION_REPEATABLE_READ},
+ * {@link Connection#TRANSACTION_SERIALIZABLE},
+ * or -1 for the default connection level specified by the context in
+ * which this fetch configuration is being used.</p>
+ *
+ * @since 0.9.7
+ */
+ public JDBCFetchConfiguration setIsolationLevel(int level);
}
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java Thu Apr 5 15:55:52 2007
@@ -17,6 +17,7 @@
import java.io.Serializable;
import java.sql.ResultSet;
+import java.sql.Connection;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
@@ -65,6 +66,7 @@
public int size = 0;
public int syntax = 0;
public Set joins = null;
+ public int isolationLevel = -1;
}
private final JDBCConfigurationState _state;
@@ -318,5 +320,23 @@
if (!(conf instanceof JDBCConfiguration))
return null;
return (JDBCConfiguration) conf;
+ }
+
+ public int getIsolationLevel() {
+ return _state.isolationLevel;
+ }
+
+ public JDBCFetchConfiguration setIsolationLevel(int level) {
+ if (level != -1
+ && level != Connection.TRANSACTION_NONE
+ && level != Connection.TRANSACTION_READ_UNCOMMITTED
+ && level != Connection.TRANSACTION_READ_COMMITTED
+ && level != Connection.TRANSACTION_REPEATABLE_READ
+ && level != Connection.TRANSACTION_SERIALIZABLE)
+ throw new IllegalArgumentException(
+ _loc.get("bad-level", Integer.valueOf(level)).getMessage());
+
+ _state.isolationLevel = level;
+ return this;
}
}
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java Thu Apr 5 15:55:52 2007
@@ -23,8 +23,8 @@
import java.util.StringTokenizer;
import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
import org.apache.openjpa.jdbc.schema.Sequence;
-import org.apache.openjpa.lib.log.Log;
import org.apache.openjpa.util.OpenJPAException;
+import org.apache.openjpa.kernel.LockLevels;
/**
* Dictionary for IBM DB2 database.
@@ -34,20 +34,21 @@
public String optimizeClause = "optimize for";
public String rowClause = "row";
- private int db2ServerType = 0;
- private static final int db2ISeriesV5R3AndEarlier = 1;
+ private int db2ServerType = 0;
+ private static final int db2ISeriesV5R3AndEarlier = 1;
private static final int db2UDBV81OrEarlier = 2;
private static final int db2ZOSV8x = 3;
private static final int db2UDBV82AndLater = 4;
- private static final int db2ISeriesV5R4AndLater = 5;
- private static final String forUpdateOfClause="FOR UPDATE OF";
- private static final String withRSClause="WITH RS";
- private static final String withRRClause="WITH RR";
- private static final String useKeepUpdateLockClause= "USE AND KEEP UPDATE LOCKS";
- private static final String useKeepExclusiveLockClause="USE AND KEEP EXCLUSIVE LOCKS";
- private static final String forReadOnlyClause = "FOR READ ONLY";
- public static final String UPDATE_HINT = "openjpa.hint.updateClause";
- public static final String ISOLATION_HINT = "openjpa.hint.isolationLevel";
+ private static final int db2ISeriesV5R4AndLater = 5;
+ private static final String forUpdateOfClause = "FOR UPDATE OF";
+ private static final String withRSClause = "WITH RS";
+ private static final String withRRClause = "WITH RR";
+ private static final String useKeepUpdateLockClause
+ = "USE AND KEEP UPDATE LOCKS";
+ private static final String useKeepExclusiveLockClause
+ = "USE AND KEEP EXCLUSIVE LOCKS";
+ private static final String forReadOnlyClause = "FOR READ ONLY";
+
public DB2Dictionary() {
platform = "DB2";
validationSQL = "SELECT DISTINCT(CURRENT TIMESTAMP) FROM "
@@ -186,18 +187,18 @@
if (isJDBC3(metaData)) {
int maj = metaData.getDatabaseMajorVersion();
int min = metaData.getDatabaseMinorVersion();
-
+
// Determine the type of DB2 database
if (isDB2ISeriesV5R3AndEarlier(metaData))
- db2ServerType =db2ISeriesV5R3AndEarlier;
+ db2ServerType = db2ISeriesV5R3AndEarlier;
else if (isDB2UDBV81OrEarlier(metaData,maj,min))
- db2ServerType =db2UDBV81OrEarlier;
+ db2ServerType = db2UDBV81OrEarlier;
else if (isDB2ZOSV8x(metaData,maj))
- db2ServerType =db2ZOSV8x;
+ db2ServerType = db2ZOSV8x;
else if (isDB2UDBV82AndLater(metaData,maj,min))
- db2ServerType=db2UDBV82AndLater;
+ db2ServerType = db2UDBV82AndLater;
else if (isDB2ISeriesV5R4AndLater(metaData))
- db2ServerType=db2ISeriesV5R4AndLater;
+ db2ServerType = db2ISeriesV5R4AndLater;
if (maj >= 9 || (maj == 8 && min >= 2)) {
supportsLockingWithMultipleTables = true;
@@ -225,128 +226,126 @@
}
}
}
-
- /** Get the update clause for the query based on the
+
+ /**
+ * Get the update clause for the query based on the
* updateClause and isolationLevel hints
*/
- public String getForUpdateClause(JDBCFetchConfiguration fetch, boolean forUpdate) {
- String isolationLevel = null;
- boolean updateClause;
- DatabaseMetaData metaData = null;
+ protected String getForUpdateClause(JDBCFetchConfiguration fetch,
+ boolean forUpdate) {
+ int isolationLevel;
StringBuffer forUpdateString = new StringBuffer();
try {
- // Determine the update clause/isolationLevel the hint
- // overrides the persistence.xml value
- if (fetch != null && fetch.getHint(UPDATE_HINT)
- !=null )
- updateClause = ((Boolean)fetch.
- getHint(UPDATE_HINT)).booleanValue();
- else
- updateClause = forUpdate;
- if (fetch != null &&fetch.getHint(ISOLATION_HINT)
- !=null )
- isolationLevel = (String)fetch.
- getHint(ISOLATION_HINT);
- else
- isolationLevel = conf.getTransactionIsolation();
- if (updateClause == false)
+ // Determine the isolationLevel; the fetch
+ // configuration data overrides the persistence.xml value
+ if (fetch != null && fetch.getIsolationLevel() != -1)
+ isolationLevel = fetch.getIsolationLevel();
+ else
+ isolationLevel = conf.getTransactionIsolationConstant();
+
+ if (!forUpdate) {
// This sql is not for update so add FOR Read Only clause
forUpdateString.append(" ").append(forReadOnlyClause)
.append(" ");
- else if (updateClause == true){
+ } else {
- switch(db2ServerType){
+ switch(db2ServerType) {
case db2ISeriesV5R3AndEarlier:
- case db2UDBV81OrEarlier:
- if (isolationLevel.equals("read-uncommitted"))
+ case db2UDBV81OrEarlier:
+ if (isolationLevel ==
+ Connection.TRANSACTION_READ_UNCOMMITTED) {
forUpdateString.append(" ").append(withRSClause)
- .append(" ").append(forUpdateOfClause).append(" ");
- else
+ .append(" ").append(forUpdateOfClause).append(" ");
+ } else {
forUpdateString.append(" ").append(forUpdateOfClause)
- .append(" ");
- break;
+ .append(" ");
+ }
+ break;
case db2ZOSV8x:
- case db2UDBV82AndLater:
- if (isolationLevel.equals("serializable"))
+ case db2UDBV82AndLater:
+ if (isolationLevel == Connection.TRANSACTION_SERIALIZABLE) {
forUpdateString.append(" ").append(withRRClause)
- .append(" ").append(useKeepUpdateLockClause)
- .append(" ");
- else
+ .append(" ").append(useKeepUpdateLockClause)
+ .append(" ");
+ } else {
forUpdateString.append(" ").append(withRSClause)
- .append(" ").append(useKeepUpdateLockClause)
- .append(" ");
+ .append(" ").append(useKeepUpdateLockClause)
+ .append(" ");
+ }
break;
case db2ISeriesV5R4AndLater:
- if (isolationLevel.equals("serializable"))
+ if (isolationLevel == Connection.TRANSACTION_SERIALIZABLE) {
forUpdateString.append(" ").append(withRRClause)
- .append(" ").append(useKeepExclusiveLockClause)
- .append(" ");
- else
+ .append(" ").append(useKeepExclusiveLockClause)
+ .append(" ");
+ } else {
forUpdateString.append(" ").append(withRSClause)
- .append(" ").append(useKeepExclusiveLockClause)
- .append(" ");
+ .append(" ").append(useKeepExclusiveLockClause)
+ .append(" ");
+ }
+ break;
}
}
- }
+ }
catch (Exception e) {
if (log.isTraceEnabled())
log.error(e.toString(),e);
}
return forUpdateString.toString();
- }
-
+ }
+
public boolean isDB2UDBV82AndLater(DatabaseMetaData metadata, int maj,
int min) throws SQLException {
boolean match = false;
- if (metadata.getDatabaseProductVersion().indexOf("SQL") != -1
- && ((maj ==8 && min >=2) ||(maj >=8)))
- match = true;
+ if (metadata.getDatabaseProductVersion().indexOf("SQL") != -1
+ && ((maj == 8 && min >= 2) ||(maj >= 8)))
+ match = true;
return match;
}
- public boolean isDB2ZOSV8x(DatabaseMetaData metadata,int maj)
+ public boolean isDB2ZOSV8x(DatabaseMetaData metadata, int maj)
throws SQLException {
boolean match = false;
- if (metadata.getDatabaseProductVersion().indexOf("DSN") != -1
- && maj ==8 )
- match = true;
+ if (metadata.getDatabaseProductVersion().indexOf("DSN") != -1
+ && maj == 8)
+ match = true;
return match;
}
public boolean isDB2ISeriesV5R3AndEarlier(DatabaseMetaData metadata)
throws SQLException {
boolean match = false;
- if (metadata.getDatabaseProductVersion().indexOf("AS") != -1
+ if (metadata.getDatabaseProductVersion().indexOf("AS") != -1
&& generateVersionNumber(metadata.getDatabaseProductVersion())
- <= 530 )
- match = true;
+ <= 530)
+ match = true;
return match;
}
public boolean isDB2ISeriesV5R4AndLater(DatabaseMetaData metadata)
throws SQLException {
boolean match = false;
- if (metadata.getDatabaseProductVersion().indexOf("AS") != -1
+ if (metadata.getDatabaseProductVersion().indexOf("AS") != -1
&& generateVersionNumber(metadata.getDatabaseProductVersion())
- >= 540 )
- match = true;
+ >= 540)
+ match = true;
return match;
}
- public boolean isDB2UDBV81OrEarlier(DatabaseMetaData metadata,int maj,
+ public boolean isDB2UDBV81OrEarlier(DatabaseMetaData metadata, int maj,
int min) throws SQLException {
boolean match = false;
if (metadata.getDatabaseProductVersion().indexOf("SQL") != -1 &&
- ((maj ==8 && min <=1)|| maj <8 ))
- match = true;
+ ((maj == 8 && min <= 1)|| maj < 8))
+ match = true;
return match;
}
/** Get the version number for the ISeries
- */
+ */
protected int generateVersionNumber(String versionString) {
String s = versionString.substring(versionString.indexOf('V'));
- s = s.toUpperCase();
+ s = s.toUpperCase();
int i = -1;
StringTokenizer stringtokenizer = new StringTokenizer(s, "VRM", false);
if (stringtokenizer.countTokens() == 3)
@@ -358,72 +357,17 @@
}
return i;
}
-
-
- /**
- * Override the toOperationMethod of DBDictionary to pass the
- * forUpdateString.
- */
- protected SQLBuffer toOperation(String op, SQLBuffer selects,
- SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
- SQLBuffer order, boolean distinct, boolean forUpdate, long start,
- long end,String forUpdateString) {
- SQLBuffer buf = new SQLBuffer(this);
- buf.append(op);
- boolean range = start != 0 || end != Long.MAX_VALUE;
- if (range && rangePosition == RANGE_PRE_DISTINCT)
- appendSelectRange(buf, start, end);
- if (distinct)
- buf.append(" DISTINCT");
- if (range && rangePosition == RANGE_POST_DISTINCT)
- appendSelectRange(buf, start, end);
- buf.append(" ").append(selects).append(" FROM ").append(from);
-
- if (where != null && !where.isEmpty())
- buf.append(" WHERE ").append(where);
- if (group != null && !group.isEmpty())
- buf.append(" GROUP BY ").append(group);
- if (having != null && !having.isEmpty()) {
- assertSupport(supportsHaving, "SupportsHaving");
- buf.append(" HAVING ").append(having);
- }
- if (order != null && !order.isEmpty())
- buf.append(" ORDER BY ").append(order);
- if (range && rangePosition == RANGE_POST_SELECT)
- appendSelectRange(buf, start, end);
-
- if (!simulateLocking ) {
- assertSupport(supportsSelectForUpdate, "SupportsSelectForUpdate");
- buf.append(" ").append(forUpdateString);
- }
- if (range && rangePosition == RANGE_POST_LOCK)
- appendSelectRange(buf, start, end);
- return buf;
- }
public SQLBuffer toSelect(Select sel, boolean forUpdate,
JDBCFetchConfiguration fetch) {
- sel.addJoinClassConditions();
- boolean update = forUpdate && sel.getFromSelect() == null;
- SQLBuffer select = getSelects(sel, false, update);
- SQLBuffer ordering = null;
- if (!sel.isAggregate() || sel.getGrouping() != null)
- ordering = sel.getOrdering();
- SQLBuffer from;
- if (sel.getFromSelect() != null)
- from = getFromSelect(sel, forUpdate);
- else
- from = getFrom(sel, update);
- SQLBuffer where = getWhere(sel, update);
- String forUpdateString = getForUpdateClause(fetch,forUpdate);
- SQLBuffer buf = toOperation(getSelectOperation(fetch), select,
- from, where,sel.getGrouping(), sel.getHaving(), ordering,
- sel.isDistinct(), forUpdate, sel.getStartIndex(),
- sel.getEndIndex(),forUpdateString);
- if (sel.getExpectedResultCount() > 0)
- buf.append(" ").append(optimizeClause).append(" ").
- append(String.valueOf(sel.getExpectedResultCount())).
- append(" ").append(rowClause);
+ SQLBuffer buf = super.toSelect(sel, forUpdate, fetch);
+
+ if (sel.getExpectedResultCount() > 0) {
+ buf.append(" ").append(optimizeClause).append(" ")
+ .append(String.valueOf(sel.getExpectedResultCount()))
+ .append(" ").append(rowClause);
+ }
+
return buf;
}
@@ -452,20 +396,20 @@
getMethod("getSqlWarn", null);
Method getSqlErrdMethd = sqlca.getClass().
getMethod("getSqlErrd", null);
- exceptionMsg = exceptionMsg.concat( "SQLCA OUTPUT" +
+ exceptionMsg = exceptionMsg.concat( "SQLCA OUTPUT" +
"[Errp=" +getSqlErrpMethd.invoke(sqlca,new Object[]{})
+ ", Errd=" + Arrays.toString((int[])
(getSqlErrdMethd.invoke(sqlca, new Object[]{}))));
String Warn = new String((char[])getSqlWarnMethd.
invoke(sqlca, new Object[]{}));
if(Warn.trim().length() != 0)
- exceptionMsg = exceptionMsg.concat(", Warn=" +Warn + "]" );
+ exceptionMsg = exceptionMsg.concat(", Warn=" +Warn + "]" );
else
- exceptionMsg = exceptionMsg.concat( "]" );
+ exceptionMsg = exceptionMsg.concat( "]" );
msg = msg.concat(exceptionMsg);
return msg;
} catch (Throwable t) {
return sqle.getMessage();
}
}
- }
\ No newline at end of file
+ }
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java Thu Apr 5 15:55:52 2007
@@ -2145,7 +2145,22 @@
SQLBuffer having, SQLBuffer order,
boolean distinct, boolean forUpdate, long start, long end) {
return toOperation(getSelectOperation(fetch), selects, from, where,
- group, having, order, distinct, forUpdate, start, end);
+ group, having, order, distinct, forUpdate, start, end,
+ getForUpdateClause(fetch, forUpdate));
+ }
+
+ /**
+ * Get the update clause for the query based on the
+ * updateClause and isolationLevel hints
+ */
+ protected String getForUpdateClause(JDBCFetchConfiguration fetch,
+ boolean forUpdate) {
+ if (fetch.getIsolationLevel() != -1)
+ throw new IllegalStateException(_loc.get(
+ "isolation-level-config-not-supported", getClass().getName())
+ .getMessage());
+ else
+ return forUpdateClause;
}
/**
@@ -2158,10 +2173,10 @@
/**
* Return the SQL for the given selecting operation.
*/
- protected SQLBuffer toOperation(String op, SQLBuffer selects,
- SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
- SQLBuffer order, boolean distinct, boolean forUpdate, long start,
- long end) {
+ protected SQLBuffer toOperation(String op, SQLBuffer selects,
+ SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
+ SQLBuffer order, boolean distinct, boolean forUpdate, long start,
+ long end, String forUpdateClause) {
SQLBuffer buf = new SQLBuffer(this);
buf.append(op);
@@ -2190,8 +2205,8 @@
if (forUpdate && !simulateLocking) {
assertSupport(supportsSelectForUpdate, "SupportsSelectForUpdate");
- if (forUpdateClause != null)
- buf.append(" ").append(forUpdateClause);
+ if (this.forUpdateClause != null)
+ buf.append(" ").append(this.forUpdateClause);
}
if (range && rangePosition == RANGE_POST_LOCK)
appendSelectRange(buf, start, end);
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/H2Dictionary.java Thu Apr 5 15:55:52 2007
@@ -159,32 +159,12 @@
return buf.toString();
}
- protected SQLBuffer toOperation(String op, SQLBuffer selects,
- SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
- SQLBuffer order, boolean distinct, boolean forUpdate, long start,
- long end) {
- return super.toOperation(op, selects, from, where, group, having,
- order, distinct, forUpdate, start, end);
- }
-
public Column[] getColumns(DatabaseMetaData meta, String catalog,
String schemaName, String tableName, String columnName, Connection conn)
throws SQLException {
Column[] cols = super.getColumns(meta, catalog, schemaName, tableName,
columnName, conn);
return cols;
- }
-
- public void setDouble(PreparedStatement stmnt, int idx, double val,
- Column col)
- throws SQLException {
- super.setDouble(stmnt, idx, val, col);
- }
-
- public void setBigDecimal(PreparedStatement stmnt, int idx, BigDecimal val,
- Column col)
- throws SQLException {
- super.setBigDecimal(stmnt, idx, val, col);
}
protected void appendSelectRange(SQLBuffer buf, long start, long end) {
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/HSQLDictionary.java Thu Apr 5 15:55:52 2007
@@ -190,16 +190,16 @@
return buf.toString();
}
- protected SQLBuffer toOperation(String op, SQLBuffer selects,
- SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
- SQLBuffer order, boolean distinct, boolean forUpdate, long start,
- long end) {
+ protected SQLBuffer toOperation(String op, SQLBuffer selects,
+ SQLBuffer from, SQLBuffer where, SQLBuffer group, SQLBuffer having,
+ SQLBuffer order, boolean distinct, boolean forUpdate, long start,
+ long end, String forUpdateClause) {
// hsql requires ordering when limit is used
if ((start != 0 || end != Long.MAX_VALUE)
&& (order == null || order.isEmpty()))
order = _oneBuffer;
return super.toOperation(op, selects, from, where, group, having,
- order, distinct, forUpdate, start, end);
+ order, distinct, forUpdate, start, end, forUpdateClause);
}
public Column[] getColumns(DatabaseMetaData meta, String catalog,
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties Thu Apr 5 15:55:52 2007
@@ -97,3 +97,8 @@
native-seq-usage: Usage: java org.apache.openjpa.jdbc.kernel.NativeJDBCSeq\n\
\t[-properties/-p <properties file or resource>]\n\
\t[-<property name> <property value>]*
+bad-level: Invalid isolation level. Valid levels are -1, \
+ Connection.TRANSACTION_NONE, Connection.TRANSACTION_READ_UNCOMMITTED, \
+ Connection.TRANSACTION_READ_COMMITTED, \
+ Connection.TRANSACTION_REPEATABLE_READ, or \
+ Connection.TRANSACTION_SERIALIZABLE. Specified value: {0}.
\ No newline at end of file
Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties (original)
+++ incubator/openjpa/trunk/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/localizer.properties Thu Apr 5 15:55:52 2007
@@ -160,3 +160,5 @@
less than 9.2. Downgrading the driver will solve this, or it can be \
worked around by setting the "SupportsTimestampNanos" DBDictionary \
property to "true".
+isolation-level-config-not-supported: This DBDictionary does not support \
+ customization of isolation levels on a per-query basis. DBDictionary: {0}.
\ No newline at end of file
Modified: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java (original)
+++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlan.java Thu Apr 5 15:55:52 2007
@@ -15,6 +15,8 @@
*/
package org.apache.openjpa.persistence.jdbc;
+import java.sql.Connection;
+
import org.apache.openjpa.jdbc.kernel.EagerFetchModes;
import org.apache.openjpa.jdbc.kernel.LRSSizes;
import org.apache.openjpa.jdbc.sql.JoinSyntaxes;
@@ -114,4 +116,38 @@
* @see JoinSyntaxes
*/
public JDBCFetchPlan setJoinSyntax(int syntax);
+
+ /**
+ * <p>The isolation level for queries issued to the database. This overrides
+ * the persistence-unit-wide <code>openjpa.jdbc.TransactionIsolation</code>
+ * value.</p>
+ *
+ * <p>Must be one of {@link Connection#TRANSACTION_NONE},
+ * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
+ * {@link Connection#TRANSACTION_READ_COMMITTED},
+ * {@link Connection#TRANSACTION_REPEATABLE_READ},
+ * {@link Connection#TRANSACTION_SERIALIZABLE},
+ * or -1 for the default connection level specified by the context in
+ * which this fetch plan is being used.</p>
+ *
+ * @since 0.9.7
+ */
+ public int getIsolationLevel();
+
+ /**
+ * <p>The isolation level for queries issued to the database. This overrides
+ * the persistence-unit-wide <code>openjpa.jdbc.TransactionIsolation</code>
+ * value.</p>
+ *
+ * <p>Must be one of {@link Connection#TRANSACTION_NONE},
+ * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
+ * {@link Connection#TRANSACTION_READ_COMMITTED},
+ * {@link Connection#TRANSACTION_REPEATABLE_READ},
+ * {@link Connection#TRANSACTION_SERIALIZABLE},
+ * or -1 for the default connection level specified by the context in
+ * which this fetch plan is being used.</p>
+ *
+ * @since 0.9.7
+ */
+ public JDBCFetchPlan setIsolationLevel(int level);
}
Modified: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java?view=diff&rev=525997&r1=525996&r2=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java (original)
+++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/JDBCFetchPlanImpl.java Thu Apr 5 15:55:52 2007
@@ -102,4 +102,13 @@
_fetch.setJoinSyntax(syntax);
return this;
}
+
+ public int getIsolationLevel() {
+ return _fetch.getIsolationLevel();
+ }
+
+ public JDBCFetchPlan setIsolationLevel(int level) {
+ _fetch.setIsolationLevel(level);
+ return this;
+ }
}
Added: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java?view=auto&rev=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java (added)
+++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java Thu Apr 5 15:55:52 2007
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ *
+ * Licensed 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.jdbc;
+
+import java.sql.Connection;
+import javax.persistence.EntityManager;
+import javax.persistence.LockModeType;
+import javax.persistence.PersistenceException;
+
+import org.apache.openjpa.persistence.test.SQLListenerTestCase;
+import org.apache.openjpa.persistence.simple.AllFieldTypes;
+import org.apache.openjpa.persistence.OpenJPAPersistence;
+import org.apache.openjpa.persistence.FetchPlan;
+import org.apache.openjpa.persistence.OpenJPAEntityManager;
+import org.apache.openjpa.jdbc.sql.DBDictionary;
+import org.apache.openjpa.jdbc.sql.DB2Dictionary;
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+
+public class TestIsolationLevelOverride
+ extends SQLListenerTestCase {
+
+ public void setUp() {
+ setUp(AllFieldTypes.class,
+ "openjpa.Optimistic", "false",
+ "openjpa.LockManager", "pessimistic");
+ }
+
+ public void testIsolationLevelOverride() {
+ OpenJPAEntityManager em =
+ OpenJPAPersistence.cast(emf.createEntityManager());
+ DBDictionary dict = ((JDBCConfiguration) em.getConfiguration())
+ .getDBDictionaryInstance();
+ sql.clear();
+ try {
+ em.getTransaction().begin();
+ ((JDBCFetchPlan) em.getFetchPlan())
+ .setIsolationLevel(Connection.TRANSACTION_SERIALIZABLE);
+ em.find(AllFieldTypes.class, 0);
+
+ if (dict instanceof DB2Dictionary) {
+ assertEquals(1, sql.size());
+ assertSQL(".*DB2-specific SQL to test for goes here.*");
+ } else {
+ fail("OpenJPA currently only supports per-query isolation " +
+ "level configuration on the following databases: DB2");
+ }
+ } catch (PersistenceException pe) {
+ // if we're not using DB2, we expect an IllegalStateException.
+ if (dict instanceof DB2Dictionary
+ || !(pe.getCause() instanceof IllegalStateException))
+ throw pe;
+ } finally {
+ em.getTransaction().rollback();
+ em.close();
+ }
+ }
+}
Propchange: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestIsolationLevelOverride.java
------------------------------------------------------------------------------
svn:eol-style = native
Added: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java
URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java?view=auto&rev=525997
==============================================================================
--- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java (added)
+++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java Thu Apr 5 15:55:52 2007
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ *
+ * Licensed 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.jdbc;
+
+import javax.persistence.EntityManager;
+import javax.persistence.LockModeType;
+
+import org.apache.openjpa.persistence.test.SQLListenerTestCase;
+import org.apache.openjpa.persistence.simple.AllFieldTypes;
+import org.apache.openjpa.persistence.OpenJPAPersistence;
+import org.apache.openjpa.persistence.FetchPlan;
+
+public class TestSelectForUpdateOverride
+ extends SQLListenerTestCase {
+
+ public void setUp() {
+ setUp(AllFieldTypes.class,
+ "openjpa.Optimistic", "false",
+ "openjpa.LockManager", "pessimistic",
+ "openjpa.ReadLockLevel", "none");
+ }
+
+ public void testSelectForUpdateOverride() {
+ EntityManager em = emf.createEntityManager();
+ sql.clear();
+ try {
+ em.getTransaction().begin();
+ OpenJPAPersistence.cast(em).getFetchPlan()
+ .setReadLockMode(LockModeType.WRITE);
+ em.find(AllFieldTypes.class, 0);
+
+ assertEquals(1, sql.size());
+ assertSQL(".*FOR UPDATE.*");
+ } finally {
+ em.getTransaction().rollback();
+ em.close();
+ }
+ }
+}
Propchange: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/TestSelectForUpdateOverride.java
------------------------------------------------------------------------------
svn:eol-style = native
RE: svn commit: r525997 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main
Posted by Patrick Linskey <pl...@bea.com>.
-1 to anything with 'transaction' in it. This is a per-query override
for the transaction isolation level, not a per-transaction setting.
-Patrick
--
Patrick Linskey
BEA Systems, Inc.
_______________________________________________________________________
Notice: This email message, together with any attachments, may contain
information of BEA Systems, Inc., its subsidiaries and affiliated
entities, that may be confidential, proprietary, copyrighted and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.
> -----Original Message-----
> From: Marc Prud'hommeaux [mailto:mprudhomapache@gmail.com] On
> Behalf Of Marc Prud'hommeaux
> Sent: Friday, April 06, 2007 12:48 PM
> To: open-jpa-dev@incubator.apache.org
> Subject: Re: svn commit: r525997 - in
> /incubator/openjpa/trunk:
> openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/
> openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/
> openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel
> / openjpa-jdbc/src/main
>
>
> FWIW, my personal naming preference in descending order:
>
> 1. setTransactionIsolationLevel
> 2. setTransactionIsolation (user: "Is this a boolean? Of course I
> want my transactions to be isolated!")
> 3. setIsolation (user: "What exactly is being 'isolated'? The
> EntityManager or the cache or something?")
> 4. setIsolationLevel (same a #3)
>
> I'd really be fine with any of them, though.
>
>
> On Apr 6, 2007, at 12:31 PM, Abe White wrote:
>
> >>> Why is this setting called "IsolationLevel" where our
> global setting
> >>> is called "TransactionIsolation"? Shouldn't this local
> setting just
> >>> be called "Isolation" for consistency? Same with the
> >>> FetchPlan facade.
> >>
> >> Personally, I feel that 'IsolationLevel' is a
> more-well-known term
> >> for
> >> the concept. I'm fine with either name, though.
> >
> > Does anyone else have an opinion on this? I think get/setIsolation
> > would be more consistent with the global TransactionIsolation
> > property. I doubt the lack of a "Level" suffix is going to hurt.
> >
> > Notice: This email message, together with any attachments, may
> > contain information of BEA Systems, Inc., its subsidiaries
> > and affiliated entities, that may be confidential,
> proprietary,
> > copyrighted and/or legally privileged, and is intended solely for
> > the use of the individual or entity named in this message. If you
> > are not the intended recipient, and have received this message in
> > error, please immediately return this by email and then delete it.
>
>
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: svn commit: r525997 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main
Posted by Marc Prud'hommeaux <mp...@apache.org>.
FWIW, my personal naming preference in descending order:
1. setTransactionIsolationLevel
2. setTransactionIsolation (user: "Is this a boolean? Of course I
want my transactions to be isolated!")
3. setIsolation (user: "What exactly is being 'isolated'? The
EntityManager or the cache or something?")
4. setIsolationLevel (same a #3)
I'd really be fine with any of them, though.
On Apr 6, 2007, at 12:31 PM, Abe White wrote:
>>> Why is this setting called "IsolationLevel" where our global setting
>>> is called "TransactionIsolation"? Shouldn't this local setting just
>>> be called "Isolation" for consistency? Same with the
>>> FetchPlan facade.
>>
>> Personally, I feel that 'IsolationLevel' is a more-well-known term
>> for
>> the concept. I'm fine with either name, though.
>
> Does anyone else have an opinion on this? I think get/setIsolation
> would be more consistent with the global TransactionIsolation
> property. I doubt the lack of a "Level" suffix is going to hurt.
>
> Notice: This email message, together with any attachments, may
> contain information of BEA Systems, Inc., its subsidiaries
> and affiliated entities, that may be confidential, proprietary,
> copyrighted and/or legally privileged, and is intended solely for
> the use of the individual or entity named in this message. If you
> are not the intended recipient, and have received this message in
> error, please immediately return this by email and then delete it.
Re: svn commit: r525997 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main
Posted by Abe White <aw...@bea.com>.
>> Why is this setting called "IsolationLevel" where our global setting
>> is called "TransactionIsolation"? Shouldn't this local setting just
>> be called "Isolation" for consistency? Same with the
>> FetchPlan facade.
>
> Personally, I feel that 'IsolationLevel' is a more-well-known term for
> the concept. I'm fine with either name, though.
Does anyone else have an opinion on this? I think get/setIsolation
would be more consistent with the global TransactionIsolation
property. I doubt the lack of a "Level" suffix is going to hurt.
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
RE: svn commit: r525997 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main
Posted by Patrick Linskey <pl...@bea.com>.
> Why is this setting called "IsolationLevel" where our global setting
> is called "TransactionIsolation"? Shouldn't this local setting just
> be called "Isolation" for consistency? Same with the
> FetchPlan facade.
Personally, I feel that 'IsolationLevel' is a more-well-known term for
the concept. I'm fine with either name, though.
--
Patrick Linskey
BEA Systems, Inc.
_______________________________________________________________________
Notice: This email message, together with any attachments, may contain
information of BEA Systems, Inc., its subsidiaries and affiliated
entities, that may be confidential, proprietary, copyrighted and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.
> -----Original Message-----
> From: Abe White [mailto:awhite@bea.com]
> Sent: Friday, April 06, 2007 7:10 AM
> To: open-jpa-dev@incubator.apache.org
> Subject: Re: svn commit: r525997 - in
> /incubator/openjpa/trunk:
> openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/
> openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/
> openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel
> / openjpa-jdbc/src/main
>
> Comments inline...
>
> > + /**
> > + * <p>The isolation level for queries issued to the database.
> > This overrides
> > + * the persistence-unit-wide
> > <code>openjpa.jdbc.TransactionIsolation</code>
> > + * value.</p>
> > + *
> > + * <p>Must be one of {@link Connection#TRANSACTION_NONE},
> > + * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
> > + * {@link Connection#TRANSACTION_READ_COMMITTED},
> > + * {@link Connection#TRANSACTION_REPEATABLE_READ},
> > + * {@link Connection#TRANSACTION_SERIALIZABLE},
> > + * or -1 for the default connection level specified by the
> > context in
> > + * which this fetch configuration is being used.</p>
> > + *
> > + * @since 0.9.7
> > + */
> > + public int getIsolationLevel();
>
> Why is this setting called "IsolationLevel" where our global setting
> is called "TransactionIsolation"? Shouldn't this local setting just
> be called "Isolation" for consistency? Same with the
> FetchPlan facade.
>
> >
> > public Set joins = null;
> > + public int isolationLevel = -1;
>
> The FetchConfiguration interface defines a DEFAULT constant.
> The doc
> on this constant is: "Constant to revert any setting back to its
> default value." We should either change all the places where we're
> using -1 to use DEFAULT or at least change the setIsolation
> setter to
> look for DEFAULT and translate it to -1.
I chose -1 to parallel the JavaDoc and code in JDBCConfiguration /
JDBCConfigurationImpl -- there, -1 is used for the 'default' alias.
> at least change the setIsolation setter to
> look for DEFAULT and translate it to -1.
Done.
> > + public JDBCFetchConfiguration setIsolationLevel(int level) {
> > + if (level != -1
> > + && level != Connection.TRANSACTION_NONE
> > + && level != Connection.TRANSACTION_READ_UNCOMMITTED
> > + && level != Connection.TRANSACTION_READ_COMMITTED
> > + && level != Connection.TRANSACTION_REPEATABLE_READ
> > + && level != Connection.TRANSACTION_SERIALIZABLE)
> > + throw new IllegalArgumentException(
> > + _loc.get("bad-level", Integer.valueOf
> > (level)).getMessage());
>
> Switch statement...
I prefer if statements, and know of no reason to use a switch instead.
> > + /**
> > + * Get the update clause for the query based on the
> > + * updateClause and isolationLevel hints
> > + */
> > + protected String
> getForUpdateClause(JDBCFetchConfiguration fetch,
> > + boolean forUpdate) {
> > + if (fetch.getIsolationLevel() != -1)
> > + throw new IllegalStateException(_loc.get(
> > + "isolation-level-config-not-supported", getClass
> > ().getName())
> > + .getMessage());
> > + else
> > + return forUpdateClause;
> > }
>
> Any reason we aren't using our own InvalidStateException (extends
> UserException)?
Are there any advantages to doing so? Now that exceptions nest well and
we have exception translation at the boundaries, it seems as though
there's no reason to use our internal exception types vs. standard Java
exceptions like IllegalStateException or IllegalArgumentException. I can
go either way on this though.
> > if (forUpdate && !simulateLocking) {
> > assertSupport(supportsSelectForUpdate,
> > "SupportsSelectForUpdate");
> > - if (forUpdateClause != null)
> > - buf.append(" ").append(forUpdateClause);
> > + if (this.forUpdateClause != null)
> > + buf.append(" ").append(this.forUpdateClause);
> > }
>
> Why are we passing a new "forUpdateClause" param to this method if
> we're going to ignore it and use our "forUpdateClause" member?
Bug. I'm not sure how that got in there. Bummer that we don't have unit
tests for any of the DB2-specific behavior. Fixed in 526212.
-Patrick
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: svn commit: r525997 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/...
Posted by Abe White <aw...@bea.com>.
Comments inline...
> + /**
> + * <p>The isolation level for queries issued to the database.
> This overrides
> + * the persistence-unit-wide
> <code>openjpa.jdbc.TransactionIsolation</code>
> + * value.</p>
> + *
> + * <p>Must be one of {@link Connection#TRANSACTION_NONE},
> + * {@link Connection#TRANSACTION_READ_UNCOMMITTED},
> + * {@link Connection#TRANSACTION_READ_COMMITTED},
> + * {@link Connection#TRANSACTION_REPEATABLE_READ},
> + * {@link Connection#TRANSACTION_SERIALIZABLE},
> + * or -1 for the default connection level specified by the
> context in
> + * which this fetch configuration is being used.</p>
> + *
> + * @since 0.9.7
> + */
> + public int getIsolationLevel();
Why is this setting called "IsolationLevel" where our global setting
is called "TransactionIsolation"? Shouldn't this local setting just
be called "Isolation" for consistency? Same with the FetchPlan facade.
>
> public Set joins = null;
> + public int isolationLevel = -1;
The FetchConfiguration interface defines a DEFAULT constant. The doc
on this constant is: "Constant to revert any setting back to its
default value." We should either change all the places where we're
using -1 to use DEFAULT or at least change the setIsolation setter to
look for DEFAULT and translate it to -1.
> + public JDBCFetchConfiguration setIsolationLevel(int level) {
> + if (level != -1
> + && level != Connection.TRANSACTION_NONE
> + && level != Connection.TRANSACTION_READ_UNCOMMITTED
> + && level != Connection.TRANSACTION_READ_COMMITTED
> + && level != Connection.TRANSACTION_REPEATABLE_READ
> + && level != Connection.TRANSACTION_SERIALIZABLE)
> + throw new IllegalArgumentException(
> + _loc.get("bad-level", Integer.valueOf
> (level)).getMessage());
Switch statement...
> + /**
> + * Get the update clause for the query based on the
> + * updateClause and isolationLevel hints
> + */
> + protected String getForUpdateClause(JDBCFetchConfiguration fetch,
> + boolean forUpdate) {
> + if (fetch.getIsolationLevel() != -1)
> + throw new IllegalStateException(_loc.get(
> + "isolation-level-config-not-supported", getClass
> ().getName())
> + .getMessage());
> + else
> + return forUpdateClause;
> }
Any reason we aren't using our own InvalidStateException (extends
UserException)?
> if (forUpdate && !simulateLocking) {
> assertSupport(supportsSelectForUpdate,
> "SupportsSelectForUpdate");
> - if (forUpdateClause != null)
> - buf.append(" ").append(forUpdateClause);
> + if (this.forUpdateClause != null)
> + buf.append(" ").append(this.forUpdateClause);
> }
Why are we passing a new "forUpdateClause" param to this method if
we're going to ignore it and use our "forUpdateClause" member?
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.