8:40 a.m.
Author: stliu
Date: 2009-11-13 09:40:44 -0500 (Fri, 13 Nov 2009)
New Revision: 17971
Modified:
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/engine/query/QueryPlanCache.java
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/util/CollectionHelper.java
core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/filter/DynamicFilterTest.java
Log:
JBPAPP-3098 HHH-4065 - Incorrect SQL is used for HQL if the number of values for a filter
collection parameter is changed
Modified:
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/engine/query/QueryPlanCache.java
===================================================================
---
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/engine/query/QueryPlanCache.java 2009-11-13
07:24:38 UTC (rev 17970)
+++
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/engine/query/QueryPlanCache.java 2009-11-13
14:40:44 UTC (rev 17971)
@@ -1,16 +1,19 @@
package org.hibernate.engine.query;
import org.hibernate.util.ArrayHelper;
+import org.hibernate.util.CollectionHelper;
import org.hibernate.util.SimpleMRUCache;
import org.hibernate.util.SoftLimitMRUCache;
import org.hibernate.engine.SessionFactoryImplementor;
import org.hibernate.engine.query.sql.NativeSQLQuerySpecification;
+import org.hibernate.impl.FilterImpl;
import org.hibernate.QueryException;
import org.hibernate.MappingException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import java.io.Serializable;
+import java.util.Collection;
import java.util.Map;
import java.util.HashMap;
import java.util.List;
@@ -152,7 +155,7 @@
private static class HQLQueryPlanKey implements Serializable {
private final String query;
private final boolean shallow;
- private final Set filterNames;
+ private final Set filterKeys;
private final int hashCode;
public HQLQueryPlanKey(String query, boolean shallow, Map enabledFilters) {
@@ -160,17 +163,23 @@
this.shallow = shallow;
if ( enabledFilters == null || enabledFilters.isEmpty() ) {
- filterNames = Collections.EMPTY_SET;
+ filterKeys = Collections.EMPTY_SET;
}
else {
- Set tmp = new HashSet();
- tmp.addAll( enabledFilters.keySet() );
- this.filterNames = Collections.unmodifiableSet( tmp );
+ Set tmp = new HashSet(
+ CollectionHelper.determineProperSizing( enabledFilters ),
+ CollectionHelper.LOAD_FACTOR
+ );
+ Iterator itr = enabledFilters.values().iterator();
+ while ( itr.hasNext() ) {
+ tmp.add( new DynamicFilterKey( ( FilterImpl ) itr.next() ) );
+ }
+ this.filterKeys = Collections.unmodifiableSet( tmp );
}
int hash = query.hashCode();
hash = 29 * hash + ( shallow ? 1 : 0 );
- hash = 29 * hash + filterNames.hashCode();
+ hash = 29 * hash + filterKeys.hashCode();
this.hashCode = hash;
}
@@ -184,24 +193,69 @@
final HQLQueryPlanKey that = ( HQLQueryPlanKey ) o;
- if ( shallow != that.shallow ) {
- return false;
+ return shallow == that.shallow
+ && filterKeys.equals( that.filterKeys )
+ && query.equals( that.query );
+
+ }
+
+ public int hashCode() {
+ return hashCode;
+ }
+ }
+ private static class DynamicFilterKey implements Serializable {
+ private final String filterName;
+ private final Map parameterMetadata;
+ private final int hashCode;
+
+ private DynamicFilterKey(FilterImpl filter) {
+ this.filterName = filter.getName();
+ if ( filter.getParameters().isEmpty() ) {
+ parameterMetadata = Collections.EMPTY_MAP;
}
- if ( !filterNames.equals( that.filterNames ) ) {
- return false;
+ else {
+ parameterMetadata = new HashMap(
+ CollectionHelper.determineProperSizing( filter.getParameters() ),
+ CollectionHelper.LOAD_FACTOR
+ );
+ Iterator itr = filter.getParameters().entrySet().iterator();
+ while ( itr.hasNext() ) {
+ final Integer valueCount;
+ final Map.Entry entry = ( Map.Entry ) itr.next();
+ if ( Collection.class.isInstance( entry.getValue() ) ) {
+ valueCount = new Integer( ( (Collection) entry.getValue() ).size() );
+ }
+ else {
+ valueCount = new Integer(1);
+ }
+ parameterMetadata.put( entry.getKey(), valueCount );
+ }
}
- if ( !query.equals( that.query ) ) {
+
+ int hash = filterName.hashCode();
+ hash = 31 * hash + parameterMetadata.hashCode();
+ this.hashCode = hash;
+ }
+
+ public boolean equals(Object o) {
+ if ( this == o ) {
+ return true;
+ }
+ if ( o == null || getClass() != o.getClass() ) {
return false;
}
- return true;
+ DynamicFilterKey that = ( DynamicFilterKey ) o;
+
+ return filterName.equals( that.filterName )
+ && parameterMetadata.equals( that.parameterMetadata );
+
}
public int hashCode() {
return hashCode;
}
}
-
private static class FilterQueryPlanKey implements Serializable {
private final String query;
private final String collectionRole;
@@ -240,22 +294,14 @@
final FilterQueryPlanKey that = ( FilterQueryPlanKey ) o;
- if ( shallow != that.shallow ) {
- return false;
- }
- if ( !filterNames.equals( that.filterNames ) ) {
- return false;
- }
- if ( !query.equals( that.query ) ) {
- return false;
- }
- if ( !collectionRole.equals( that.collectionRole ) ) {
- return false;
- }
+ return shallow == that.shallow
+ && filterNames.equals( that.filterNames )
+ && query.equals( that.query )
+ && collectionRole.equals( that.collectionRole );
- return true;
}
+
public int hashCode() {
return hashCode;
}
Modified: core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/util/CollectionHelper.java
===================================================================
---
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/util/CollectionHelper.java 2009-11-13
07:24:38 UTC (rev 17970)
+++
core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/util/CollectionHelper.java 2009-11-13
14:40:44 UTC (rev 17971)
@@ -1,4 +1,26 @@
-//$Id$
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors. All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA 02110-1301 USA
+ */
package org.hibernate.util;
import java.util.ArrayList;
@@ -7,16 +29,68 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
+ * Various help for handling collections.
+ *
* @author Gavin King
+ * @author Steve Ebersole
*/
public final class CollectionHelper {
+ public static final int MINIMUM_INITIAL_CAPACITY = 16;
+ public static final float LOAD_FACTOR = 0.75f;
public static final List EMPTY_LIST = Collections.unmodifiableList( new ArrayList(0) );
public static final Collection EMPTY_COLLECTION = Collections.unmodifiableCollection(
new ArrayList(0) );
public static final Map EMPTY_MAP = Collections.unmodifiableMap( new HashMap(0) );
-
- private CollectionHelper() {}
+ private CollectionHelper() {
+ }
+
+ /**
+ * Build a properly sized map, especially handling load size and load factor to prevent
immediate resizing.
+ * <p/>
+ * Especially helpful for copy map contents.
+ *
+ * @param size The size to make the map.
+ * @return The sized map.
+ */
+ public static Map mapOfSize(int size) {
+ return new HashMap( determineProperSizing( size ), LOAD_FACTOR );
+ }
+
+ /**
+ * Given a map, determine the proper initial size for a new Map to hold the same number
of values.
+ * Specifically we want to account for load size and load factor to prevent immediate
resizing.
+ *
+ * @param original The original map
+ * @return The proper size.
+ */
+ public static int determineProperSizing(Map original) {
+ return determineProperSizing( original.size() );
+ }
+
+ /**
+ * Given a set, determine the proper initial size for a new set to hold the same number
of values.
+ * Specifically we want to account for load size and load factor to prevent immediate
resizing.
+ *
+ * @param original The original set
+ * @return The proper size.
+ */
+ public static int determineProperSizing(Set original) {
+ return determineProperSizing( original.size() );
+ }
+
+ /**
+ * Determine the proper initial size for a new collection in order for it to hold the
given a number of elements.
+ * Specifically we want to account for load size and load factor to prevent immediate
resizing.
+ *
+ * @param numberOfElements The number of elements to be stored.
+ * @return The proper size.
+ */
+ public static int determineProperSizing(int numberOfElements) {
+ int actual = ( (int) (numberOfElements / LOAD_FACTOR) ) + 1;
+ return Math.max( actual, MINIMUM_INITIAL_CAPACITY );
+ }
}
Modified:
core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/filter/DynamicFilterTest.java
===================================================================
---
core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-13
07:24:38 UTC (rev 17970)
+++
core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-13
14:40:44 UTC (rev 17971)
@@ -37,6 +37,7 @@
*
* @author Steve
*/
+@SuppressWarnings({ "WhileLoopReplaceableByForEach", "unchecked" })
public class DynamicFilterTest extends FunctionalTestCase {
public DynamicFilterTest(String testName) {
@@ -88,7 +89,7 @@
ts = ( ( SessionImplementor ) session ).getTimestamp();
session.enableFilter( "fulfilledOrders" ).setParameter( "asOfDate",
testData.lastMonth.getTime() );
sp = ( Salesperson ) session.createQuery( "from Salesperson as s where s.id =
:id" )
- .setLong( "id", testData.steveId.longValue() )
+ .setLong( "id", testData.steveId )
.uniqueResult();
assertEquals( "Filtered-collection not bypassing 2L-cache", 1,
sp.getOrders().size() );
@@ -130,7 +131,14 @@
assertEquals( "Incorrect order count", 1, sp.getOrders().size() );
session.clear();
-
+ session.disableFilter( "regionlist" );
+ session.enableFilter( "regionlist" ).setParameterList( "regions",
new String[]{"LA", "APAC", "APAC"} );
+ // Second test retreival through hql with the collection as non-eager with different
region list
+ salespersons = session.createQuery( "select s from Salesperson as s"
).list();
+ assertEquals( "Incorrect salesperson count", 1, salespersons.size() );
+ sp = ( Salesperson ) salespersons.get( 0 );
+ assertEquals( "Incorrect order count", 1, sp.getOrders().size() );
+ session.clear();
// test retreival through hql with the collection join fetched
salespersons = session.createQuery( "select s from Salesperson as s left join
fetch s.orders" ).list();
assertEquals( "Incorrect salesperson count", 1, salespersons.size() );
@@ -189,7 +197,7 @@
assertEquals( "Incorrect order count", 1, ( ( Salesperson ) salespersons.get(
0 ) ).getOrders().size() );
List products = session.createCriteria( Product.class )
- .add( Restrictions.eq( "stockNumber", new Integer( 124 ) ) )
+ .add( Restrictions.eq( "stockNumber", 124 ) )
.list();
assertEquals( "Incorrect product count", 1, products.size() );
@@ -598,7 +606,7 @@
// Force the categories to not get initialized here
List result = session.createQuery( "from Product as p where p.id = :id" )
- .setLong( "id", testData.prod1Id.longValue() )
+ .setLong( "id", testData.prod1Id )
.list();
assertTrue( "No products returned from HQL", !result.isEmpty() );