You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Patrick Linskey <pl...@bea.com> on 2007/02/22 00:06:38 UTC

RE: svn commit: r510281 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Some comments:

1. What does "optimize for n" do? Do you have a link where I could read
up on it?

2. Is there equivalent magic for other databases?

3. OpenJPA does support a means of passing Oracle hints along through to
the DBDictionary. Should we be trying to reuse some of the capabilities
here?

4. In the following snippets, I'd rather if we used 'Integer.valueOf(1)'
or, better yet, a symbolic constant, instead of creating new integer all
the time.

> +        fetch.setHint("openjpa.hint.optimize", new Integer(1));

> +    	_query.getFetchConfiguration().
> +    		setHint("openjpa.hint.optimize", new Integer(1));

5. I don't like the name 'openjpa.hint.optimize', as it's a bit
ambiguous as to what's being optimized. I don't really know what
'optimize for' does, so I'm just guessing here, but how about
'openjpa.ExpectedRecordCount'?

6. How does the user access this from JPQL queries when they don't want
to use a getSingleResult() call? Will it work to use the JPA query hint
facilities, or do they have to use the broker APIs? Do we even care,
since they could just use getSingleResult()? Maybe it's better to not
allow this to be user-configurable, if we can always infer the right
setting. But I'm guessing that sometimes the user might want to say that
they expect 10 results or something like that

7. It seems like this could also be useful for when a one-to-one or a
many-to-one is traversed.

