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 [EMAIL PROTECTED] Connection#TRANSACTION_NONE},
> + * [EMAIL PROTECTED] Connection#TRANSACTION_READ_UNCOMMITTED},
> + * [EMAIL PROTECTED] Connection#TRANSACTION_READ_COMMITTED},
> + * [EMAIL PROTECTED] Connection#TRANSACTION_REPEATABLE_READ},
> + * [EMAIL PROTECTED] 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.
