Skip to content

Commit

Permalink
HHH-17956 Criteria multiselect ignores type of the criteria query and…
Browse files Browse the repository at this point in the history
… always returns list of Object[]
  • Loading branch information
dreab8 authored and beikov committed Apr 25, 2024
1 parent 493efee commit 02c4f2f
Show file tree
Hide file tree
Showing 16 changed files with 476 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -708,9 +708,9 @@ The following functions are abbreviations for `extract()`:
| `year(x)` | `extract(year from x)` | ✖
| `month(x)` | `extract(month from x)` | ✖
| `day(x)` | `extract(day from x)` | ✖
| `hour(x)` | `extract(year from x)` | ✖
| `minute(x)` | `extract(year from x)` | ✖
| `second(x)` | `extract(year from x)` | ✖
| `hour(x)` | `extract(hour from x)` | ✖
| `minute(x)` | `extract(minute from x)` | ✖
| `second(x)` | `extract(second from x)` | ✖
|===

TIP: These abbreviations aren't part of the JPQL standard, but on the other hand they're a lot less verbose.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
*
* @author Christian Beikov
*/
class SumReturnTypeResolver implements FunctionReturnTypeResolver {
public class SumReturnTypeResolver implements FunctionReturnTypeResolver {

private final BasicType<Long> longType;
private final BasicType<Double> doubleType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@
import jakarta.persistence.criteria.CriteriaQuery;
import jakarta.persistence.criteria.CriteriaUpdate;
import org.hibernate.stat.spi.StatisticsImplementor;
import org.hibernate.type.descriptor.java.JavaType;
import org.hibernate.type.descriptor.java.spi.UnknownBasicJavaType;

import static java.lang.Boolean.TRUE;
import static org.hibernate.internal.util.ReflectHelper.isClass;
Expand Down Expand Up @@ -962,8 +964,7 @@ else if ( getFactory().getMappingMetamodel().isEntityClass( resultClass ) ) {
query.addEntity( resultClass, LockMode.READ );
}
else if ( resultClass != Object.class && resultClass != Object[].class ) {
if ( isClass( resultClass )
&& getTypeConfiguration().getJavaTypeRegistry().findDescriptor( resultClass ) == null ) {
if ( isClass( resultClass ) && !hasJavaTypeDescriptor( resultClass ) ) {
// not a basic type
query.setTupleTransformer( new NativeQueryConstructorTransformer<>( resultClass ) );
}
Expand All @@ -973,6 +974,11 @@ && getTypeConfiguration().getJavaTypeRegistry().findDescriptor( resultClass ) ==
}
}

private <T> boolean hasJavaTypeDescriptor(Class<T> resultClass) {
final JavaType<Object> descriptor = getTypeConfiguration().getJavaTypeRegistry().findDescriptor( resultClass );
return descriptor != null && descriptor.getClass() != UnknownBasicJavaType.class;
}

@Override
public <T> NativeQueryImplementor<T> createNativeQuery(String sqlString, Class<T> resultClass, String tableAlias) {
@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,24 +260,24 @@ protected void visitQueryReturnType(
SqmQueryPart<R> queryPart,
Class<R> expectedResultType,
SessionFactoryImplementor factory) {
assert getQueryString().equals( CRITERIA_HQL_STRING );

if ( queryPart instanceof SqmQuerySpec<?> ) {
final SqmQuerySpec<R> sqmQuerySpec = (SqmQuerySpec<R>) queryPart;
final List<SqmSelection<?>> sqmSelections = sqmQuerySpec.getSelectClause().getSelections();

if ( sqmSelections == null || sqmSelections.isEmpty() ) {
// make sure there is at least one root
final List<SqmRoot<?>> sqmRoots = sqmQuerySpec.getFromClause().getRoots();
if ( sqmRoots == null || sqmRoots.isEmpty() ) {
throw new IllegalArgumentException( "Criteria did not define any query roots" );
}
// if there is a single root, use that as the selection
if ( sqmRoots.size() == 1 ) {
sqmQuerySpec.getSelectClause().add( sqmRoots.get( 0 ), null );
}
else {
throw new IllegalArgumentException( "Criteria has multiple query roots" );
if ( getQueryString() == CRITERIA_HQL_STRING ) {

Check warning

Code scanning / CodeQL

Reference equality test on strings Warning

String values compared with == .
if ( sqmSelections == null || sqmSelections.isEmpty() ) {
// make sure there is at least one root
final List<SqmRoot<?>> sqmRoots = sqmQuerySpec.getFromClause().getRoots();
if ( sqmRoots == null || sqmRoots.isEmpty() ) {
throw new IllegalArgumentException( "Criteria did not define any query roots" );
}
// if there is a single root, use that as the selection
if ( sqmRoots.size() == 1 ) {
sqmQuerySpec.getSelectClause().add( sqmRoots.get( 0 ), null );
}
else {
throw new IllegalArgumentException( "Criteria has multiple query roots" );
}
}
}

Expand All @@ -302,28 +302,71 @@ protected static <T> void checkQueryReturnType(
if ( selections.size() == 1 ) {
// we have one item in the select list,
// the type has to match (no instantiation)
final SqmSelection<?> sqmSelection = selections.get(0);

// special case for parameters in the select list
final SqmSelectableNode<?> selection = sqmSelection.getSelectableNode();
if ( selection instanceof SqmParameter ) {
final SqmParameter<?> sqmParameter = (SqmParameter<?>) selection;
final SqmExpressible<?> nodeType = sqmParameter.getNodeType();
// we may not yet know a selection type
if ( nodeType == null || nodeType.getExpressibleJavaType() == null ) {
// we can't verify the result type up front
return;
final SqmSelection<?> sqmSelection = selections.get( 0 );
final SqmSelectableNode<?> selectableNode = sqmSelection.getSelectableNode();
if ( selectableNode.isCompoundSelection() ) {
final Class<?> expectedSelectItemType = expectedResultClass.isArray()
? expectedResultClass.getComponentType()
: expectedResultClass;
for ( JpaSelection<?> selection : selectableNode.getSelectionItems() ) {
verifySelectionType( expectedSelectItemType, sessionFactory, (SqmSelectableNode<?>) selection );
}
}

if ( !sessionFactory.getSessionFactoryOptions().getJpaCompliance().isJpaQueryComplianceEnabled() ) {
verifyResultType( expectedResultClass, sqmSelection.getNodeType() );
else {
verifySelectionType( expectedResultClass, sessionFactory, sqmSelection.getSelectableNode() );
}
}
else if ( expectedResultClass.isArray() ) {
final Class<?> componentType = expectedResultClass.getComponentType();
for ( SqmSelection<?> selection : selections ) {
verifySelectionType( componentType, sessionFactory, selection.getSelectableNode() );
}
}
// else, let's assume we can instantiate it!
}
}

private static <T> void verifySelectionType(
Class<T> expectedResultClass,
SessionFactoryImplementor sessionFactory,
SqmSelection<?> sqmSelection) {
// special case for parameters in the select list
final SqmSelectableNode<?> selection = sqmSelection.getSelectableNode();
if ( selection instanceof SqmParameter ) {
final SqmParameter<?> sqmParameter = (SqmParameter<?>) selection;
final SqmExpressible<?> nodeType = sqmParameter.getNodeType();
// we may not yet know a selection type
if ( nodeType == null || nodeType.getExpressibleJavaType() == null ) {
// we can't verify the result type up front
return;
}
}

if ( !sessionFactory.getSessionFactoryOptions().getJpaCompliance().isJpaQueryComplianceEnabled() ) {
verifyResultType( expectedResultClass, selection.getExpressible() );
}
}

private static <T> void verifySelectionType(
Class<T> expectedResultClass,
SessionFactoryImplementor sessionFactory,
SqmSelectableNode<?> selection) {
// special case for parameters in the select list
if ( selection instanceof SqmParameter ) {
final SqmParameter<?> sqmParameter = (SqmParameter<?>) selection;
final SqmExpressible<?> nodeType = sqmParameter.getNodeType();
// we may not yet know a selection type
if ( nodeType == null || nodeType.getExpressibleJavaType() == null ) {
// we can't verify the result type up front
return;
}
}

if ( !sessionFactory.getSessionFactoryOptions().getJpaCompliance().isJpaQueryComplianceEnabled() ) {
verifyResultType( expectedResultClass, selection.getExpressible() );
}
}

private static boolean isInstantiableWithoutMetadata(Class<?> resultType) {
return resultType == null
|| resultType.isArray()
Expand All @@ -334,17 +377,18 @@ private static boolean isInstantiableWithoutMetadata(Class<?> resultType) {
private static <T> boolean isResultTypeAlwaysAllowed(Class<T> expectedResultClass) {
return expectedResultClass == null
|| expectedResultClass == Object.class
|| expectedResultClass == Object[].class
|| expectedResultClass == List.class
|| expectedResultClass == Tuple.class
|| expectedResultClass.isArray();
|| expectedResultClass == Map.class
|| expectedResultClass == Tuple.class;
}

protected static <T> void verifyResultType(Class<T> resultClass, SqmExpressible<?> sqmExpressible) {
assert sqmExpressible != null;
final JavaType<?> expressibleJavaType = sqmExpressible.getExpressibleJavaType();
assert expressibleJavaType != null;
final Class<?> javaTypeClass = expressibleJavaType.getJavaTypeClass();
if ( !resultClass.isAssignableFrom( javaTypeClass ) ) {
if ( javaTypeClass != Object.class && !resultClass.isAssignableFrom( javaTypeClass ) ) {
if ( expressibleJavaType instanceof PrimitiveJavaType ) {
final PrimitiveJavaType<?> javaType = (PrimitiveJavaType<?>) expressibleJavaType;
if ( javaType.getPrimitiveClass() != resultClass ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,8 @@ default boolean hasCallbackActions() {
* The underlying session
*/
SharedSessionContractImplementor getSession();

default Class<?> getResultType() {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,13 @@ public ConcreteSqmSelectQueryPlan(
jdbcParameterBindings
);
session.autoFlushIfRequired( jdbcSelect.getAffectedTableNames(), true );
//noinspection unchecked
return session.getFactory().getJdbcServices().getJdbcSelectExecutor().list(
jdbcSelect,
jdbcParameterBindings,
listInterpreterExecutionContext( hql, executionContext, jdbcSelect, subSelectFetchKeyHandler ),
rowTransformer,
(Class<R>) executionContext.getResultType(),
uniqueSemantic
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,34 +251,7 @@ public QuerySqmImpl(
bindCriteriaParameter((SqmJpaCriteriaParameterWrapper<?>) sqmParameter);
}
}

if ( sqm instanceof SqmSelectStatement<?> ) {
SqmUtil.verifyIsSelectStatement( sqm, null );
final SqmQueryPart<R> queryPart = ( (SqmSelectStatement<R>) sqm ).getQueryPart();
// For criteria queries, we have to validate the fetch structure here
queryPart.validateQueryStructureAndFetchOwners();
visitQueryReturnType(
queryPart,
expectedResultType,
producer.getFactory()
);
}
else {
if ( expectedResultType != null ) {
throw new IllegalQueryOperationException( "Result type given for a non-SELECT Query", hql, null );
}
if ( sqm instanceof SqmUpdateStatement<?> ) {
final SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqm;
verifyImmutableEntityUpdate( CRITERIA_HQL_STRING, updateStatement, producer.getFactory() );
if ( updateStatement.getSetClause() == null
|| updateStatement.getSetClause().getAssignments().isEmpty() ) {
throw new IllegalArgumentException( "No assignments specified as part of UPDATE criteria" );
}
}
else if ( sqm instanceof SqmInsertStatement<?> ) {
verifyInsertTypesMatch( CRITERIA_HQL_STRING, (SqmInsertStatement<R>) sqm );
}
}
validateStatement( sqm, expectedResultType );

resultType = expectedResultType;
tupleMetadata = buildTupleMetadata( criteria, expectedResultType );
Expand All @@ -298,7 +271,12 @@ private <T> void bindCriteriaParameter(SqmJpaCriteriaParameterWrapper<T> sqmPara

private void validateStatement(SqmStatement<R> sqmStatement, Class<R> resultType) {
if ( sqmStatement instanceof SqmSelectStatement<?> ) {
SqmUtil.verifyIsSelectStatement( sqmStatement, hql );
final SqmQueryPart<R> queryPart = ( (SqmSelectStatement<R>) sqm ).getQueryPart();
if ( hql == CRITERIA_HQL_STRING ) {

Check warning

Code scanning / CodeQL

Reference equality test on strings Warning

String values compared with == .
// For criteria queries, we have to validate the fetch structure here
queryPart.validateQueryStructureAndFetchOwners();
}
visitQueryReturnType( queryPart, resultType, getSessionFactory() );
}
else {
if ( resultType != null ) {
Expand All @@ -307,6 +285,10 @@ private void validateStatement(SqmStatement<R> sqmStatement, Class<R> resultType
if ( sqmStatement instanceof SqmUpdateStatement<?> ) {
final SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqmStatement;
verifyImmutableEntityUpdate( hql, updateStatement, getSessionFactory() );
if ( updateStatement.getSetClause() == null
|| updateStatement.getSetClause().getAssignments().isEmpty() ) {
throw new IllegalArgumentException( "No assignments specified as part of UPDATE criteria" );
}
verifyUpdateTypesMatch( hql, updateStatement );
}
else if ( sqmStatement instanceof SqmInsertStatement<?> ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import java.util.function.Supplier;

import org.hibernate.SessionFactory;
import org.hibernate.dialect.function.AvgFunction;
import org.hibernate.dialect.function.SumReturnTypeResolver;
import org.hibernate.dialect.function.array.DdlTypeHelper;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.internal.CoreLogging;
Expand Down Expand Up @@ -81,6 +83,7 @@
import org.hibernate.query.sqm.function.NamedSqmFunctionDescriptor;
import org.hibernate.query.sqm.function.SqmFunctionDescriptor;
import org.hibernate.query.sqm.produce.function.FunctionArgumentException;
import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
import org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers;
import org.hibernate.query.sqm.spi.SqmCreationContext;
import org.hibernate.query.sqm.tree.SqmQuery;
Expand Down Expand Up @@ -201,6 +204,8 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext,
private transient BasicType<Integer> integerType;
private transient BasicType<Long> longType;
private transient BasicType<Character> characterType;
private transient FunctionReturnTypeResolver sumReturnTypeResolver;
private transient FunctionReturnTypeResolver avgReturnTypeResolver;
private final transient Map<Class<? extends HibernateCriteriaBuilder>, HibernateCriteriaBuilder> extensions;

public SqmCriteriaNodeBuilder(
Expand Down Expand Up @@ -248,7 +253,6 @@ public SessionFactoryImplementor getSessionFactory() {
}



@Override
public BasicType<Boolean> getBooleanType() {
final BasicType<Boolean> booleanType = this.booleanType;
Expand Down Expand Up @@ -293,6 +297,22 @@ public BasicType<Character> getCharacterType() {
return characterType;
}

public FunctionReturnTypeResolver getSumReturnTypeResolver() {
final FunctionReturnTypeResolver resolver = sumReturnTypeResolver;
if ( resolver == null ) {
return this.sumReturnTypeResolver = new SumReturnTypeResolver( getTypeConfiguration() );
}
return resolver;
}

public FunctionReturnTypeResolver getAvgReturnTypeResolver() {
final FunctionReturnTypeResolver resolver = avgReturnTypeResolver;
if ( resolver == null ) {
return this.avgReturnTypeResolver = new AvgFunction.ReturnTypeResolver( getTypeConfiguration() );
}
return resolver;
}

@Override
public QueryEngine getQueryEngine() {
return queryEngine;
Expand Down

0 comments on commit 02c4f2f

Please sign in to comment.