From c6a367ad26407415330ff6a8e1ee40a3dc3593b8 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Wed, 21 May 2025 23:36:12 +0200 Subject: [PATCH 1/2] HHH-19383 add more tests --- .../sql/NativeQueryResultCheckingTests.java | 166 +++++++++++++++--- 1 file changed, 137 insertions(+), 29 deletions(-) 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 index 7367776939a7..fba4161d3101 100644 --- 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 @@ -55,7 +55,7 @@ public void testForHHH19376(SessionFactoryScope scope) { } @Test - public void testOkMutateResultSetMappingWithString(SessionFactoryScope scope) { + public void testOneColumn(SessionFactoryScope scope) { scope.inTransaction( session -> assertDoesNotThrow( @@ -64,15 +64,50 @@ public void testOkMutateResultSetMappingWithString(SessionFactoryScope scope) { .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 ) + () -> session.createNativeQuery( "select isbn from Book", Integer.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book", Object[].class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book", Tuple.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book", Map.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book", Object.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select isbn from Book" ) .addScalar( "isbn", String.class ) .getResultList() ) @@ -80,26 +115,67 @@ public void testNokMutateResultSetMappingWithString(SessionFactoryScope scope) { } @Test - public void testOkMutateResultSetMappingWithBook(SessionFactoryScope scope) { + public void testTwoStringColumns(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertThrows( IllegalArgumentException.class, + () -> session.createNativeQuery( "select name, isbn from Book", String.class ) + .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); scope.inTransaction( session -> assertDoesNotThrow( - () -> session.createNativeQuery( "select id, name from Book", Book.class ) - .addScalar( "id", Integer.class ) + () -> session.createNativeQuery( "select name, isbn from Book", Object[].class ) .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select name, isbn from Book", Tuple.class ) + .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select name, isbn from Book", Map.class ) + .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select name, isbn from Book", Object.class ) + .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) + .getResultList() + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> session.createNativeQuery( "select name, isbn from Book" ) + .addScalar( "name", String.class ) + .addScalar( "isbn", String.class ) .getResultList() ) ); - } - - @Test - public void testNokMutateResultSetMappingWithBook(SessionFactoryScope scope) { scope.inTransaction( session -> assertThrows( IllegalArgumentException.class, - () -> session.createNativeQuery( "select title, isbn from Book", Book.class ) + () -> session.createNativeQuery( "select name, isbn from Book", Book.class ) // this mapping doesn't have an appropriate constructor in Book, should throw error - .addScalar( "title", String.class ) + .addScalar( "name", String.class ) .addScalar( "isbn", String.class ) .getResultList() ) @@ -107,7 +183,20 @@ public void testNokMutateResultSetMappingWithBook(SessionFactoryScope scope) { } @Test - public void testMutateResultSetMappingWithObjectArray(SessionFactoryScope scope) { + 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 testAllColumns(SessionFactoryScope scope) { scope.inTransaction( session -> assertDoesNotThrow( @@ -119,10 +208,6 @@ public void testMutateResultSetMappingWithObjectArray(SessionFactoryScope scope) } ) ); - } - - @Test - public void testMutateResultSetMappingWithTuple(SessionFactoryScope scope) { scope.inTransaction( session -> assertDoesNotThrow( @@ -134,10 +219,6 @@ public void testMutateResultSetMappingWithTuple(SessionFactoryScope scope) { } ) ); - } - - @Test - public void testMutateResultSetMappingWithMap(SessionFactoryScope scope) { scope.inTransaction( session -> assertDoesNotThrow( @@ -149,10 +230,28 @@ public void testMutateResultSetMappingWithMap(SessionFactoryScope scope) { } ) ); - } - - @Test - public void testMutateResultSetMappingWithList(SessionFactoryScope scope) { + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", Object.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); + scope.inTransaction( + session -> + assertDoesNotThrow( + () -> { + session.createNativeQuery( "select * from Book", Book.class ) + .addScalar( "id", Integer.class ) + .addScalar( "name", String.class ) + .getResultList(); + } + ) + ); scope.inTransaction( session -> assertDoesNotThrow( @@ -164,6 +263,15 @@ public void testMutateResultSetMappingWithList(SessionFactoryScope scope) { } ) ); + scope.inTransaction( + session -> + assertThrows( IllegalArgumentException.class, + () -> session.createNativeQuery( "select * from Book", Book.class ) + .addScalar( "isbn", String.class ) + .addScalar( "name", String.class ) + .getResultList() + ) + ); } @Test From 709180a4d7d77aa2beef06c169344e8ff46d33e9 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Wed, 21 May 2025 23:42:36 +0200 Subject: [PATCH 2/2] HHH-19383 partial fix to checking of constructor signature NativeQueryConstructorTransformer does not prevent the result type from having multiple constructors, it just rejects a result type with multiple constructors with matching arity --- .../query/sql/internal/NativeQueryImpl.java | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) 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 b751e9331e43..ccacc8d58916 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,7 +5,6 @@ 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; @@ -355,32 +354,43 @@ private void checkResultType(Class resultType, ResultSetMapping resultSetMapp } 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 + // 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 no 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" ); + throw new IllegalArgumentException( + "The return type for a multivalued result set mapping should be Object[], Map, List, or Tuple" + + " or it must have an appropriate constructor" + ); } } } } 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; + // TODO: Only one constructor with the right number of parameters is allowed + // (see NativeQueryConstructorTransformer) so we should validate that + outer: for ( var constructor : resultType.getConstructors() ) { + if ( constructor.getParameterCount() == resultSetMapping.getNumberOfResultBuilders() ) { + final var resultBuilders = resultSetMapping.getResultBuilders(); + final var paramTypes = constructor.getParameterTypes(); + for ( int i = 0; i < resultBuilders.size(); i++ ) { + if ( !constructorParameterMatches( resultBuilders.get( i ), paramTypes[i] ) ) { + continue outer; + } + } + return true; } } - return true; + return false; + } + + private static boolean constructorParameterMatches(ResultBuilder resultBuilder, Class paramType) { + final Class parameterClass = + paramType.isPrimitive() + ? getDescriptorByPrimitiveType( paramType ).getWrapperClass() + : paramType; + return resultBuilder.getJavaType() == parameterClass; } protected void setTupleTransformerForResultType(Class resultClass) {