Skip to content

HHH-19383 - validation of NativeQuery result mappings #10085

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -328,24 +328,35 @@ private void handleExplicitResultSetMapping() {
setTupleTransformerForResultType( resultType );
}
else {
checkResultType( resultType );
checkResultType( resultType, resultSetMapping );
}
}
}

private void checkResultType(Class<R> 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<R> 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:
for ( ResultBuilder resultBuilder : resultSetMapping.getResultBuilders() ) {
final Class rbJavaType = resultBuilder.getJavaType();
if ( !resultType.isAssignableFrom( rbJavaType ) ) {
throw new IllegalArgumentException( "The result set mapping of the typed query doesn't match the declared return type" );
}
}
}
Comment on lines +353 to +359
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are multiple resultBuilders, shouldn't the resultType be Object[]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily, at least that is what I thought?
If the return type is Object[], there's really not much to check since that flag covers everything.
For that reason I initially surrounded that for loop with if ( resultType != Object[].class ) , but it should already be covered by the outer if.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean this should be an error:

s.createNativeQuery("select title, isbn from books", String.class)
     .addScalar("title", String.class)
     .addScalar("isbn", String.class)
     .getResultList();

The result type of that query has to be Object[], not String.

}
}

Expand Down Expand Up @@ -716,6 +727,7 @@ else if ( !isResultTypeAlwaysAllowed( resultType )
else {
mapping = resultSetMapping;
}
checkResultType( resultType, mapping );
return isCacheableQuery()
? getInterpretationCache()
.resolveSelectQueryPlan( selectInterpretationsKey( mapping ), () -> createQueryPlan( mapping ) )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.hibernate.query.NativeQuery;
import org.hibernate.query.Query;
import org.hibernate.query.ResultListTransformer;
import org.hibernate.testing.orm.junit.Jira;
import org.hibernate.testing.orm.junit.JiraGroup;
import org.hibernate.transform.ResultTransformer;
import org.hibernate.transform.Transformers;
import org.hibernate.type.StandardBasicTypes;
Expand All @@ -59,6 +61,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

Expand Down Expand Up @@ -938,6 +941,32 @@ public void testAliasToBeanMap(SessionFactoryScope scope) {
);
}

@Test
@JiraGroup(
{
@Jira("https://hibernate.atlassian.net/browse/HHH-19376"),
@Jira("https://hibernate.atlassian.net/browse/HHH-19383")
}
)
public void testMutateResultSetMapping(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<Person> query = session.createNativeQuery(sql, Person.class);
query.setParameter("name", "Ja%");
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() );
}
);
}

private String buildLongString(int size, char baseChar) {
StringBuilder buff = new StringBuilder();
for( int i = 0; i < size; i++ ) {
Expand Down