From 442c1cee4870edc0916191e1872ae8223496e84f Mon Sep 17 00:00:00 2001 From: "David J. Wisneski" Date: Mon, 26 Mar 2007 17:59:03 +0000 Subject: [PATCH] Patch for OPENJPA-168 git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@522581 13f79535-47bb-0310-9956-ffa450edef68 --- .../openjpa/jdbc/kernel/JDBCStoreManager.java | 10 ++- .../openjpa/jdbc/kernel/JDBCStoreQuery.java | 14 ++++ .../openjpa/jdbc/meta/strats/LRSProxyMap.java | 2 +- .../meta/strats/RelationFieldStrategy.java | 2 +- .../openjpa/jdbc/sql/DB2Dictionary.java | 67 ++++++++++++++++++- .../apache/openjpa/jdbc/sql/LogicalUnion.java | 55 +++++++++++---- .../openjpa/jdbc/sql/SelectExecutor.java | 19 ++++++ .../apache/openjpa/jdbc/sql/SelectImpl.java | 36 +++++++++- .../org/apache/openjpa/jdbc/sql/Union.java | 14 ---- .../openjpa/kernel/AbstractStoreQuery.java | 1 + .../org/apache/openjpa/kernel/QueryImpl.java | 6 +- .../openjpa/kernel/localizer.properties | 3 + .../apache/openjpa/persistence/QueryImpl.java | 41 +++++++----- 13 files changed, 218 insertions(+), 52 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java index 62d837b2c..94c86c74d 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreManager.java @@ -43,6 +43,7 @@ import org.apache.openjpa.jdbc.sql.SQLExceptions; import org.apache.openjpa.jdbc.sql.SQLFactory; import org.apache.openjpa.jdbc.sql.Select; import org.apache.openjpa.jdbc.sql.SelectExecutor; +import org.apache.openjpa.jdbc.sql.SelectImpl; import org.apache.openjpa.jdbc.sql.Union; import org.apache.openjpa.kernel.FetchConfiguration; import org.apache.openjpa.kernel.LockManager; @@ -368,9 +369,12 @@ public class JDBCStoreManager if (!select(sel, mapping, subs, sm, null, fetch, JDBCFetchConfiguration.EAGER_JOIN, true, false)) return null; - sel.wherePrimaryKey(sm.getObjectId(), mapping, this); - return sel.execute(this, fetch); + //Set the expectedResultCount for the select as 1 as a single + //object is being loaded. force = true is an indicator that it is + //internally generated value + sel.setExpectedResultCount(1,true); + return sel.execute(this, fetch); } /** @@ -385,7 +389,7 @@ public class JDBCStoreManager JDBCFetchConfiguration.EAGER_JOIN); Union union = _sql.newUnion(mappings.length); - union.setSingleResult(true); + union.setExpectedResultCount(1,true); if (fetch.getSubclassFetchMode(mapping) != fetch.EAGER_JOIN) union.abortUnion(); union.select(new Union.Selector() { 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 f5f727173..e70e9ef5a 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 @@ -312,6 +312,7 @@ public class JDBCStoreQuery ClassMapping[] verts; boolean unionable = true; Select sel; + Object optHint = null; for (int i = 0; i < mappings.length; i++) { // determine vertical mappings to select separately verts = getVerticalMappings(mappings[i], subclasses, exps[i], @@ -322,6 +323,19 @@ public class JDBCStoreQuery // create criteria select and clone for each vert mapping sel = ((JDBCExpressionFactory) facts[i]).getSelectConstructor(). evaluate(ctx, null, null, exps[i], states[i]); + //it means it is coming from getSingleResult so set the + //expectedResultCount to 1.force = true indicates that this is + //internally generated value + if(this.ctx.isUnique()) + sel.setExpectedResultCount(1,true); + //it means this is coming from getResultList so set the + //expectedResultCount based on any optimize hint if provided + else{ + if((optHint = ctx.fetch.getHint + (this.optimizeHint))!= null) + sel.setExpectedResultCount + (((Integer)optHint).intValue(),false); + } for (int j = 0; j < verts.length; j++) { selMappings.add(verts[j]); if (j == verts.length - 1) { diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/LRSProxyMap.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/LRSProxyMap.java index 0ea6cd798..c3a2173dd 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/LRSProxyMap.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/LRSProxyMap.java @@ -238,7 +238,7 @@ class LRSProxyMap final Joins[] resJoins = new Joins[Math.max(1, clss.length)]; Union union = store.getSQLFactory().newUnion (Math.max(1, clss.length)); - union.setSingleResult(true); + union.setExpectedResultCount(1,true); if (fetch.getSubclassFetchMode(_strat.getFieldMapping(). getElementMapping().getTypeMapping()) != JDBCFetchConfiguration.EAGER_JOIN) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java index 8b46adca3..9352a17ec 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java @@ -578,7 +578,7 @@ public class RelationFieldStrategy // back to our fk table if not an inverse mapping (in which case we // can just make sure the inverse cols == our pk values) Union union = store.getSQLFactory().newUnion(rels.length); - union.setSingleResult(true); + union.setExpectedResultCount(1,true); if (fetch.getSubclassFetchMode(field.getTypeMapping()) != JDBCFetchConfiguration.EAGER_JOIN) union.abortUnion(); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java index 9d26e9821..dffd7cf05 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java @@ -20,6 +20,7 @@ import java.sql.DatabaseMetaData; import java.sql.SQLException; import java.util.Arrays; +import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration; import org.apache.openjpa.jdbc.schema.Sequence; /** @@ -27,7 +28,10 @@ import org.apache.openjpa.jdbc.schema.Sequence; */ public class DB2Dictionary extends AbstractDB2Dictionary { - + + //variables to support optimize clause + public String optimizeClause = "optimize for"; + public String rowClause = "row"; public DB2Dictionary() { platform = "DB2"; validationSQL = "SELECT DISTINCT(CURRENT TIMESTAMP) FROM " @@ -193,4 +197,65 @@ public class DB2Dictionary } } } + //based on the expectedResultCount of the select create the optimize + //for clause + public String getOptimizeClause(JDBCFetchConfiguration fetch, + int expectedResultCount) { + Integer rows = null; + StringBuffer optimizeString = new StringBuffer(); + + if(expectedResultCount != 0) + optimizeString.append(" ").append(optimizeClause).append(" ") + .append(expectedResultCount).append(" ") + .append(rowClause).append(" "); + return optimizeString.toString(); + } + + //override the DBDictionary toSelect to call getOptimizeClause and append + //to the select string + public SQLBuffer toSelect(SQLBuffer selects, JDBCFetchConfiguration fetch, + SQLBuffer from, SQLBuffer where, SQLBuffer group, + SQLBuffer having, SQLBuffer order, + boolean distinct, boolean forUpdate, long start, long end, + int expectedResultCount) { + + String optimizeString = null; + SQLBuffer selString = toOperation(getSelectOperation(fetch), + selects, from, where, + group, having, order, distinct, + forUpdate, start, end); + //return toOperation(getSelectOperation(fetch), selects, from, where, + //group, having, order, distinct, forUpdate, start, end); + + if(fetch != null) + optimizeString = getOptimizeClause(fetch, expectedResultCount); + if(optimizeString != null && optimizeString.length() > 0) + selString.append(optimizeString); + + return selString; + + } + //override the DBDictionary toSelect to pass expectedResultcount to the + //other toSelect method + 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); + return toSelect(select, fetch, from, where, sel.getGrouping(), + sel.getHaving(), ordering, sel.isDistinct(), forUpdate, + sel.getStartIndex(), + sel.getEndIndex(),sel.getExpectedResultCount()); + } + } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java index 12d6fdb00..359e42a14 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/LogicalUnion.java @@ -47,13 +47,18 @@ public class LogicalUnion private static final Localizer _loc = Localizer.forPackage (LogicalUnion.class); - + //expected number of results for this select to be used in + // optimize for clause + protected int expectedResultCount = 0; + //indicate if this is internally generated result count + //or not + protected boolean force = false; protected final UnionSelect[] sels; protected final DBDictionary dict; protected final ClassMapping[] mappings; protected final BitSet desc = new BitSet(); private boolean _distinct = true; - private boolean _single = false; + /** * Constructor. @@ -102,15 +107,7 @@ public class LogicalUnion public Select[] getSelects() { return sels; } - - public boolean isSingleResult() { - return _single; - } - - public void setSingleResult(boolean single) { - _single = single; - } - + public boolean isUnion() { return false; } @@ -215,9 +212,12 @@ public class LogicalUnion return res; } - if (_single) { + if (this.getExpectedResultCount()== 1) { AbstractResult res; for (int i = 0; i < sels.length; i++) { + //for each select set the expected result count to 1 + //and force true indicating that this internally generated value + sels[i].sel.setExpectedResultCount(1,true); res = (AbstractResult) sels[i].execute(store, fetch, lockLevel); res.setBaseMapping(mappings[i]); @@ -305,7 +305,12 @@ public class LogicalUnion protected final int pos; protected int orders = 0; protected List orderIdxs = null; - + // expected number of results for this select to be used in + // optimize for clause + protected int expectedResultCount = 0; + //force indicates it is internally generated result count or not + protected boolean force = false; + public UnionSelect(SelectImpl sel, int pos) { this.sel = sel; this.pos = pos; @@ -838,6 +843,18 @@ public class LogicalUnion public String toString() { return sel.toString(); } + + public int getExpectedResultCount() { + return expectedResultCount; + } + + public void setExpectedResultCount(int expectedResultCount, boolean force) { + this.expectedResultCount = expectedResultCount; + this.force = force; + } + public boolean isExpRsltCntForced() { + return force; + } } /** @@ -918,4 +935,16 @@ public class LogicalUnion return a1.length - a2.length; } } + + public int getExpectedResultCount() { + return expectedResultCount; + } + + public void setExpectedResultCount(int expectedResultCount,boolean force) { + this.expectedResultCount = expectedResultCount; + this.force = force; + } + public boolean isExpRsltCntForced() { + return force; + } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java index 1d6d36149..3a7994c8e 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectExecutor.java @@ -119,4 +119,23 @@ public interface SelectExecutor { public Result execute(JDBCStore store, JDBCFetchConfiguration fetch, int lockLevel) throws SQLException; + + /** + * Return the expected result count for the query + */ + public int getExpectedResultCount() ; + + /** + * Set the expected result count for the query + * force indicates whether the count is internally generated + * or given by the user as optimize hint + */ + + public void setExpectedResultCount(int expectedResultCount,boolean force) ; + + /** + * Indicates whether the expectedResultCount is internally generated + */ + + public boolean isExpRsltCntForced(); } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java index 55aa62cbf..df73b6365 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java @@ -153,7 +153,14 @@ public class SelectImpl // from select if this select selects from a tmp table created by another private SelectImpl _from = null; private SelectImpl _outer = null; - + + //expected number of results for this select to be used in + // optimize for clause + private int expectedResultCount = 0; + //true if the expectedResultCount is internally set false if + //it is set by the user + private boolean force = false; + /** * Helper method to return the proper table alias for the given alias index. */ @@ -304,6 +311,20 @@ public class SelectImpl JDBCFetchConfiguration fetch, int lockLevel) throws SQLException { boolean forUpdate = false; + + //expectedResultCount = 1 and force means that it is internally generated value + //for getSingleResult,single valued relationship.We need to check if + //there are any eager joins in the select if there are then the + //optimize for 1 row clause is not generated else we do. if + //!force then it is set by the user through hint and we + //do not check the eager joins + if(this.expectedResultCount == 1 && force ){ + if(this.hasEagerJoin(true)) + this.setExpectedResultCount(0,false); + else + this.setExpectedResultCount(1,false); + } + if (!isAggregate() && _grouping == null) { JDBCLockManager lm = store.getLockManager(); if (lm != null) @@ -2799,6 +2820,19 @@ public class SelectImpl _idents = null; } } + + public int getExpectedResultCount() { + return expectedResultCount; + } + + public void setExpectedResultCount(int expectedResultCount, boolean force) { + this.expectedResultCount = expectedResultCount; + this.force = force; + } + + public boolean isExpRsltCntForced() { + return force; + } } /** diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Union.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Union.java index c5c333e33..755082fdf 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Union.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/Union.java @@ -33,20 +33,6 @@ public interface Union */ public String getOrdering(); - /** - * Whether this union will return at most a single result. Setting this - * flag makes it more efficient to execute logical unions that are actually - * made up from multiple selects executed in batch. - */ - public boolean isSingleResult(); - - /** - * Whether this union will return at most a single result. Setting this - * flag makes it more efficient to execute logical unions that are actually - * made up from multiple selects executed in batch. - */ - public void setSingleResult(boolean single); - /** * Whether this is a true UNION, rather than a logical combination of * independent selects. diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java index 835c43709..4b3617d79 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractStoreQuery.java @@ -33,6 +33,7 @@ public abstract class AbstractStoreQuery implements StoreQuery { protected QueryContext ctx = null; + public static final String optimizeHint ="openjpa.hint.OptimizeResultCount"; public QueryContext getContext() { return ctx; diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java index d33036b47..d2a94f8f8 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/QueryImpl.java @@ -1289,7 +1289,11 @@ public class QueryImpl // Collections.singletonList is JDK 1.3, so... return Arrays.asList(new Object[]{ single }); } - + + if(single == null) + throw new InvalidStateException(_loc.get("is-null", + _class, _query)); + // return single result return single; } finally { diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties index 8e9058b64..6e6fab82d 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties @@ -259,6 +259,9 @@ executing-query-with-params: Executing query: [{0}] with parameters: {1} not-unique: The query on candidate type "{0}" with filter "{1}" was \ configured to have a unique result, but more than one instance matched \ the query. +is-null: The query on candidate type "{0}" with filter "{1}" was \ + configured to have a unique result, but no instance matched \ + the query. serialized: Queries that have been serialized do not support this operation. read-only: Attempt to modify a read-only query object. no-class: A candidate Class must be specified before executing a query. diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java index 5dd37a89f..883c5be37 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java @@ -279,24 +279,15 @@ public class QueryImpl */ public Object getSingleResult() { _em.assertNotCloseInvoked(); + //Indicate that this query returns single result.Later copied into + //select.expectedResultCount + _query.setUnique(true); + try{ Object ob = execute(); - if (!(ob instanceof List)) - return ob; + return ob; - List res = (List) ob; - try { - // don't use size() b/c can be inefficient under some LRS settings - Iterator itr = res.iterator(); - if (!itr.hasNext()) - throw new NoResultException(_loc.get("no-results", - _query.getQueryString()).getMessage(), null, null, false); - Object ret = itr.next(); - if (itr.hasNext()) - throw new NonUniqueResultException(_loc.get("mult-results", - _query.getQueryString()).getMessage(), null, null, false); - return ret; } finally { - OpenJPAPersistence.close(res); + _query.setUnique(false); } } @@ -375,13 +366,29 @@ public class QueryImpl } else if (k.startsWith("FetchPlan.")) { k = k.substring("FetchPlan.".length()); Filters.hintToSetter(getFetchPlan(), k, value); - } else if (k.startsWith("hint.")) + } else if (k.startsWith("hint.")){ + if("hint.OptimizeResultCount".equals(k)){ + if((!(value instanceof String)&&!(value instanceof Integer)) + || (value instanceof String &&(Integer.parseInt + ((String)value)< 0))||((value instanceof Integer) + && (((Integer)value).intValue()<0)) ) + throw new ArgumentException(_loc.get + ("bad-hint-value", key), + null, null, false); + if(value instanceof String) + value = new Integer((String)value); + } _query.getFetchConfiguration().setHint(key, value); + } else throw new ArgumentException(_loc.get("bad-query-hint", key), null, null, false); return this; - } catch (Exception e) { + }catch(NumberFormatException e1){ + throw new ArgumentException(_loc.get("bad-hint-value", key), + null, null, false); + } + catch (Exception e) { throw PersistenceExceptions.toPersistenceException(e); } }