Author: gbadner Date: 2010-07-29 23:03:41 -0400 (Thu, 29 Jul 2010) New Revision: 20078 Modified: core/trunk/core/src/main/java/org/hibernate/cache/QueryKey.java core/trunk/core/src/main/java/org/hibernate/hql/HolderInstantiator.java core/trunk/core/src/main/java/org/hibernate/hql/classic/QueryTranslatorImpl.java core/trunk/core/src/main/java/org/hibernate/loader/Loader.java core/trunk/core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java core/trunk/core/src/main/java/org/hibernate/loader/custom/CustomLoader.java core/trunk/core/src/main/java/org/hibernate/loader/hql/QueryLoader.java core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCacheNormalResultTransformerTest.java core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCachePutResultTransformerTest.java core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/QueryCacheTest.java Log: HHH-5424 HHH-5425 : Put ResultTransformer in QueryKey only if data is transformed; PropertyAccessException when caching 1 result per row Modified: core/trunk/core/src/main/java/org/hibernate/cache/QueryKey.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/cache/QueryKey.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/cache/QueryKey.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -73,6 +73,8 @@ * @param queryParameters The query parameters * @param filterKeys The keys of any enabled filters. * @param session The current session. + * @param customTransformer The result transformer; should be + * null if data is not transformed before being cached. * * @return The generate query cache key. */ @@ -80,7 +82,8 @@ String queryString, QueryParameters queryParameters, Set filterKeys, - SessionImplementor session) { + SessionImplementor session, + ResultTransformer customTransformer) { // disassemble positional parameters final int positionalParameterCount = queryParameters.getPositionalParameterTypes().length; final Type[] types = new Type[positionalParameterCount]; @@ -134,7 +137,7 @@ maxRows, filterKeys, session.getEntityMode(), - queryParameters.getResultTransformer() + customTransformer ); } Modified: core/trunk/core/src/main/java/org/hibernate/hql/HolderInstantiator.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/hql/HolderInstantiator.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/hql/HolderInstantiator.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -41,13 +41,16 @@ private final String[] queryReturnAliases; public static HolderInstantiator getHolderInstantiator(ResultTransformer selectNewTransformer, ResultTransformer customTransformer, String[] queryReturnAliases) { - if(selectNewTransformer!=null) { - return new HolderInstantiator(selectNewTransformer, queryReturnAliases); - } else { - return new HolderInstantiator(customTransformer, queryReturnAliases); - } + return new HolderInstantiator( + resolveResultTransformer( selectNewTransformer, customTransformer ), + queryReturnAliases + ); } - + + public static ResultTransformer resolveResultTransformer(ResultTransformer selectNewTransformer, ResultTransformer customTransformer) { + return selectNewTransformer != null ? selectNewTransformer : customTransformer; + } + public static ResultTransformer createSelectNewTransformer(Constructor constructor, boolean returnMaps, boolean returnLists) { if ( constructor != null ) { return new AliasToBeanConstructorResultTransformer(constructor); @@ -65,14 +68,15 @@ static public HolderInstantiator createClassicHolderInstantiator(Constructor constructor, ResultTransformer transformer) { - if ( constructor != null ) { - return new HolderInstantiator(new AliasToBeanConstructorResultTransformer(constructor), null); - } - else { - return new HolderInstantiator(transformer, null); - } + return new HolderInstantiator( resolveClassicResultTransformer( constructor, transformer ), null ); } - + + static public ResultTransformer resolveClassicResultTransformer( + Constructor constructor, + ResultTransformer transformer) { + return constructor != null ? new AliasToBeanConstructorResultTransformer( constructor ) : transformer; + } + public HolderInstantiator( ResultTransformer transformer, String[] queryReturnAliases Modified: core/trunk/core/src/main/java/org/hibernate/hql/classic/QueryTranslatorImpl.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/hql/classic/QueryTranslatorImpl.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/hql/classic/QueryTranslatorImpl.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -983,6 +983,13 @@ throw new UnsupportedOperationException( "Not supported! Use the AST translator..."); } + protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) { + return HolderInstantiator.resolveClassicResultTransformer( + holderConstructor, + resultTransformer + ); + } + protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session) throws SQLException, HibernateException { row = toResultRow( row ); Modified: core/trunk/core/src/main/java/org/hibernate/loader/Loader.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/loader/Loader.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/loader/Loader.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -1008,11 +1008,31 @@ .endLoadingCollections( collectionPersister ); } + /** + * Determine the actual ResultTransformer that will be used to + * transform query results. + * + * @param resultTransformer the specified result transformer + * @return the actual result transformer + */ + protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) { + return resultTransformer; + } + protected List getResultList(List results, ResultTransformer resultTransformer) throws QueryException { return results; } /** + * Are rows transformed immediately after being read from the ResultSet? + * @param transformer, the specified transformer + * @return true, if getResultColumnOrRow() transforms the results; false, otherwise + */ + protected boolean areResultSetRowsTransformedImmediately(ResultTransformer transformer) { + return false; + } + + /** * Get the actual object that is returned in the user-visible result list. * This empty implementation merely returns its first argument. This is * overridden by some subclasses. @@ -2272,7 +2292,11 @@ getSQLString(), queryParameters, filterKeys, - session + session, + ( areResultSetRowsTransformedImmediately( queryParameters.getResultTransformer() ) ? + queryParameters.getResultTransformer() : + null + ) ); if ( querySpaces == null || querySpaces.size() == 0 ) { @@ -2294,7 +2318,7 @@ if ( result == null ) { result = doList( session, queryParameters ); - putResultInQueryCache( + putResultInQueryCache( session, queryParameters, resultTypes, @@ -2304,6 +2328,7 @@ ); } + return getResultList( result, queryParameters.getResultTransformer() ); } @@ -2344,6 +2369,22 @@ persistenceContext.setDefaultReadOnly( defaultReadOnlyOrig ); } + // If there is a result transformer, but the loader is not expecting the data to be + // transformed yet, then the loader expects result elements that are Object[]. + // The problem is that StandardQueryCache.get(...) does not return a tuple when + // resultTypes.length == 1. The following changes the data returned from the cache + // to be a tuple. + // TODO: this really doesn't belong here, but only Loader has the information + // to be able to do this. + if ( result != null && + resultTypes.length == 1 && + key.getResultTransformer() == null && + resolveResultTransformer( queryParameters.getResultTransformer() ) != null ) { + for ( int i = 0 ; i < result.size() ; i++ ) { + result.set( i, new Object[] { result.get( i ) } ); + } + } + if ( factory.getStatistics().isStatisticsEnabled() ) { if ( result == null ) { factory.getStatisticsImplementor() @@ -2374,7 +2415,23 @@ result ); } - boolean put = queryCache.put( key, resultTypes, result, queryParameters.isNaturalKeyLookup(), session ); + // If there is a result transformer, but the data has not been transformed yet, + // then result elements are Object[]. The problem is that StandardQueryCache.put(...) + // does not expect a tuple when resultTypes.length == 1. The following changes the + // data being cached to what StandardQueryCache.put(...) expects. + // TODO: this really doesn't belong here, but only Loader has the information + // to be able to do this. + List cachedResult = result; + if ( resultTypes.length == 1 && + key.getResultTransformer() == null && + resolveResultTransformer( queryParameters.getResultTransformer() ) != null ) { + cachedResult = new ArrayList( result.size() ); + for ( int i = 0 ; i < result.size() ; i++ ) { + cachedResult.add( ( ( Object[] ) result.get( i ) )[ 0 ] ); + } + } + + boolean put = queryCache.put( key, resultTypes, cachedResult, queryParameters.isNaturalKeyLookup(), session ); if ( put && factory.getStatistics().isStatisticsEnabled() ) { factory.getStatisticsImplementor() .queryCachePut( getQueryIdentifier(), queryCache.getRegion().getName() ); @@ -2382,7 +2439,6 @@ } } - private void logCachedResultDetails(ResultTransformer resultTransformer, Type[] returnTypes, List result) { if ( ! log.isTraceEnabled() ) { return; Modified: core/trunk/core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/loader/criteria/CriteriaLoader.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -120,6 +120,16 @@ } + protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) { + return translator.getRootCriteria().getResultTransformer(); + } + + protected boolean areResultSetRowsTransformedImmediately( ResultTransformer transformer ) { + // comparing to null just in case there is no transformer + // (there should always be a result transformer; + return resolveResultTransformer( transformer ) != null; + } + protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session) throws SQLException, HibernateException { final Object[] result; @@ -145,8 +155,7 @@ result = row; aliases = userAliases; } - return translator.getRootCriteria().getResultTransformer() - .transformTuple(result, aliases); + return resolveResultTransformer( transformer ).transformTuple(result, aliases); } public Set getQuerySpaces() { @@ -196,9 +205,9 @@ protected boolean isSubselectLoadingEnabled() { return hasSubselectLoadableCollections(); } - + protected List getResultList(List results, ResultTransformer resultTransformer) { - return translator.getRootCriteria().getResultTransformer().transformList( results ); + return resolveResultTransformer( resultTransformer ).transformList( results ); } } Modified: core/trunk/core/src/main/java/org/hibernate/loader/custom/CustomLoader.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/loader/custom/CustomLoader.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/loader/custom/CustomLoader.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -335,6 +335,10 @@ return new HolderInstantiator(resultTransformer, queryReturnAliases); } } + + protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) { + return HolderInstantiator.resolveResultTransformer( null, resultTransformer ); + } protected Object getResultColumnOrRow( Object[] row, Modified: core/trunk/core/src/main/java/org/hibernate/loader/hql/QueryLoader.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/loader/hql/QueryLoader.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/core/src/main/java/org/hibernate/loader/hql/QueryLoader.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -382,6 +382,10 @@ return implicitResultTransformer != null; } + protected ResultTransformer resolveResultTransformer(ResultTransformer resultTransformer) { + return HolderInstantiator.resolveResultTransformer( implicitResultTransformer, resultTransformer ); + } + protected Object getResultColumnOrRow(Object[] row, ResultTransformer transformer, ResultSet rs, SessionImplementor session) throws SQLException, HibernateException { Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCacheNormalResultTransformerTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCacheNormalResultTransformerTest.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCacheNormalResultTransformerTest.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -45,29 +45,4 @@ protected CacheMode getQueryCacheMode() { return CacheMode.NORMAL; } - - public void testAliasToBeanDtoMultiArgList() { - reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode", - "Query using Transformers.aliasToBean with cache" - ); - } - public void testAliasToBeanDtoMultiArgListFailureExpected() throws Exception { - super.testAliasToBeanDtoMultiArgList(); - } - - public void testAliasToBeanDtoLiteralArgList() { - reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode", - "Query using Transformers.aliasToBean with cache" ); - } - public void testAliasToBeanDtoLiteralArgListFailureExpected() throws Exception { - super.testAliasToBeanDtoLiteralArgList(); - } - - public void testAliasToBeanDtoWithNullAliasList() { - reportSkip( "Results from queries using Transformers.aliasToBean cannot be found in the cache due to bug in hashCode", - "Query using Transformers.aliasToBean with cache" ); - } - public void testAliasToBeanDtoWithNullAliasListFailureExpected() throws Exception { - super.testAliasToBeanDtoWithNullAliasList(); - } } Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCachePutResultTransformerTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCachePutResultTransformerTest.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/HqlQueryCachePutResultTransformerTest.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -50,29 +50,4 @@ protected boolean areDynamicNonLazyAssociationsChecked() { return false; } - - public void testAliasToEntityMapOneProjectionList() { - reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value", - "HQL queries using a ResultTransformer has row with a single value"); - } - public void testAliasToEntityMapOneProjectionListFailureExpected() throws Exception { - super.testAliasToEntityMapOneProjectionList(); - } - - public void testAliasToBeanDtoOneArgList() { - reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value", - "HQL queries using a ResultTransformer has row with a single value"); - } - - public void testAliasToBeanDtoOneArgListFailureExpected() throws Exception { - super.testAliasToBeanDtoOneArgList(); - } - - public void testOneSelectNewList() { - reportSkip( "HQL queries using a ResultTransformer are known to fail when caching a row with a single value", - "HQL queries using a ResultTransformer has row with a single value"); - } - public void testOneSelectNewListFailureExpected() throws Exception { - super.testOneSelectNewList(); - } } Modified: core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/QueryCacheTest.java =================================================================== --- core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/QueryCacheTest.java 2010-07-29 21:29:48 UTC (rev 20077) +++ core/trunk/testsuite/src/test/java/org/hibernate/test/querycache/QueryCacheTest.java 2010-07-30 03:03:41 UTC (rev 20078) @@ -224,7 +224,7 @@ getSessions().evictQueries(); getSessions().getStatistics().clear(); - final String queryString = "select i.description from Item i where i.name='widget'"; + final String queryString = "select i.description as desc from Item i where i.name='widget'"; Session s = openSession(); Transaction t = s.beginTransaction(); @@ -239,48 +239,64 @@ QueryStatistics qs = s.getSessionFactory().getStatistics().getQueryStatistics( queryString ); EntityStatistics es = s.getSessionFactory().getStatistics().getEntityStatistics( Item.class.getName() ); + assertEquals( qs.getCacheHitCount(), 0 ); + assertEquals( qs.getCacheMissCount(), 1 ); + assertEquals( qs.getCachePutCount(), 1 ); + Thread.sleep(200); s = openSession(); t = s.beginTransaction(); List result = s.createQuery( queryString ).setCacheable(true).list(); assertEquals( result.size(), 1 ); + assertEquals( i.getDescription(), ( ( String ) result.get( 0 ) ) ); t.commit(); s.close(); assertEquals( qs.getCacheHitCount(), 0 ); + assertEquals( qs.getCacheMissCount(), 2 ); + assertEquals( qs.getCachePutCount(), 2 ); s = openSession(); t = s.beginTransaction(); result = s.createQuery( queryString ).setCacheable(true).list(); assertEquals( result.size(), 1 ); + assertEquals( i.getDescription(), result.get( 0 ) ); t.commit(); s.close(); assertEquals( qs.getCacheHitCount(), 1 ); + assertEquals( qs.getCacheMissCount(), 2 ); + assertEquals( qs.getCachePutCount(), 2 ); s = openSession(); t = s.beginTransaction(); result = s.createQuery( queryString ).setCacheable(true).setResultTransformer(Transformers.ALIAS_TO_ENTITY_MAP).list(); assertEquals( result.size(), 1 ); Map m = (Map) result.get(0); - assertEquals(1, m.size()); + assertEquals( 1, m.size() ); + assertEquals( i.getDescription(), m.get( "desc" ) ); t.commit(); s.close(); - assertEquals( "hit count should not go up since we are adding a resulttransformer", qs.getCacheHitCount(), 1 ); - + assertEquals( "hit count should go up since data is not transformed until after it is cached", qs.getCacheHitCount(), 2 ); + assertEquals( qs.getCacheMissCount(), 2 ); + assertEquals( qs.getCachePutCount(), 2 ); + s = openSession(); t = s.beginTransaction(); result = s.createQuery( queryString ).setCacheable(true).setResultTransformer(Transformers.ALIAS_TO_ENTITY_MAP).list(); assertEquals( result.size(), 1 ); m = (Map) result.get(0); assertEquals(1, m.size()); + assertEquals( i.getDescription(), m.get( "desc" ) ); t.commit(); s.close(); - - assertEquals( "hit count should go up since we are using the same resulttransformer", qs.getCacheHitCount(), 2 ); - + + assertEquals( "hit count should go up since data is not transformed until after it is cachedr", qs.getCacheHitCount(), 3 ); + assertEquals( qs.getCacheMissCount(), 2 ); + assertEquals( qs.getCachePutCount(), 2 ); + s = openSession(); t = s.beginTransaction(); result = s.createQuery( queryString ).setCacheable(true).list(); @@ -292,8 +308,9 @@ t.commit(); s.close(); - assertEquals( qs.getCacheHitCount(), 3 ); - assertEquals( qs.getCacheMissCount(), 3 ); + assertEquals( qs.getCacheHitCount(), 4 ); + assertEquals( qs.getCacheMissCount(), 2 ); + assertEquals( qs.getCachePutCount(), 2 ); Thread.sleep(200); @@ -303,15 +320,19 @@ assertEquals( result.size(), 1 ); i = (Item) s.get( Item.class, new Long(i.getId()) ); assertEquals( (String) result.get(0), "A middle-quality widget." ); - + + assertEquals( qs.getCacheHitCount(), 4 ); + assertEquals( qs.getCacheMissCount(), 3 ); + assertEquals( qs.getCachePutCount(), 3 ); + s.delete(i); t.commit(); s.close(); - assertEquals( qs.getCacheHitCount(), 3 ); - assertEquals( qs.getCacheMissCount(), 4 ); - assertEquals( qs.getCachePutCount(), 4 ); - assertEquals( qs.getExecutionCount(), 4 ); + assertEquals( qs.getCacheHitCount(), 4 ); + assertEquals( qs.getCacheMissCount(), 3 ); + assertEquals( qs.getCachePutCount(), 3 ); + assertEquals( qs.getExecutionCount(), 3 ); assertEquals( es.getFetchCount(), 0 ); //check that it was being cached }