Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHH-17956 Criteria multiselect ignores type of the criteria query and always returns list of Object[] #8247

Merged
merged 2 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -740,9 +740,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 @@ -41,7 +41,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 @@ -30,6 +30,7 @@
import jakarta.persistence.Tuple;
import jakarta.persistence.TupleElement;
import jakarta.persistence.criteria.CompoundSelection;
import org.checkerframework.checker.nullness.qual.Nullable;

import org.hibernate.CacheMode;
import org.hibernate.FlushMode;
Expand Down Expand Up @@ -260,24 +261,24 @@
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 +303,71 @@
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.getExpressible() );
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.getExpressible();
// 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,27 +378,29 @@
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();
}

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 ( expressibleJavaType instanceof PrimitiveJavaType ) {
final PrimitiveJavaType<?> javaType = (PrimitiveJavaType<?>) expressibleJavaType;
if ( javaType.getPrimitiveClass() != resultClass ) {
|| expectedResultClass == Map.class
|| expectedResultClass == Tuple.class;
}

protected static <T> void verifyResultType(Class<T> resultClass, @Nullable SqmExpressible<?> sqmExpressible) {
if ( sqmExpressible != null ) {
final JavaType<?> expressibleJavaType = sqmExpressible.getExpressibleJavaType();
assert expressibleJavaType != null;
final Class<?> javaTypeClass = expressibleJavaType.getJavaTypeClass();
if ( javaTypeClass != Object.class && !resultClass.isAssignableFrom( javaTypeClass ) ) {
if ( expressibleJavaType instanceof PrimitiveJavaType ) {
final PrimitiveJavaType<?> javaType = (PrimitiveJavaType<?>) expressibleJavaType;
if ( javaType.getPrimitiveClass() != resultClass ) {
throwQueryTypeMismatchException( resultClass, sqmExpressible );
}
}
else if ( !isMatchingDateType( javaTypeClass, resultClass, sqmExpressible ) ) {
throwQueryTypeMismatchException( resultClass, sqmExpressible );
}
// else special case, we are good
}
else if ( !isMatchingDateType( javaTypeClass, resultClass, sqmExpressible ) ) {
throwQueryTypeMismatchException( resultClass, sqmExpressible );
}
// else special case, we are good
}
}

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 @@ -137,11 +137,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 @@
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 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 @@
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