From f41f79360b3ae4072492133fdf18b7a950455c84 Mon Sep 17 00:00:00 2001 From: Jody Grassel Date: Tue, 22 May 2012 16:04:14 +0000 Subject: [PATCH] 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 --- .../jdbc/kernel/PreparedQueryCacheImpl.java | 4 +- .../jdbc/kernel/PreparedQueryImpl.java | 5 +- .../openjpa/jdbc/kernel/localizer.properties | 6 +- .../openjpa/kernel/exps/QueryExpressions.java | 1 + .../kernel/jpql/JPQLExpressionBuilder.java | 8 +- .../jdbc/query/cache/CatalogEntry.java | 79 +++++++++ .../query/cache/CatalogEntryDescription.java | 78 ++++++++ .../jdbc/query/cache/TestCase.java | 166 ++++++++++++++++++ 8 files changed, 341 insertions(+), 6 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntry.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntryDescription.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestCase.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryCacheImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryCacheImpl.java index 8799e63f9..92d43e8d8 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PreparedQueryCacheImpl.java +++ b/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 { 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 51ce99f9f..95ddb7b91 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 @@ -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)}; } } 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 5bb99f68a..3e0dd5166 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 @@ -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. \ No newline at end of file diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java index 560c6f9ed..cc46528e7 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java @@ -78,6 +78,7 @@ public class QueryExpressions private Stack _contexts = null; public Object state; public ResultShape shape; + public boolean hasInExpression; /** * Set reference to the JPQL query contexts. 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 1344e5d5f..ae7ebb0d8 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 @@ -92,6 +92,7 @@ public class JPQLExpressionBuilder private OrderedMap> 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 IN () or // IN () if (!(val2 instanceof Literal) && node.getChildCount() == 2) diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntry.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntry.java new file mode 100644 index 000000000..412bb5f6b --- /dev/null +++ b/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; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntryDescription.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/CatalogEntryDescription.java new file mode 100644 index 000000000..ea5fffa56 --- /dev/null +++ b/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; + } +} + diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/query/cache/TestCase.java new file mode 100644 index 000000000..cdb11a183 --- /dev/null +++ b/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 catalogEntries){ + if(catalogEntries != null && !catalogEntries.isEmpty()){ + for (CatalogEntry catalogEntry : catalogEntries) { + System.out.println("catalogEntryId: " + catalogEntry.getCatalogEntryId() + + ", partNumber: " + catalogEntry.getPartNumber()); + } + } + } + + private void printDescriptions(List 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(""); + 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 catalogEntryIds = new ArrayList(); + catalogEntryIds.add(new Long(10001)); + catalogEntryIds.add(new Long(10002)); + catalogEntryIds.add(new Long(10003)); + List catalogPartNum = new ArrayList(); + 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 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(""); + 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 catalogEntryIds = new ArrayList(); + catalogEntryIds.add(new Long(10001)); + catalogEntryIds.add(new Long(10002)); + catalogEntryIds.add(new Long(10003)); + List catalogPartNum = new ArrayList(); + 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 catalogEntries = jpql.getResultList(); + printCatentries(catalogEntries); + assertEquals(2, catalogEntries.size()); + } + } + + public void testCase2(){// IN clause with entities in the list + List catalogEntries = em.createQuery("SELECT catentry FROM CatalogEntry catentry " + + "WHERE catentry.catalogEntryId IN(10001,10002)").getResultList(); + { + System.out.println(""); + Query jpql = em.createQuery("SELECT catentdesc FROM CatalogEntryDescription catentdesc " + + "WHERE catentdesc.CatalogEntryForCatalogEntryDescription IN(:catalogEntry)"); + jpql.setParameter("catalogEntry", catalogEntries); + printDescriptions(jpql.getResultList()); + System.out.println(""); + System.out.println(); + } + // at this point, the prepared statements are assumed to be cached, ast that is the default behaviour. + + { + System.out.println(""); + 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(""); + System.out.println(); + } + + } +} +