openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abe White <awh...@bea.com>
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/...
Date Fri, 06 Apr 2007 14:10:27 GMT
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.

Mime
View raw message