diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java index bf0d98267166..b751e9331e43 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java @@ -5,6 +5,7 @@ package org.hibernate.query.sql.internal; import java.io.Serializable; +import java.lang.reflect.Constructor; import java.time.Instant; import java.util.Calendar; import java.util.Collection; @@ -126,6 +127,7 @@ import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty; import static org.hibernate.internal.util.collections.CollectionHelper.isNotEmpty; import static org.hibernate.internal.util.collections.CollectionHelper.makeCopy; +import static org.hibernate.internal.util.type.PrimitiveWrapperHelper.getDescriptorByPrimitiveType; import static org.hibernate.jpa.HibernateHints.HINT_NATIVE_LOCK_MODE; import static org.hibernate.query.results.internal.Builders.resultClassBuilder; import static org.hibernate.query.results.ResultSetMapping.resolveResultSetMapping; @@ -331,25 +333,54 @@ private void handleExplicitResultSetMapping() { setTupleTransformerForResultType( resultType ); } else { - checkResultType( resultType ); + checkResultType( resultType, resultSetMapping ); } } } - private void checkResultType(Class resultType) { - switch ( resultSetMapping.getNumberOfResultBuilders() ) { - case 0: - throw new IllegalArgumentException( "Named query exists, but did not specify a resultClass" ); - case 1: - final Class actualResultJavaType = - resultSetMapping.getResultBuilders().get( 0 ).getJavaType(); - if ( actualResultJavaType != null && !resultType.isAssignableFrom( actualResultJavaType ) ) { - throw buildIncompatibleException( resultType, actualResultJavaType ); - } - break; - default: - throw new IllegalArgumentException( "Cannot create TypedQuery for query with more than one return" ); + private void checkResultType(Class resultType, ResultSetMapping resultSetMapping) { + // resultType can be null if any of the deprecated methods were used to create the query + if ( resultType != null && !isResultTypeAlwaysAllowed( resultType )) { + switch ( resultSetMapping.getNumberOfResultBuilders() ) { + case 0: + if ( !resultSetMapping.isDynamic() ) { + throw new IllegalArgumentException( "Named query exists, but did not specify a resultClass" ); + } + break; + case 1: + final Class actualResultJavaType = + resultSetMapping.getResultBuilders().get( 0 ).getJavaType(); + if ( actualResultJavaType != null && !resultType.isAssignableFrom( actualResultJavaType ) ) { + throw buildIncompatibleException( resultType, actualResultJavaType ); + } + break; + default: + // The return type has to be a class with an appropriate constructor, i.e. one whose parameter types match + // the types of the result builders. If none such constructor is found, throw an IAE + if ( !validConstructorFoundForResultType( resultType, resultSetMapping ) ) { + throw new IllegalArgumentException( "The declared return type for a multi-valued result set mapping should be Object[], Map, List, or Tuple" ); + } + } + } + } + + private boolean validConstructorFoundForResultType(Class resultType, ResultSetMapping resultSetMapping) { + // Only 1 constructor with the right number of parameters is allowed (see NativeQueryConstructorTransformer) + Constructor constructor = resultType.getConstructors()[0]; + if ( constructor.getParameterCount() != resultSetMapping.getNumberOfResultBuilders() ) { + return false; + } + final List resultBuilders = resultSetMapping.getResultBuilders(); + Class[] paramTypes = constructor.getParameterTypes(); + for ( int i = 0; i < resultBuilders.size(); i++ ) { + if ( + resultBuilders.get( i ).getJavaType() != ( paramTypes[i].isPrimitive() ? + getDescriptorByPrimitiveType(paramTypes[i] ).getWrapperClass() : + paramTypes[i]) ) { + return false; + } } + return true; } protected void setTupleTransformerForResultType(Class resultClass) { @@ -747,6 +778,7 @@ else if ( !isResultTypeAlwaysAllowed( resultType ) else { mapping = resultSetMapping; } + checkResultType( resultType, mapping ); return isCacheableQuery() ? getInterpretationCache() .resolveSelectQueryPlan( selectInterpretationsKey( mapping ), () -> createQueryPlan( mapping ) ) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/NativeQueryResultCheckingTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/NativeQueryResultCheckingTests.java new file mode 100644 index 000000000000..7367776939a7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/NativeQueryResultCheckingTests.java @@ -0,0 +1,198 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.query.sql; + +import java.util.List; +import java.util.Map; + +import jakarta.persistence.Tuple; +import org.hibernate.orm.test.sql.hand.Person; +import org.hibernate.query.NativeQuery; +import org.hibernate.testing.orm.domain.StandardDomainModel; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.domain.library.Book; +import org.hibernate.type.StandardBasicTypes; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +/** + * @author Jan Schatteman + */ +@DomainModel( + standardModels = StandardDomainModel.LIBRARY +) +@SessionFactory +@Jira("https://hibernate.atlassian.net/browse/HHH-19383") +public class NativeQueryResultCheckingTests { + + @Test + @Jira("https://hibernate.atlassian.net/browse/HHH-19376") + public void testForHHH19376(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + String sql = "SELECT p.*, COUNT(*) OVER() AS total_count " + + "FROM Person p " + + "WHERE p.name ILIKE :name " + + "ORDER BY p.id"; + // Declare Person as result type + NativeQuery query = session.createNativeQuery(sql, Person.class); + query.setParameter("name", "Ga%"); + query.setMaxResults(2); + query.setFirstResult(0); + // Now mutate the result set mapping and verify an Exception is thrown + assertThrows( IllegalArgumentException.class, + () -> query.addScalar( "total_count", StandardBasicTypes.LONG).list() ); + } + ); + } + + @Test + public void testOkMutateResultSetMappingWithString(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + } + + @Test + public void testNokMutateResultSetMappingWithString(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertThrows( IllegalArgumentException.class, + () -> session.createNativeQuery( "select title, isbn from Book", String.class ) + .addScalar( "title", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + } + + @Test + public void testOkMutateResultSetMappingWithBook(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select id, name from Book", Book.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList() + ) + ); + } + + @Test + public void testNokMutateResultSetMappingWithBook(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertThrows( IllegalArgumentException.class, + () -> session.createNativeQuery( "select title, isbn from Book", Book.class ) + // this mapping doesn't have an appropriate constructor in Book, should throw error + .addScalar( "title", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + } + + @Test + public void testMutateResultSetMappingWithObjectArray(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", Object[].class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); + } + + @Test + public void testMutateResultSetMappingWithTuple(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", Tuple.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); + } + + @Test + public void testMutateResultSetMappingWithMap(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", Map.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); + } + + @Test + public void testMutateResultSetMappingWithList(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", List.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); + } + + @Test + public void testRecordWithPrimitiveField(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select id, name from Person", Record.class) + // map a Long onto a primitive long, shouldn't throw an exception + .addScalar("id", Long.class) + .addScalar("name", String.class) + .getResultList() + ) + ); + } + + static class Record { + long id; + String name; + public Record(long id, String name) { + this.id = id; + this.name = name; + } + long id() { + return id; + } + String name() { + return name; + } + } + +}