OPENJPA-1845: the prepared query cache doesn't currently work correclty with 'SELECT IN' statements

git-svn-id: https://svn.apache.org/repos/asf/openjpa/branches/2.0.x@1341547 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryCacheImpl.java

@@ -178,8 +178,8 @@ public class PreparedQueryCacheImpl implements PreparedQueryCache {
 		lock();
 		try {
 			if (_uncachables.put(id, exclusion) == null) {
-			    if (_log != null && _log.isTraceEnabled()) 
-			        _log.trace(_loc.get("prepared-query-uncache", id, exclusion));
+			    if (_log != null && _log.isWarnEnabled()) 
+			        _log.warn(_loc.get("prepared-query-uncache", id, exclusion));
 			}
 			return _delegate.remove(id);
 		} finally {

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryImpl.java

@@ -214,7 +214,10 @@ public class PreparedQueryImpl implements PreparedQuery {
             return new Object[]{null, _loc.get("exclude-not-executor", _id)};
         _exps = ((StoreQuery.Executor)executor).getQueryExpressions();
         for (int i = 0; i < _exps.length; i++) {
-            if (isUsingExternalizedParameter(_exps[i])) {
+            QueryExpressions exp = _exps[i];
+            if (exp.hasInExpression)
+                return new Object[]{null, _loc.get("exclude-in-expression", _id)};
+            if (isUsingExternalizedParameter(exp)) {
                 return new Object[]{null, _loc.get("exclude-externalized-param", _id)};
             }
         }

openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/localizer.properties

@@ -167,7 +167,9 @@ exclude-not-executor: Query "{0}" is not cached because it was not executed on a
     data store.    
 exclude-externalized-param: Query "{0}" is not cached because some parameterized \
     field values are externalized.  
-exclude-user-strategy: This query "{0}" is not cached because some parameterized \
+exclude-user-strategy: Query "{0}" is not cached because some parameterized \
     field value depends on user-defined field strategy.        
-exclude-pagination: This query "{0}" involves pagination and is not cached.
+exclude-pagination: Query "{0}" is not cached because it uses pagination.
+exclude-in-expression: Query "{0}" is not cached because it uses IN expression with \
+    variable-length parameter.
       

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java

@@ -78,6 +78,7 @@ public class QueryExpressions
     private Stack<Context> _contexts = null;
     public Object state;
     public ResultShape<?> shape;
+    public boolean hasInExpression;
     
     /**
      * Set reference to the JPQL query contexts.

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java

@@ -92,6 +92,7 @@ public class JPQLExpressionBuilder
     private OrderedMap<Object, Class<?>> parameterTypes;
     private int aliasCount = 0;
     private boolean inAssignSubselectProjection = false;
+    private boolean hasParameterizedInExpression = false;
 
     /**
      * Constructor.
@@ -309,7 +310,8 @@ public class JPQLExpressionBuilder
             exps.parameterTypes = parameterTypes;
 
         exps.accessPath = getAccessPath();
-
+        exps.hasInExpression = this.hasParameterizedInExpression;
+        
         return exps;
     }
 
@@ -1135,6 +1137,10 @@ public class JPQLExpressionBuilder
                     else
                         val2 = getValue(next);
 
+                    if (val2 instanceof Parameter) {
+                        hasParameterizedInExpression = true;
+                    }
+                    
                     // special case for <value> IN (<subquery>) or
                     // <value> IN (<single value>)
                     if (!(val2 instanceof Literal) && node.getChildCount() == 2)

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntry.java

@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+
+package org.apache.openjpa.persistence.jdbc.query.cache;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+
+@Entity
+public class CatalogEntry {
+	@Id
+	private long catalogEntryId;
+	
+	private String partNumber;
+	
+	private int quantity = 50;
+	
+	@OneToMany(targetEntity=CatalogEntryDescription.class, 
+			mappedBy="CatalogEntryForCatalogEntryDescription", cascade=CascadeType.MERGE)
+	private List CatalogEntryDescription = new ArrayList();
+
+	public long getCatalogEntryId() {
+		return catalogEntryId;
+	}
+
+	public void setCatalogEntryId(long catalogEntryId) {
+		this.catalogEntryId = catalogEntryId;
+	}
+
+	public String getPartNumber() {
+		return partNumber;
+	}
+
+	public void setPartNumber(String partNumber) {
+		this.partNumber = partNumber;
+	}
+	
+	public List getCatalogEntryDescription() {
+		return CatalogEntryDescription;
+	}
+
+	public void setCatalogEntryDescription(List catalogEntryDescription) {
+		CatalogEntryDescription = catalogEntryDescription;
+	}
+
+	public CatalogEntry() {
+	}
+
+	/**
+	 * @param catalogEntryId
+	 * @param partNumber
+	 */
+	public CatalogEntry(long catalogEntryId, String partNumber) {
+		super();
+		this.catalogEntryId = catalogEntryId;
+		this.partNumber = partNumber;
+	}
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntryDescription.java

@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+
+package org.apache.openjpa.persistence.jdbc.query.cache;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+
+@Entity
+public class CatalogEntryDescription {
+	@ManyToOne(fetch=FetchType.LAZY, cascade=CascadeType.MERGE)
+	@JoinColumn(name="catalogEntryId")
+	private CatalogEntry CatalogEntryForCatalogEntryDescription;
+
+	@Id
+	private long catalogEntryId;
+	
+	private String description;
+	
+	public CatalogEntryDescription() {
+	}
+
+	/**
+	 * @param catalogEntryId
+	 * @param description
+	 */
+	public CatalogEntryDescription(long catalogEntryId, String description) {
+		super();
+		this.catalogEntryId = catalogEntryId;
+		this.description = description;
+	}
+
+	public CatalogEntry getCatalogEntryForCatalogEntryDescription() {
+		return CatalogEntryForCatalogEntryDescription;
+	}
+
+	public void setCatalogEntryForCatalogEntryDescription(
+			CatalogEntry catalogEntryForCatalogEntryDescription) {
+		CatalogEntryForCatalogEntryDescription = catalogEntryForCatalogEntryDescription;
+	}
+
+	public long getCatalogEntryId() {
+		return catalogEntryId;
+	}
+
+	public void setCatalogEntryId(long catalogEntryId) {
+		this.catalogEntryId = catalogEntryId;
+	}
+
+	public String getDescription() {
+		return description;
+	}
+
+	public void setDescription(String description) {
+		this.description = description;
+	}
+}
+

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestCase.java

@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+
+package org.apache.openjpa.persistence.jdbc.query.cache;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.persistence.EntityManager;
+import javax.persistence.Query;
+
+import org.apache.openjpa.persistence.test.SQLListenerTestCase;
+
+public class TestCase extends SQLListenerTestCase {
+
+	private static EntityManager em;
+	
+	public void setUp() {
+        setUp(CatalogEntry.class, CatalogEntryDescription.class, CLEAR_TABLES);
+        em = emf.createEntityManager();
+        boolean isDataPoulated =  em.createQuery("select count(c) from CatalogEntry c", Long.class)
+        		.getSingleResult().longValue() > 0;
+        if(!isDataPoulated){
+            em.getTransaction().begin();
+            CatalogEntry catalogEntry1 = new CatalogEntry(10001,"SKU10001");
+            CatalogEntry catalogEntry2 = new CatalogEntry(10002,"SKU10002");
+            CatalogEntry catalogEntry3 = new CatalogEntry(10003,"SKU10003");
+            CatalogEntry catalogEntry4 = new CatalogEntry(10004,"SKU10004");
+            em.persist(catalogEntry1);
+            em.persist(catalogEntry2);
+            em.persist(catalogEntry3);
+            em.persist(catalogEntry4);
+            CatalogEntryDescription catalogEntryDescription1 = 
+            		new CatalogEntryDescription(10001,"Description for SKU10001");
+            CatalogEntryDescription catalogEntryDescription2 = 
+            		new CatalogEntryDescription(10002,"Description for SKU10002");
+            CatalogEntryDescription catalogEntryDescription3 = 
+            		new CatalogEntryDescription(10003,"Description for SKU10003");
+            CatalogEntryDescription catalogEntryDescription4 = 
+            		new CatalogEntryDescription(10004,"Description for SKU10004");
+            em.persist(catalogEntryDescription1);
+            em.persist(catalogEntryDescription2);
+            em.persist(catalogEntryDescription3);
+            em.persist(catalogEntryDescription4);
+    		em.getTransaction().commit();
+        }
+	}
+	
+	private void printCatentries(List<CatalogEntry> catalogEntries){
+		if(catalogEntries != null && !catalogEntries.isEmpty()){
+			for (CatalogEntry catalogEntry : catalogEntries) {
+				System.out.println("catalogEntryId: " + catalogEntry.getCatalogEntryId() 
+						+ ", partNumber: " + catalogEntry.getPartNumber());
+			}
+		}
+	}
+	
+	private void printDescriptions(List<CatalogEntryDescription> descriptions){
+		if(descriptions != null && !descriptions.isEmpty()){
+			for (CatalogEntryDescription description : descriptions) {
+				System.out.println("catalogEntryId: " + description.getCatalogEntryId() 
+						+ ", description: " + description.getDescription());
+			}
+		}
+	}
+	
+	public void testCase1(){// IN clause with primitive types in the list
+		{
+			System.out.println("<First execution - works fine>");
+			Query jpql = em.createQuery("SELECT catentry FROM CatalogEntry catentry " +
+					"WHERE catentry.catalogEntryId IN(:catalogEntryId1) " +
+					"and catentry.catalogEntryId IN(:catalogEntryId2) " +
+					"and catentry.partNumber IN(:catalogPartNum) " +
+					"and catentry.quantity = (:quantity) " +
+					"and catentry.catalogEntryId IN(:catalogEntryId3)");
+			List<Long> catalogEntryIds = new ArrayList<Long>();
+			catalogEntryIds.add(new Long(10001));
+			catalogEntryIds.add(new Long(10002));
+			catalogEntryIds.add(new Long(10003));
+			List<String> catalogPartNum = new ArrayList<String>();
+			catalogPartNum.add("SKU10001");
+			catalogPartNum.add("SKU10002");
+			jpql.setParameter("catalogEntryId1", catalogEntryIds);
+			jpql.setParameter("catalogEntryId2", catalogEntryIds);
+			jpql.setParameter("catalogEntryId3", catalogEntryIds);
+			jpql.setParameter("catalogPartNum", catalogPartNum);
+			jpql.setParameter("quantity", 50);
+			List<CatalogEntry> catalogEntries = jpql.getResultList();
+			printCatentries(catalogEntries);
+			assertEquals(2, catalogEntries.size());
+		}
+		// at this point, the prepared statements are assumed to be cached, that is the default behaviour.
+
+		{	
+			System.out.println("<Subsequent execution based on query cache>");
+			Query jpql = em.createQuery("SELECT catentry FROM CatalogEntry catentry " +
+					"WHERE catentry.catalogEntryId IN(:catalogEntryId1) " +
+					"and catentry.catalogEntryId IN(:catalogEntryId2) " +
+					"and catentry.partNumber IN(:catalogPartNum) " +
+					"and catentry.quantity = (:quantity) " +
+					"and catentry.catalogEntryId IN(:catalogEntryId3)");
+			List<Long> catalogEntryIds = new ArrayList<Long>();
+			catalogEntryIds.add(new Long(10001));
+			catalogEntryIds.add(new Long(10002));
+			catalogEntryIds.add(new Long(10003));
+			List<String> catalogPartNum = new ArrayList<String>();
+			catalogPartNum.add("SKU10001");
+			catalogPartNum.add("SKU10002");
+			jpql.setParameter("catalogEntryId1", catalogEntryIds);
+			jpql.setParameter("catalogEntryId2", catalogEntryIds);
+			jpql.setParameter("catalogEntryId3", catalogEntryIds);
+			jpql.setParameter("catalogPartNum", catalogPartNum);
+			jpql.setParameter("quantity", 50);
+			List<CatalogEntry> catalogEntries = jpql.getResultList();
+			printCatentries(catalogEntries);
+			assertEquals(2, catalogEntries.size());
+		}
+	}
+	
+	public void testCase2(){// IN clause with entities in the list
+		List<CatalogEntry> catalogEntries = em.createQuery("SELECT catentry FROM CatalogEntry catentry " +
+				"WHERE catentry.catalogEntryId IN(10001,10002)").getResultList();
+		{
+			System.out.println("<First execution - works fine>");
+			Query jpql = em.createQuery("SELECT catentdesc FROM CatalogEntryDescription catentdesc " +
+					"WHERE catentdesc.CatalogEntryForCatalogEntryDescription IN(:catalogEntry)");
+			jpql.setParameter("catalogEntry", catalogEntries);
+			printDescriptions(jpql.getResultList());
+			System.out.println("</First execution - works fine>");
+			System.out.println();
+		}
+		// at this point, the prepared statements are assumed to be cached, ast that is the default behaviour.
+
+		{	
+			System.out.println("<Subsequent execution - throws exception>");
+			Query jpql = em.createQuery("SELECT catentdesc FROM CatalogEntryDescription catentdesc " +
+					"WHERE catentdesc.CatalogEntryForCatalogEntryDescription IN(:catalogEntry)");
+			try{
+				jpql.setParameter("catalogEntry", catalogEntries);
+				printDescriptions(jpql.getResultList());
+			}catch (Exception e) {
+				e.printStackTrace();
+			}
+			System.out.println("</Subsequent execution - throws exception>");
+			System.out.println();
+		}
+
+	}
+}
+