[hibernate-commits] Hibernate SVN: r17959 - in core/trunk: core/src/main/java/org/hibernate/util and 1 other directories.

Author: steve.ebersole(a)jboss.com Date: 2009-11-11 15:27:39 -0500 (Wed, 11 Nov 2009) New Revision: 17959 Modified: core/trunk/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java core/trunk/core/src/main/java/org/hibernate/util/CollectionHelper.java core/trunk/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/trunk/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java 2009-11-11 18:56:29 UTC (rev 17958) +++ core/trunk/core/src/main/java/org/hibernate/engine/query/QueryPlanCache.java 2009-11-11 20:27:39 UTC (rev 17959) @@ -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,64 @@ 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 +319,11 @@ 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() { Modified: core/trunk/core/src/main/java/org/hibernate/util/CollectionHelper.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/util/CollectionHelper.java 2009-11-11 18:56:29 UTC (rev 17958) +++ core/trunk/core/src/main/java/org/hibernate/util/CollectionHelper.java 2009-11-11 20:27:39 UTC (rev 17959) @@ -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,6 +38,8 @@ * @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) ); @@ -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/trunk/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-11 18:56:29 UTC (rev 17958) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/filter/DynamicFilterTest.java 2009-11-11 20:27:39 UTC (rev 17959) @@ -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() ); @@ -224,7 +236,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() ); @@ -288,10 +300,10 @@ session.enableFilter("region").setParameter("region", "APAC"); DetachedCriteria lineItemSubquery = DetachedCriteria.forClass(LineItem.class) - .add(Restrictions.ge( "quantity", new Long(1L) )) - .createCriteria("product") - .add(Restrictions.eq("name", "Acme Hair Gel")) - .setProjection(Property.forName("id")); + .add( Restrictions.ge( "quantity", 1L ) ) + .createCriteria( "product" ) + .add( Restrictions.eq( "name", "Acme Hair Gel" ) ) + .setProjection( Property.forName( "id" ) ); List orders = session.createCriteria(Order.class) .add(Subqueries.exists(lineItemSubquery)) @@ -309,7 +321,7 @@ .setProjection(Property.forName("id")); lineItemSubquery = DetachedCriteria.forClass(LineItem.class) - .add(Restrictions.ge("quantity", new Long(1L) )) + .add(Restrictions.ge("quantity", 1L )) .createCriteria("product") .add(Subqueries.propertyIn("id", productSubquery)) .setProjection(Property.forName("id")); @@ -662,7 +674,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() );