You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by mi...@apache.org on 2010/07/21 18:42:02 UTC

svn commit: r966307 - in /openjpa/branches/2.0.x: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/ openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/ openjpa-persistence-jdb...

Author: mikedd
Date: Wed Jul 21 16:42:02 2010
New Revision: 966307

URL: http://svn.apache.org/viewvc?rev=966307&view=rev
Log:
OPENJPA-1719: Prepared SQL cache user parameter ordering problem with subqueries.
Originally fixed in trunk by Catalina Wei

Modified:
    openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java
    openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CollectionParam.java
    openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java
    openjpa/branches/2.0.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java

Modified: openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java
URL: http://svn.apache.org/viewvc/openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java?rev=966307&r1=966306&r2=966307&view=diff
==============================================================================
--- openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java (original)
+++ openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java Wed Jul 21 16:42:02 2010
@@ -42,6 +42,7 @@ import org.apache.openjpa.kernel.QueryIm
 import org.apache.openjpa.kernel.QueryLanguages;
 import org.apache.openjpa.kernel.StoreQuery;
 import org.apache.openjpa.kernel.PreparedQueryCache.Exclusion;
+import org.apache.openjpa.kernel.exps.Parameter;
 import org.apache.openjpa.kernel.exps.QueryExpressions;
 import org.apache.openjpa.lib.rop.RangeResultObjectProvider;
 import org.apache.openjpa.lib.rop.ResultList;
