fix bug in query plan cache where fetch profiles were not considered

This commit is contained in:
Gavin 2023-05-23 12:13:30 +02:00 committed by Gavin King
parent ba0221da36
commit b3e27788fa
1 changed files with 40 additions and 48 deletions

View File

@ -6,6 +6,8 @@
*/ */
package org.hibernate.query.sqm.internal; package org.hibernate.query.sqm.internal;
import java.util.Collection;
import java.util.HashSet;
import java.util.function.Supplier; import java.util.function.Supplier;
import org.hibernate.LockOptions; import org.hibernate.LockOptions;
@ -16,6 +18,7 @@ import org.hibernate.query.spi.QueryInterpretationCache;
import org.hibernate.query.spi.QueryOptions; import org.hibernate.query.spi.QueryOptions;
import static java.lang.Boolean.TRUE; import static java.lang.Boolean.TRUE;
import static org.hibernate.query.spi.AbstractSelectionQuery.CRITERIA_HQL_STRING;
/** /**
* @author Steve Ebersole * @author Steve Ebersole
@ -33,49 +36,39 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key {
} }
public static SqmInterpretationsKey createInterpretationsKey(InterpretationsKeySource keySource) { public static SqmInterpretationsKey createInterpretationsKey(InterpretationsKeySource keySource) {
if ( ! isCacheable( keySource ) ) { if ( isCacheable (keySource ) ) {
return new SqmInterpretationsKey(
keySource.getQueryString(),
keySource.getResultType(),
keySource.getQueryOptions().getLockOptions(),
keySource.getQueryOptions().getTupleTransformer(),
keySource.getQueryOptions().getResultListTransformer(),
new HashSet<>( keySource.getLoadQueryInfluencers().getEnabledFetchProfileNames() )
);
}
else {
return null; return null;
} }
return new SqmInterpretationsKey(
keySource.getQueryString(),
keySource.getResultType(),
keySource.getQueryOptions().getLockOptions(),
keySource.getQueryOptions().getTupleTransformer(),
keySource.getQueryOptions().getResultListTransformer()
);
} }
@SuppressWarnings("RedundantIfStatement")
private static boolean isCacheable(InterpretationsKeySource keySource) { private static boolean isCacheable(InterpretationsKeySource keySource) {
assert keySource.getQueryOptions().getAppliedGraph() != null; assert keySource.getQueryOptions().getAppliedGraph() != null;
if ( QuerySqmImpl.CRITERIA_HQL_STRING.equals( keySource.getQueryString() ) ) { // for now at least, skip caching Criteria-based plans
// for now at least, skip caching Criteria-based plans // - especially wrt parameters atm; this works with HQL because the
// - especially wrt parameters atm; this works with HQL because the parameters // parameters are part of the query string; with Criteria, they're not.
// are part of the query string; with Criteria, they are not. return ! CRITERIA_HQL_STRING.equals( keySource.getQueryString() )
return false; // At the moment we cannot cache query plan if there is filter enabled.
} && ! keySource.getLoadQueryInfluencers().hasEnabledFilters()
// At the moment we cannot cache query plan if it has an entity graph
if ( keySource.getLoadQueryInfluencers().hasEnabledFilters() ) { && keySource.getQueryOptions().getAppliedGraph().getSemantic() == null
// At the moment we cannot cache query plan if there is filter enabled. // todo (6.0) : this one may be ok because of how I implemented multi-valued param handling
return false; // - the expansion is done per-execution based on the "static" SQM
} // - Note from Christian: The call to domainParameterXref.clearExpansions()
// in ConcreteSqmSelectQueryPlan is a concurrency issue when cached
if ( keySource.getQueryOptions().getAppliedGraph().getSemantic() != null ) { // - This could be solved by using a method-local clone of domainParameterXref
// At the moment we cannot cache query plan if there is an // when multi-valued params exist
// EntityGraph enabled. && ! keySource.hasMultiValuedParameterBindingsChecker().get() == TRUE;
return false;
}
if ( keySource.hasMultiValuedParameterBindingsChecker().get() == TRUE ) {
// todo (6.0) : this one may be ok because of how I implemented multi-valued param handling
// - the expansion is done per-execution based on the "static" SQM
// - Note from Christian: The call to domainParameterXref.clearExpansions() in ConcreteSqmSelectQueryPlan is a concurrency issue when cached
// - This could be solved by using a method-local clone of domainParameterXref when multi-valued params exist
return false;
}
return true;
} }
public static QueryInterpretationCache.Key generateNonSelectKey(InterpretationsKeySource keyDetails) { public static QueryInterpretationCache.Key generateNonSelectKey(InterpretationsKeySource keyDetails) {
@ -86,24 +79,26 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key {
return null; return null;
} }
private final String query; private final String query;
private final Class<?> resultType; private final Class<?> resultType;
private final LockOptions lockOptions; private final LockOptions lockOptions;
private final TupleTransformer<?> tupleTransformer; private final TupleTransformer<?> tupleTransformer;
private final ResultListTransformer resultListTransformer; private final ResultListTransformer<?> resultListTransformer;
private final Collection<String> enabledFetchProfiles;
private SqmInterpretationsKey( private SqmInterpretationsKey(
String query, String query,
Class<?> resultType, Class<?> resultType,
LockOptions lockOptions, LockOptions lockOptions,
TupleTransformer<?> tupleTransformer, TupleTransformer<?> tupleTransformer,
ResultListTransformer resultListTransformer) { ResultListTransformer<?> resultListTransformer,
Collection<String> enabledFetchProfiles) {
this.query = query; this.query = query;
this.resultType = resultType; this.resultType = resultType;
this.lockOptions = lockOptions; this.lockOptions = lockOptions;
this.tupleTransformer = tupleTransformer; this.tupleTransformer = tupleTransformer;
this.resultListTransformer = resultListTransformer; this.resultListTransformer = resultListTransformer;
this.enabledFetchProfiles = enabledFetchProfiles;
} }
@Override @Override
@ -114,7 +109,8 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key {
// Since lock options are mutable, we need a copy for the cache key // Since lock options are mutable, we need a copy for the cache key
lockOptions.makeCopy(), lockOptions.makeCopy(),
tupleTransformer, tupleTransformer,
resultListTransformer resultListTransformer,
enabledFetchProfiles
); );
} }
@ -137,16 +133,12 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key {
&& areEqual( resultType, that.resultType ) && areEqual( resultType, that.resultType )
&& areEqual( lockOptions, that.lockOptions ) && areEqual( lockOptions, that.lockOptions )
&& areEqual( tupleTransformer, that.tupleTransformer ) && areEqual( tupleTransformer, that.tupleTransformer )
&& areEqual( resultListTransformer, that.resultListTransformer ); && areEqual( resultListTransformer, that.resultListTransformer )
&& areEqual( enabledFetchProfiles, that.enabledFetchProfiles );
} }
private <T> boolean areEqual(T o1, T o2) { private <T> boolean areEqual(T o1, T o2) {
if ( o1 == null ) { return o1 == null ? o2 == null : o1.equals(o2);
return o2 == null;
}
else {
return o1.equals( o2 );
}
} }
@Override @Override