-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: wisneskid@apache.org [mailto:wisneskid@apache.org] 
> Sent: Wednesday, February 21, 2007 2:50 PM
> To: open-jpa-commits@incubator.apache.org
> Subject: svn commit: r510281 - in /incubator/openjpa/trunk: 
> openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ 
> openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ 
> openjpa-persistence/src/main/java/org/apache/openjpa/persistence/
> 
> Author: wisneskid
> Date: Wed Feb 21 14:50:04 2007
> New Revision: 510281
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=510281
> Log:
> DB2 Optimize for clause enhancement
> 
> Modified:
>     
> 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-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java
>     
> incubator/openjpa/trunk/openjpa-persistence/src/main/java/org/
apache/openjpa/persistence/QueryImpl.java
> 
> 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-j
dbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java?>
view=diff&rev=510281&r1=510280&r2=510281
> ==============================================================
> ================
> --- 
> 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 Wed Feb 21 14:50:04 2007
> @@ -20,6 +20,7 @@
>  import java.sql.SQLException;
>  import java.util.Arrays;
>  
> +import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
>  import org.apache.openjpa.jdbc.schema.Sequence;
>  
>  /**
> @@ -33,7 +34,7 @@
>          validationSQL = "SELECT DISTINCT(CURRENT TIMESTAMP) FROM "
>              + "SYSIBM.SYSTABLES";
>          supportsSelectEndIndex = true;
> -
> +        optimizeClause ="optimize for";
>          nextSequenceQuery = "VALUES NEXTVAL FOR {0}";
>  
>          sequenceSQL = "SELECT SEQSCHEMA AS SEQUENCE_SCHEMA, "
> @@ -190,5 +191,21 @@
>                  }
>              }
>      	}
> +    }
> +    
> +    public String getOptimizeClause(JDBCFetchConfiguration fetch) {
> +        Integer rows = null;
> +        StringBuffer optimizeString = null;
> +        if (fetch.getHint("openjpa.hint.optimize") != null) {
> +            optimizeString = new StringBuffer();
> +            rows = (Integer)fetch.getHint("openjpa.hint.optimize");
> +            optimizeString.append(" 
> ").append(optimizeClause).append(" ")
> +                .append(rows).append(" ");
> +            if(rows.intValue() > 1)
> +    		    optimizeString.append(rowsClause).append(" ");
> +            else
> +    		    optimizeString.append(rowClause).append(" ");
> +        }
> +        return optimizeString.toString();    
>      }
>  }
> 
> 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-j
dbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java?>
view=diff&rev=510281&r1=510280&r2=510281
> ==============================================================
> ================
> --- 
> 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 Wed Feb 21 14:50:04 2007
> @@ -184,6 +184,9 @@
>      public String crossJoinClause = "CROSS JOIN";
>      public boolean requiresConditionForCrossJoin = false;
>      public String forUpdateClause = "FOR UPDATE";
> +    public String optimizeClause = null;
> +    public String rowClause = "row";
> +    public String rowsClause = "rows";
>      public String tableForUpdateClause = null;
>      public String distinctCountColumnSeparator = null;
>      public boolean supportsSelectForUpdate = true;
> @@ -2143,10 +2146,22 @@
>          SQLBuffer from, SQLBuffer where, SQLBuffer group,
>          SQLBuffer having, SQLBuffer order,
>          boolean distinct, boolean forUpdate, long start, long end) {
> -        return toOperation(getSelectOperation(fetch), 
> selects, from, where,
> +    	
> +        String optimizeString = null;
> +        SQLBuffer selString = toOperation(getSelectOperation(fetch), 
> +            selects, from, where,
>              group, having, order, distinct, forUpdate, start, end);
> +        if (fetch != null)
> +            optimizeString = getOptimizeClause(fetch);
> +        if (optimizeString != null)
> +            selString.append(optimizeString);
> +        return selString;    	
>      }
>  
> +    public String getOptimizeClause(JDBCFetchConfiguration fetch) {
> +        return null;    	
> +    }
> +    
>      /**
>       * Return the "SELECT" operation clause, adding any 
> available hints, etc.
>       */
> 
> Modified: 
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java
> URL: 
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-k
ernel/src/main/java/org/apache/openjpa/kernel/BrokerImpl.java?>
view=diff&rev=510281&r1=510280&r2=510281
> ==============================================================
> ================
> --- 
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java (original)
> +++ 
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apach
e/openjpa/kernel/BrokerImpl.java Wed Feb 21 14:50:04 2007
> @@ -765,7 +765,7 @@
>          }
>          if (fetch == null)
>              fetch = _fc;
> -
> +        fetch.setHint("openjpa.hint.optimize", new Integer(1));
>          beginOperation(true);
>          try {
>              assertNontransactionalRead();
> 
> Modified: 
> incubator/openjpa/trunk/openjpa-persistence/src/main/java/org/
apache/openjpa/persistence/QueryImpl.java
> URL: 
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-p
ersistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java?>
view=diff&rev=510281&r1=510280&r2=510281
> ==============================================================
> ================
> --- 
> incubator/openjpa/trunk/openjpa-persistence/src/main/java/org/
apache/openjpa/persistence/QueryImpl.java (original)
> +++ 
> incubator/openjpa/trunk/openjpa-persistence/src/main/java/org/
apache/openjpa/persistence/QueryImpl.java Wed Feb 21 14:50:04 2007
> @@ -279,6 +279,8 @@
>       */
>      public Object getSingleResult() {
>          _em.assertNotCloseInvoked();
> +    	_query.getFetchConfiguration().
> +    		setHint("openjpa.hint.optimize", new Integer(1));
>          Object ob = execute();
>          if (!(ob instanceof List))
>              return ob;
> 
> 
> 

Re: svn commit: r510281 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Kevin Sutter <kw...@gmail.com>.
I have backed out the original changes via svn revision 512327 -- just did
it this morning.  Dave will create a JIRA report for this optimization so
that we can work through the issues and then we'll integrate a new set of
changes.

Kevin

On 2/22/07, Abe White <aw...@bea.com> wrote:
>
> > 3. OpenJPA does support a means of passing Oracle hints along
> > through to
> > the DBDictionary. Should we be trying to reuse some of the
> > capabilities
> > here?
>
> +1
>
> > 4. In the following snippets, I'd rather if we used 'Integer.valueOf
> > (1)'
> > or, better yet, a symbolic constant, instead of creating new
> > integer all
> > the time.
> >
> >> +        fetch.setHint("openjpa.hint.optimize", new Integer(1));
> >
> >> +            _query.getFetchConfiguration().
> >> +                    setHint("openjpa.hint.optimize", new Integer(1));
>
> Use serp.util.Numbers.valueOf(x).
> But actually I think both of these calls have to be more thoroughly
> re-thought.  The FetchConfiguration is around for the life of the
> Broker/Query, and you're setting a hint on it that only applies to
> the very next call.  What about all the other uses of the Broker or
> possible other executions of the Query?
>
> > 5. I don't like the name 'openjpa.hint.optimize', as it's a bit
> > ambiguous as to what's being optimized. I don't really know what
> > 'optimize for' does, so I'm just guessing here, but how about
> > 'openjpa.ExpectedRecordCount'?
>
> +1  But should be openjpa.hint.XXX -- see OracleDictionary.SELECT_HINT.
>
> I think we should move all this to a
> SelectExecutor.ExpectedResultCount property.  It could replace the
> current Union.isSingleResult property.  The user can set an expected
> result count via the Query hint API, and it will get set into the
> Select.  In cases where a query range is set the property will return
> the max - min of the range.  And in cases where we know there's only
> one result like find() calls (actually JDBCStoreManager.load) we can
> use the property directly internally.  The DBDictionary can then ask
> the Select for the expected count and do what it wants with it when
> creating the SQL.
>
>
> _______________________________________________________________________
> 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: r510281 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Abe White <aw...@bea.com>.
> 3. OpenJPA does support a means of passing Oracle hints along  
> through to
> the DBDictionary. Should we be trying to reuse some of the  
> capabilities
> here?

+1

> 4. In the following snippets, I'd rather if we used 'Integer.valueOf 
> (1)'
> or, better yet, a symbolic constant, instead of creating new  
> integer all
> the time.
>
>> +        fetch.setHint("openjpa.hint.optimize", new Integer(1));
>
>> +    	_query.getFetchConfiguration().
>> +    		setHint("openjpa.hint.optimize", new Integer(1));

Use serp.util.Numbers.valueOf(x).
But actually I think both of these calls have to be more thoroughly  
re-thought.  The FetchConfiguration is around for the life of the  
Broker/Query, and you're setting a hint on it that only applies to  
the very next call.  What about all the other uses of the Broker or  
possible other executions of the Query?

> 5. I don't like the name 'openjpa.hint.optimize', as it's a bit
> ambiguous as to what's being optimized. I don't really know what
> 'optimize for' does, so I'm just guessing here, but how about
> 'openjpa.ExpectedRecordCount'?

+1  But should be openjpa.hint.XXX -- see OracleDictionary.SELECT_HINT.

I think we should move all this to a  
SelectExecutor.ExpectedResultCount property.  It could replace the  
current Union.isSingleResult property.  The user can set an expected  
result count via the Query hint API, and it will get set into the  
Select.  In cases where a query range is set the property will return  
the max - min of the range.  And in cases where we know there's only  
one result like find() calls (actually JDBCStoreManager.load) we can  
use the property directly internally.  The DBDictionary can then ask  
the Select for the expected count and do what it wants with it when  
creating the SQL.


_______________________________________________________________________
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.