@@ -406,7 +407,7 @@ public class PreparedQueryImpl implement
     void setUserParameterPositions(List list) {
         _userParamPositions = new HashMap<Object, int[]>();
         for (int i = 1; list != null && i < list.size(); i += 2) {
-            Object key = list.get(i);
+            Object key = ((Parameter)list.get(i)).getParameterKey();
             int p = (Integer)list.get(i-1);
             int[] positions = _userParamPositions.get(key);
             if (positions == null) {

Modified: openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CollectionParam.java
URL: http://svn.apache.org/viewvc/openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CollectionParam.java?rev=966307&r1=966306&r2=966307&view=diff
==============================================================================
--- openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CollectionParam.java (original)
+++ openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/CollectionParam.java Wed Jul 21 16:42:02 2010
@@ -56,6 +56,13 @@ public class CollectionParam
         setImplicitType(type);
     }
 
+    public CollectionParam clone() {
+        CollectionParam c = new CollectionParam(this._key, this._type);
+        c._idx = this._idx;
+        c._container = this._container;
+        return c;
+    }
+
     public Object getParameterKey() {
         return _key;
     }

Modified: openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java
URL: http://svn.apache.org/viewvc/openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java?rev=966307&r1=966306&r2=966307&view=diff
==============================================================================
--- openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java (original)
+++ openjpa/branches/2.0.x/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SQLBuffer.java Wed Jul 21 16:42:02 2010
@@ -32,6 +32,7 @@ import java.util.List;
 import org.apache.commons.lang.ObjectUtils;
 import org.apache.openjpa.jdbc.identifier.DBIdentifier;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
+import org.apache.openjpa.jdbc.kernel.exps.CollectionParam;
 import org.apache.openjpa.jdbc.kernel.exps.Val;
 import org.apache.openjpa.jdbc.schema.Column;
 import org.apache.openjpa.jdbc.schema.Sequence;
@@ -64,8 +65,9 @@ public final class SQLBuffer
     private List _cols = null;
     
     // Even element refers to an index of the _params list
-    // Odd element refers to the user parameter key
+    // Odd element refers to the user parameter
     private List _userIndex = null;
+    private List _userParams = null;
     
     /**
      * Default constructor.
@@ -142,6 +144,16 @@ public final class SQLBuffer
 
             if (paramIndex == _params.size()) {
                 _params.addAll(buf._params);
+                if (buf._userParams != null) {
+                    if (_userParams == null)
+                        _userParams = new ArrayList();
+                   _userParams.addAll(paramIndex, buf._userParams);
+                }
+                if (buf._userIndex != null) {
+                    if (_userIndex == null)
+                        _userIndex = new ArrayList();
+                    _userIndex.addAll(buf._userIndex);
+                }
                 if (buf._cols != null)
                     _cols.addAll(buf._cols);
                 else if (_cols != null)
@@ -149,6 +161,18 @@ public final class SQLBuffer
                         _cols.add(null);
             } else {
                 _params.addAll(paramIndex, buf._params);
+                if ( buf._userParams != null) {
+                    if (_userParams == null)
+                        _userParams = new ArrayList();
+                    _userParams.addAll(paramIndex, buf._userParams);
+                }
+                 if (buf._userIndex != null) {
+                     if (_userIndex == null) {
+                         _userIndex = new ArrayList();
+                         _userIndex.addAll(buf._userIndex);
+                     } else
+                         _userIndex.addAll(paramIndex*2, buf._userIndex);
+                 }
                 if (buf._cols != null)
                     _cols.addAll(paramIndex, buf._cols);
                 else if (_cols != null)
@@ -156,30 +180,12 @@ public final class SQLBuffer
                         _cols.add(paramIndex, null);
             }
         }
-        
-        // adding user parameters from another buffer to this buffer
-        // this buffer's user parameter index gets modified
-        if (buf._userIndex == null && this._userIndex == null) {
-            // do nothing
-        } else if (buf._userIndex != null && this._userIndex == null) {
-            // copy the other buffers data
-            this._userIndex = new ArrayList(buf._userIndex);
-        } else if (buf._userIndex == null && this._userIndex != null) {
-            // nothing to add from the other buffer
-        } else { // both has data. 
-            // modify this buffer's user parameter index
-            int otherSize = buf._userIndex.size()/2;
+
+        if (_userIndex != null) {
+            // fix up user parameter index
             for (int i = 0; i < _userIndex.size(); i+=2) {
-                int newIndex = ((Integer)_userIndex.get(i)).intValue() + otherSize;
-                _userIndex.set(i, newIndex);
+                _userIndex.set(i, _userParams.indexOf(_userIndex.get(i+1)));
             }
-            // append the other buffer's user parameters to this one
-            for (int i = 0; i < buf._userIndex.size(); i+=2) {
-                Object otherIndex = buf._userIndex.get(i);
-                Object otherParam = buf._userIndex.get(i+1);
-                _userIndex.add(otherIndex);
-                _userIndex.add(otherParam);
-            }            
         }
     }
     
@@ -293,6 +299,8 @@ public final class SQLBuffer
             // we get the first non-null col
             if (_params == null)
                 _params = new ArrayList();
+            if (_userParams == null)
+                _userParams = new ArrayList();
             if (col != null && _cols == null) {
                 _cols = new ArrayList();
                 while (_cols.size() < _params.size())
@@ -301,12 +309,18 @@ public final class SQLBuffer
 
             _params.add(o);
             if (userParam != null) {
+                Object param = userParam;
+                if (userParam instanceof CollectionParam)
+                    param = ((CollectionParam) userParam).clone();
+                _userParams.add(param);
                 if (_userIndex == null)
                     _userIndex = new ArrayList();
                 int index = _params.size()-1;
                 _userIndex.add(index);
-                _userIndex.add(userParam.getParameterKey());
+                _userIndex.add(param);
             }
+            else
+                _userParams.add(o);
             if (_cols != null)
                 _cols.add(col);
         }

Modified: openjpa/branches/2.0.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java
URL: http://svn.apache.org/viewvc/openjpa/branches/2.0.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java?rev=966307&r1=966306&r2=966307&view=diff
==============================================================================
--- openjpa/branches/2.0.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java (original)
+++ openjpa/branches/2.0.x/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/TestPreparedQueryCache.java Wed Jul 21 16:42:02 2010
@@ -213,7 +213,38 @@ public class TestPreparedQueryCache exte
 	        em.close();
 		super.tearDown();
 	}
-	
+    
+    public void testRepeatedParameterInSubqueryInDifferentOrderSubQLast() {
+        OpenJPAEntityManager em = emf.createEntityManager();
+       
+        String jpql = "SELECT o from OrderJPA o " +
+                "WHERE (o.CustomerId = :customerId) " +
+                "AND (o.WarehouseId = :warehouseId) " +
+                "AND (o.DistrictId = :districtId) " +
+                "AND o.OrderId IN (SELECT MAX (o1.OrderId) from OrderJPA o1 " +
+                    "WHERE ((o1.CustomerId = :customerId) " +
+                    "AND    (o1.DistrictId = :districtId) " +
+                    "AND    (o1.WarehouseId = :warehouseId)))";
+        
+        em.getTransaction().begin();
+        TypedQuery<OrderJPA> q1 = em.createQuery(jpql, OrderJPA.class);
+        q1.setParameter("customerId", 339)
+          .setParameter("districtId", 3)
+          .setParameter("warehouseId", 23);
+                  
+        assertEquals(JPQLParser.LANG_JPQL, OpenJPAPersistence.cast(q1).getLanguage());
+        assertFalse(q1.getResultList().isEmpty());        
+        
+        TypedQuery<OrderJPA> q2 = em.createQuery(jpql, OrderJPA.class);
+        assertEquals(QueryLanguages.LANG_PREPARED_SQL, OpenJPAPersistence.cast(q2).getLanguage());
+        q2.setParameter("customerId", 2967)
+          .setParameter("districtId", 5)
+          .setParameter("warehouseId", 22);
+        
+        assertFalse(q2.getResultList().isEmpty());
+        em.getTransaction().rollback();
+        
+    }
 
 	public void testPreparedQueryCacheIsActiveByDefault() {
 		assertNotNull(getPreparedQueryCache());