From 64329e007eb2ba4329d3f140f138e3c6a075b1c2 Mon Sep 17 00:00:00 2001 From: Pinaki Poddar Date: Mon, 2 Feb 2009 15:57:27 +0000 Subject: [PATCH] OPENJPA-703: Support Collection-valued parameters. Handle re-parameterization when collection-valued parameter has different size across invocations. git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@740016 13f79535-47bb-0310-9956-ffa450edef68 --- .../jdbc/kernel/PreparedQueryImpl.java | 77 +++++++++++---- .../jdbc/kernel/exps/InExpression.java | 4 +- .../openjpa/jdbc/kernel/localizer.properties | 11 ++- .../kernel/jpql/JPQLExpressionBuilder.java | 74 ++++---------- .../jdbc/sqlcache/TestPreparedQueryCache.java | 28 +++++- .../apache/openjpa/persistence/QueryImpl.java | 96 ++++++++++++++----- 6 files changed, 189 insertions(+), 101 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java index 5b4f56e20..23c075e20 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java @@ -19,7 +19,9 @@ package org.apache.openjpa.jdbc.kernel; -import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -35,8 +37,9 @@ import org.apache.openjpa.kernel.Query; import org.apache.openjpa.kernel.QueryImpl; import org.apache.openjpa.kernel.QueryLanguages; import org.apache.openjpa.lib.rop.ResultList; +import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.util.ImplHelper; -import org.apache.openjpa.util.InternalException; +import org.apache.openjpa.util.UserException; /** * Implements {@link PreparedQuery} for SQL queries. @@ -45,6 +48,9 @@ import org.apache.openjpa.util.InternalException; * */ public class PreparedQueryImpl implements PreparedQuery { + private static Localizer _loc = + Localizer.forPackage(PreparedQueryImpl.class); + private final String _id; private String _sql; @@ -53,11 +59,9 @@ public class PreparedQueryImpl implements PreparedQuery { private boolean _subclasses; private boolean _isProjection; - // Parameters of the query - private List _params; // Position of the user defined parameters in the _params list private Map _userParamPositions; - + private Map _template; /** * Construct. @@ -163,31 +167,43 @@ public class PreparedQueryImpl implements PreparedQuery { * {@link #initialize(Object) initialization}. * * @return 0-based parameter index mapped to corresponding values. + * */ public Map reparametrize(Map user, Broker broker) { - Map result = new HashMap(); - for (int i = 0; i < _params.size(); i++) { - result.put(i, _params.get(i)); + if (user == null || user.isEmpty()) { + if (!_userParamPositions.isEmpty()) { + throw new UserException(_loc.get("uparam-null", + _userParamPositions.keySet(), this)); + } else { + return _template; + } } - if (user == null || user.isEmpty()) - return result; + if (!_userParamPositions.keySet().equals(user.keySet())) { + throw new UserException(_loc.get("uparam-mismatch", + _userParamPositions.keySet(), user.keySet(), this)); + } + Map result = new HashMap(_template); + for (Object key : user.keySet()) { int[] indices = _userParamPositions.get(key); - if (indices == null) - continue; - Object value = user.get(key); - if (ImplHelper.isManageable(value)) { - setPersistenceCapableParameter(result, value, indices, broker); + if (indices == null || indices.length == 0) + throw new UserException(_loc.get("uparam-no-pos", key, this)); + Object val = user.get(key); + if (ImplHelper.isManageable(val)) { + setPersistenceCapableParameter(result, val, indices, broker); + } else if (val instanceof Collection) { + setCollectionValuedParameter(result, (Collection)val, indices, + key); } else { for (int j : indices) - result.put(j, value); + result.put(j, val); } } return result; } /** - * Calculate primary key identity value(s) of the given managable instance + * Calculate primary key identity value(s) of the given manageable instance * and fill in the given map. * * @param values a map of integer parameter index to parameter value @@ -209,7 +225,8 @@ public class PreparedQueryImpl implements PreparedQuery { Object[] array = (Object[])cols; int n = array.length; if (n > indices.length || indices.length%n != 0) - throw new InternalException(); + throw new UserException(_loc.get("uparam-pc-key", + pc.getClass(), n, Arrays.toString(indices))); int k = 0; for (int j : indices) { result.put(j, array[k%n]); @@ -222,8 +239,23 @@ public class PreparedQueryImpl implements PreparedQuery { } } + private void setCollectionValuedParameter(Map result, + Collection values, int[] indices, Object param) { + int n = values.size(); + Object[] array = values.toArray(); + if (n > indices.length || indices.length%n != 0) { + throw new UserException(_loc.get("uparam-coll-size", param, values, + Arrays.toString(indices))); + } + int k = 0; + for (int j : indices) { + result.put(j, array[k%n]); + k++; + } + + } /** - * Marks the positions of user parameters. + * Marks the positions and keys of user parameters. * * @param list even elements are numbers representing the position of a * user parameter in the _param list. Odd elements are the user parameter @@ -248,8 +280,11 @@ public class PreparedQueryImpl implements PreparedQuery { } void setParameters(List list) { - _params = new ArrayList(); - _params.addAll(list); + Map tmp = new HashMap(); + for (int i = 0; list != null && i < list.size(); i++) { + tmp.put(i, list.get(i)); + } + _template = Collections.unmodifiableMap(tmp); } public String toString() { diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InExpression.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InExpression.java index d662df604..d99e518b8 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InExpression.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/InExpression.java @@ -30,6 +30,7 @@ import org.apache.openjpa.jdbc.sql.Joins; import org.apache.openjpa.jdbc.sql.SQLBuffer; import org.apache.openjpa.jdbc.sql.Select; import org.apache.openjpa.kernel.exps.ExpressionVisitor; +import org.apache.openjpa.kernel.exps.Parameter; /** * Tests whether a value is IN a collection. @@ -154,7 +155,8 @@ class InExpression Column col = (cols != null && cols.length == 1) ? cols[0] : null; for (Iterator itr = coll.iterator(); itr.hasNext();) { - buf.appendValue(itr.next(), col); + buf.appendValue(itr.next(), col, _const instanceof Parameter + ? (Parameter)_const : null); if (itr.hasNext()) buf.append(", "); } diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties index 55584b0f8..d01f3e93c 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties @@ -124,4 +124,13 @@ prepared-query-add-pattern: Adding a Query exclusion pattern "{0}" has caused \ following {1} cached queries to be removed from the cache: "{2}". prepared-query-remove-pattern: Removing a Query exclusion pattern "{0}" caused \ following {1} queries to be re-inserted in the cache: "{2}". - \ No newline at end of file +uparam-mismatch: Supplied user parameters "{1}" do not match expected \ + parameters "{0}" for the prepared query "{2}". +uparam-null: No user parameter was given. Expected parameters "{0}" for the \ + prepared query "{1}". +uparam-coll-size: Parameter "{0}" has a value "{1}" which is not compatible \ + with the available positions {2} in the parameter list of the prepared query +uparam-no-pos: User parameter "{0}" does not appear in any position in the \ + prepared query "{1}". +uparam-pc-key: Class "{0}" uses {1} primary key columns but corresponding \ + positions {2} in the parameter list of the prepared query is not compatible. \ No newline at end of file diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java index d52012b00..3eda04cb5 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java @@ -821,13 +821,15 @@ public class JPQLExpressionBuilder return eval(firstChild(node)); case JJTNAMEDINPUTPARAMETER: - return getParameter(node.text, false); + return getParameter(node.text, false, false); case JJTPOSITIONALINPUTPARAMETER: - return getParameter(node.text, true); + return getParameter(node.text, true, false); case JJTCOLLECTIONPARAMETER: - return getCollectionValuedParameter(node); + JPQLNode child = onlyChild(node); + return getParameter(child.text, + child.id == JJTPOSITIONALINPUTPARAMETER, true); case JJTOR: // x OR y return factory.or(getExpression(left(node)), @@ -1288,58 +1290,23 @@ public class JPQLExpressionBuilder } /** - * Record the names and order of implicit parameters. + * Creates and records the names and order of parameters. The parameters are + * identified by a key with its type preserved. The second argument + * determines whether the first argument is used as-is or converted to + * an Integer as parameter key. + * + * @param the text as it appears in the parsed node + * @param positional if true the first argument is converted to an integer + * @param isCollectionValued true for collection-valued parameters */ - private Parameter getParameter(String id, boolean positional) { + private Parameter getParameter(String id, boolean positional, + boolean isCollectionValued) { if (parameterTypes == null) parameterTypes = new LinkedMap(6); Object paramKey = positional ? Integer.parseInt(id) : id; if (!parameterTypes.containsKey(paramKey)) parameterTypes.put(paramKey, TYPE_OBJECT); - Class type = Object.class; - ClassMetaData meta = null; - int index; - - if (positional) { - try { - // indexes in JPQL are 1-based, as opposed to 0-based in - // the core ExpressionFactory - index = Integer.parseInt(id) - 1; - } catch (NumberFormatException e) { - throw parseException(EX_USER, "bad-positional-parameter", - new Object[]{ id }, e); - } - - if (index < 0) - throw parseException(EX_USER, "bad-positional-parameter", - new Object[]{ id }, null); - } else { - // otherwise the index is just the current size of the params - index = parameterTypes.indexOf(id); - } - Parameter param = factory.newParameter(paramKey, type); - param.setMetaData(meta); - param.setIndex(index); - - return param; - } - - /** - * Record the names and order of collection valued input parameters. - */ - private Parameter getCollectionValuedParameter(JPQLNode node) { - JPQLNode child = onlyChild(node); - String id = child.text; - boolean positional = child.id == JJTPOSITIONALINPUTPARAMETER; - - if (parameterTypes == null) - parameterTypes = new LinkedMap(6); - Object paramKey = positional ? Integer.parseInt(id) : id; - if (!parameterTypes.containsKey(id)) - parameterTypes.put(paramKey, TYPE_OBJECT); - - Class type = Object.class; ClassMetaData meta = null; int index; if (positional) { @@ -1359,11 +1326,12 @@ public class JPQLExpressionBuilder // otherwise the index is just the current size of the params index = parameterTypes.indexOf(id); } - - Parameter param = factory.newCollectionValuedParameter(id, type); + Parameter param = isCollectionValued + ? factory.newCollectionValuedParameter(paramKey, TYPE_OBJECT) + : factory.newParameter(paramKey, TYPE_OBJECT); param.setMetaData(meta); param.setIndex(index); - + return param; } @@ -1502,10 +1470,10 @@ public class JPQLExpressionBuilder return factory.type(getValue(node)); case JJTNAMEDINPUTPARAMETER: - return factory.type(getParameter(node.text, false)); + return factory.type(getParameter(node.text, false, false)); case JJTPOSITIONALINPUTPARAMETER: - return factory.type(getParameter(node.text, true)); + return factory.type(getParameter(node.text, true, false)); default: // TODO: enforce jpa2.0 spec rules. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java index d80c61b04..92840eb93 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java @@ -19,6 +19,7 @@ package org.apache.openjpa.persistence.jdbc.sqlcache; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -424,6 +425,25 @@ public class TestPreparedQueryCache extends SQLListenerTestCase { compare(!IS_NAMED_QUERY, jpql, COMPANY_NAMES.length*DEPARTMENT_NAMES.length, params); } + public void testCollectionValuedParameters() { + String jpql = "select e from Employee e where e.name in :names"; + Object[] params1 = {"names", Arrays.asList(new String[]{EMPLOYEE_NAMES[0], EMPLOYEE_NAMES[1]})}; + Object[] params2 = {"names", Arrays.asList(new String[]{EMPLOYEE_NAMES[2]})}; + Object[] params3 = {"names", Arrays.asList(EMPLOYEE_NAMES)}; + + boolean checkHits = false; + + int expectedCount = 2*COMPANY_NAMES.length*DEPARTMENT_NAMES.length; + run(jpql, params1, USE_CACHE, 2, !IS_NAMED_QUERY, expectedCount, checkHits); + assertCached(jpql); + + expectedCount = 1*COMPANY_NAMES.length*DEPARTMENT_NAMES.length; + run(jpql, params2, USE_CACHE, 2, !IS_NAMED_QUERY, expectedCount, checkHits); + + expectedCount = EMPLOYEE_NAMES.length*COMPANY_NAMES.length*DEPARTMENT_NAMES.length; + run(jpql, params3, USE_CACHE, 2, !IS_NAMED_QUERY, expectedCount, checkHits); + } + /** * Compare the result of execution of the same query with and without * Prepared Query Cache. @@ -462,6 +482,10 @@ public class TestPreparedQueryCache extends SQLListenerTestCase { } } + long run(String jpql, Object[] params, boolean useCache, int N, + boolean isNamedQuery, int expectedCount) { + return run(jpql, params, useCache, N, isNamedQuery, expectedCount, true); + } /** * Create and run a query N times with the given parameters. The time for * each query execution is measured in nanosecond precision and @@ -470,7 +494,7 @@ public class TestPreparedQueryCache extends SQLListenerTestCase { * returns median time taken for single execution. */ long run(String jpql, Object[] params, boolean useCache, int N, - boolean isNamedQuery, int expectedCount) { + boolean isNamedQuery, int expectedCount, boolean checkHits) { trace("Executing " + N + " times " + (useCache ? " with " : "without") + " cache"); List stats = new ArrayList(); sql.clear(); @@ -500,7 +524,7 @@ public class TestPreparedQueryCache extends SQLListenerTestCase { stats.add(end - start); em.close(); } - if (useCache) { + if (useCache && checkHits) { String cacheKey = isNamedQuery ? getJPQL(jpql) : jpql; long total = getCache().getStatistics().getExecutionCount(cacheKey); long hits = getCache().getStatistics().getHitCount(cacheKey); 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 66928f2e2..235a395e1 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 @@ -40,6 +40,7 @@ import javax.persistence.NonUniqueResultException; import javax.persistence.Query; import javax.persistence.TemporalType; +import org.apache.openjpa.conf.OpenJPAConfiguration; import org.apache.openjpa.enhance.Reflection; import org.apache.openjpa.kernel.Broker; import org.apache.openjpa.kernel.DelegatingQuery; @@ -55,10 +56,13 @@ import org.apache.openjpa.kernel.QueryStatistics; import org.apache.openjpa.kernel.exps.AggregateListener; import org.apache.openjpa.kernel.exps.FilterListener; import org.apache.openjpa.kernel.jpql.JPQLParser; +import org.apache.openjpa.lib.log.Log; import org.apache.openjpa.lib.rop.ResultList; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.util.ImplHelper; import org.apache.openjpa.util.RuntimeExceptionTranslator; +import org.apache.openjpa.util.UserException; + import static org.apache.openjpa.kernel.QueryLanguages.LANG_PREPARED_SQL; /** @@ -86,7 +90,7 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { * Constructor; supply factory exception translator and delegate. * * @param em The EntityManager which created this query - * @param ret Exception translater for this query + * @param ret Exception translator for this query * @param query The underlying "kernel" query. */ public QueryImpl(EntityManagerImpl em, RuntimeExceptionTranslator ret, @@ -248,33 +252,18 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { _query.compile(); return this; } - + private Object execute() { if (_query.getOperation() != QueryOperations.OP_SELECT) throw new InvalidStateException(_loc.get("not-select-query", _query .getQueryString()), null, null, false); - Map params = _positional != null ? _positional : _named; - Boolean registered = null; - PreparedQueryCache cache = _em.getPreparedQueryCache(); - if (cache != null) { - FetchConfiguration fetch = _query.getFetchConfiguration(); - registered = cache.register(_id, _query, fetch); - boolean alreadyCached = (registered == null); - String lang = _query.getLanguage(); - QueryStatistics stats = cache.getStatistics(); - if (alreadyCached && LANG_PREPARED_SQL.equals(lang)) { - PreparedQuery pq = _em.getPreparedQuery(_id); - params = pq.reparametrize(params, _em.getBroker()); - stats.recordExecution(pq.getOriginalQuery(), alreadyCached); - } else { - stats.recordExecution(_query.getQueryString(), alreadyCached); - } - } + Map params = _positional != null ? _positional + : _named != null ? _named : new HashMap(); + boolean registered = preExecute(params); Object result = _query.execute(params); - - if (registered == Boolean.TRUE) { - cache.initialize(_id, result); + if (registered) { + postExecute(result); } return result; } @@ -636,6 +625,68 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { "JPA 2.0 - Method not yet implemented"); } + // + // Prepared Query Cache related methods + // + + /** + * Invoked before a query is executed. + * If this receiver is cached as a {@linkplain PreparedQuery prepared query} + * then re-parameterizes the given user parameters. The given map is cleared + * and re-parameterized values are filled in. + * + * @param params user supplied parameter key-values. Always supply a + * non-null map even if the user has not specified any parameter, because + * the same map will to be populated by re-parameterization. + * + * @return true if this invocation caused the query being registered in the + * cache. + */ + private boolean preExecute(Map params) { + PreparedQueryCache cache = _em.getPreparedQueryCache(); + if (cache == null) { + return false; + } + FetchConfiguration fetch = _query.getFetchConfiguration(); + Boolean registered = cache.register(_id, _query, fetch); + boolean alreadyCached = (registered == null); + String lang = _query.getLanguage(); + QueryStatistics stats = cache.getStatistics(); + if (alreadyCached && LANG_PREPARED_SQL.equals(lang)) { + PreparedQuery pq = _em.getPreparedQuery(_id); + try { + Map rep = pq.reparametrize(params, _em.getBroker()); + params.clear(); + params.putAll(rep); + } catch (UserException ue) { + invalidatePreparedQuery(); + Log log = _em.getConfiguration().getLog( + OpenJPAConfiguration.LOG_RUNTIME); + if (log.isWarnEnabled()) + log.warn(ue.getMessage()); + return false; + } + stats.recordExecution(pq.getOriginalQuery(), alreadyCached); + } else { + stats.recordExecution(_query.getQueryString(), alreadyCached); + } + return registered == Boolean.TRUE; + } + + /** + * Initialize the registered Prepared Query from the given opaque object. + * + * @param result an opaque object representing execution result of a query + * + * @return true if the prepared query can be initialized. + */ + boolean postExecute(Object result) { + PreparedQueryCache cache = _em.getPreparedQueryCache(); + if (cache == null) { + return false; + } + return cache.initialize(_id, result) != null; + } /** * Remove this query from PreparedQueryCache. @@ -680,5 +731,4 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { _id = id; return this; } - }