2:36 p.m.
Author: steve.ebersole(a)jboss.com
Date: 2009-11-11 15:36:03 -0500 (Wed, 11 Nov 2009)
New Revision: 17960
Modified:
core/branches/Branch_3_3/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java
core/branches/Branch_3_3/core/src/main/java/org/hibernate/util/CollectionHelper.java
core/branches/Branch_3_3/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java
Log:
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_3/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java
===================================================================
---
core/branches/Branch_3_3/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java 2009-11-11
20:27:39 UTC (rev 17959)
+++
core/branches/Branch_3_3/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java 2009-11-11
20:36:03 UTC (rev 17960)
@@ -26,10 +26,13 @@
import org.hibernate.util.SimpleMRUCache;
import org.hibernate.util.SoftLimitMRUCache;
+import org.hibernate.util.CollectionHelper;
import org.hibernate.engine.SessionFactoryImplementor;
import org.hibernate.engine.query.sql.NativeSQLQuerySpecification;
import org.hibernate.QueryException;
import org.hibernate.MappingException;
+import org.hibernate.impl.FilterImpl;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -40,6 +43,7 @@
import java.util.Set;
import java.util.HashSet;
import java.util.Collections;
+import java.util.Collection;
/**
* Acts as a cache for compiled query plans, as well as query-parameter metadata.
@@ -174,7 +178,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) {
@@ -182,17 +186,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;
}
@@ -206,17 +216,63 @@
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() {
@@ -262,20 +318,10 @@
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 true;
+ return shallow == that.shallow
+ && filterNames.equals( that.filterNames )
+ && query.equals( that.query )
+ && collectionRole.equals( that.collectionRole );
}
public int hashCode() {
Modified:
core/branches/Branch_3_3/core/src/main/java/org/hibernate/util/CollectionHelper.java
===================================================================
---
core/branches/Branch_3_3/core/src/main/java/org/hibernate/util/CollectionHelper.java 2009-11-11
20:27:39 UTC (rev 17959)
+++
core/branches/Branch_3_3/core/src/main/java/org/hibernate/util/CollectionHelper.java 2009-11-11
20:36:03 UTC (rev 17960)
@@ -29,6 +29,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
/**
* Various help for handling collections.
@@ -37,11 +38,13 @@
* @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() {
}
@@ -54,7 +57,40 @@
* @return The sized map.
*/
public static Map mapOfSize(int size) {
- final int currentSize = (int) (size / 0.75f);
- return new HashMap( Math.max( currentSize+ 1, 16), 0.75f );
+ 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_3/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java
===================================================================
---
core/branches/Branch_3_3/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-11
20:27:39 UTC (rev 17959)
+++
core/branches/Branch_3_3/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-11
20:36:03 UTC (rev 17960)
@@ -39,6 +39,7 @@
*
* @author Steve
*/
+@SuppressWarnings({ "WhileLoopReplaceableByForEach", "unchecked" })
public class DynamicFilterTest extends FunctionalTestCase {
private Logger log = LoggerFactory.getLogger( DynamicFilterTest.class );
@@ -99,7 +100,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() );
@@ -142,6 +143,17 @@
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() );
@@ -206,7 +218,7 @@
log.info( "Criteria query against Product..." );
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() );
@@ -644,7 +656,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() );