HHH-10557 fix @Loader applied to a collection

the issue here is we have no @CollectionResult for annotation-based result set mappings

documentation/src/test/java/org/hibernate/userguide/sql/CollectionLoaderTest.java

@@ -16,11 +16,9 @@ import jakarta.persistence.ElementCollection;
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.Id;
-import jakarta.persistence.NamedNativeQueries;
 import jakarta.persistence.NamedNativeQuery;
 
 import org.hibernate.annotations.Loader;
-import org.hibernate.annotations.ResultCheckStyle;
 import org.hibernate.annotations.SQLDelete;
 import org.hibernate.annotations.SQLDeleteAll;
 import org.hibernate.annotations.SQLInsert;
@@ -37,6 +35,7 @@ import org.hibernate.testing.TestForIssue;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hibernate.annotations.ResultCheckStyle.COUNT;
 import static org.hibernate.cfg.AvailableSettings.DEFAULT_LIST_SEMANTICS;
 import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
 import static org.junit.Assert.assertEquals;
@@ -71,9 +70,11 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase {
 			session.doWork(connection -> {
 				try(Statement statement = connection.createStatement();) {
 					statement.executeUpdate(String.format( "ALTER TABLE person %s valid %s",
-														   getDialect().getAddColumnString(), ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect())));
+														   getDialect().getAddColumnString(),
+							ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect())));
 					statement.executeUpdate(String.format( "ALTER TABLE Person_phones %s valid %s",
-														   getDialect().getAddColumnString(), ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect())));
+														   getDialect().getAddColumnString(),
+							ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect())));
 				}
 			});
 		});
@@ -91,55 +92,42 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase {
 			return person;
 		});
 
-		try {
+		doInJPA(this::entityManagerFactory, entityManager -> {
-
+			Long postId = _person.getId();
-			doInJPA(this::entityManagerFactory, entityManager -> {
+			Person person = entityManager.find(Person.class, postId);
-				Long postId = _person.getId();
+			assertEquals(2, person.getPhones().size());
-				Person person = entityManager.find(Person.class, postId);
+			person.getPhones().remove(0);
-				assertEquals(2, person.getPhones().size());
+			person.setName("Mr. John Doe");
-				person.getPhones().remove(0);
+		});
-				person.setName("Mr. John Doe");
-			});
 
 
-			doInJPA(this::entityManagerFactory, entityManager -> {
+		doInJPA(this::entityManagerFactory, entityManager -> {
-				Long postId = _person.getId();
+			Long postId = _person.getId();
-				Person person = entityManager.find(Person.class, postId);
+			Person person = entityManager.find(Person.class, postId);
-				assertEquals(1, person.getPhones().size());
+			assertEquals(1, person.getPhones().size());
-			});
+		});
-		}
-		catch (Exception e) {
-			log.error("Throws NullPointerException because the bag is not initialized by the @Loader");
-		}
 	}
 
 
 	//tag::sql-custom-crud-example[]
 	@Entity(name = "Person")
-	@SQLInsert(
+	@SQLInsert(sql = "INSERT INTO person (name, id, valid) VALUES (?, ?, true) ", check = COUNT)
-		sql = "INSERT INTO person (name, id, valid) VALUES (?, ?, true) ",
+	@SQLUpdate(sql = "UPDATE person SET name = ? where id = ? ")
-		check = ResultCheckStyle.COUNT
+	@SQLDelete(sql = "UPDATE person SET valid = false WHERE id = ? ")
-	)
-	@SQLUpdate(
-		sql = "UPDATE person SET name = ? where id = ? ")
-	@SQLDelete(
-		sql = "UPDATE person SET valid = false WHERE id = ? ")
 	@Loader(namedQuery = "find_valid_person")
-	@NamedNativeQueries({
+	@NamedNativeQuery(
-		@NamedNativeQuery(
+		name = "find_valid_person",
-			name = "find_valid_person",
+		query = "SELECT id, name " +
-			query = "SELECT id, name " +
+				"FROM person " +
-					"FROM person " +
+				"WHERE id = ? and valid = true",
-					"WHERE id = ? and valid = true",
+		resultClass = Person.class
-			resultClass = Person.class
+	)
-		),
+	@NamedNativeQuery(
-		@NamedNativeQuery(
+		name = "find_valid_phones",
-			name = "find_valid_phones",
+		query = "SELECT phones " +
-			query = "SELECT person_id, phones " +
+				"FROM Person_phones " +
-					"FROM Person_phones " +
+				"WHERE person_id = ? and valid = true "
-					"WHERE person_id = ? and valid = true "
+	)
-		)
-	})
 	public static class Person {
 
 		@Id
@@ -149,10 +137,8 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase {
 		private String name;
 
 		@ElementCollection
-		@SQLInsert(
+		@SQLInsert(sql = "INSERT INTO person_phones (person_id, phones, valid) VALUES (?, ?, true) ")
-			sql = "INSERT INTO person_phones (person_id, phones, valid) VALUES (?, ?, true) ")
+		@SQLDeleteAll(sql = "UPDATE person_phones SET valid = false WHERE person_id = ?")
-		@SQLDeleteAll(
-			sql = "UPDATE person_phones SET valid = false WHERE person_id = ?")
 		@Loader(namedQuery = "find_valid_phones")
 		private List<String> phones = new ArrayList<>();
 

hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java

@@ -231,8 +231,8 @@ public final class CollectionEntry implements Serializable {
 		loadedKey = getCurrentKey();
 		setLoadedPersister( getCurrentPersister() );
 
-		boolean resnapshot = collection.wasInitialized() &&
+		boolean resnapshot = collection.wasInitialized()
-				( isDoremove() || isDorecreate() || isDoupdate() );
+				&&  ( isDoremove() || isDorecreate() || isDoupdate() );
 		if ( resnapshot ) {
 			snapshot = loadedPersister == null || !loadedPersister.isMutable() ?
 					null :

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderNamedQuery.java

@@ -8,15 +8,19 @@ package org.hibernate.loader.ast.internal;
 
 import org.hibernate.FlushMode;
 import org.hibernate.collection.spi.PersistentCollection;
+import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.loader.ast.spi.CollectionLoader;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.persister.collection.CollectionPersister;
+import org.hibernate.query.QueryTypeMismatchException;
 import org.hibernate.query.named.NamedQueryMemento;
 import org.hibernate.query.spi.QueryImplementor;
 
 import jakarta.persistence.Parameter;
 
+import java.util.List;
+
 /**
  * @author Steve Ebersole
  */
@@ -36,10 +40,31 @@ public class CollectionLoaderNamedQuery implements CollectionLoader {
 
 	@Override
 	public PersistentCollection<?> load(Object key, SharedSessionContractImplementor session) {
-		final QueryImplementor<PersistentCollection<?>> query = namedQueryMemento.toQuery( session );
+		final QueryImplementor<?> query = namedQueryMemento.toQuery( session );
 		//noinspection unchecked
 		query.setParameter( (Parameter<Object>) query.getParameters().iterator().next(), key );
 		query.setHibernateFlushMode( FlushMode.MANUAL );
-		return query.getResultList().get( 0 );
+		final List<?> resultList = query.getResultList();
+		// TODO: we need a good way to inspect the query itself to see what it returns
+		if ( !resultList.isEmpty() && resultList.get(0) instanceof PersistentCollection<?> ) {
+			// in hbm.xml files we have the <load-collection/> element
+			return (PersistentCollection<?>) resultList.get(0);
+		}
+		else {
+			// using annotations we have no way to specify a @CollectionResult
+			final CollectionKey collectionKey = new CollectionKey( persister, key );
+			final PersistentCollection<?> collection = session.getPersistenceContextInternal().getCollection( collectionKey );
+			for ( Object element : resultList ) {
+				if ( element != null && !persister.getElementType().getReturnedClass().isInstance( element ) ) {
+					throw new QueryTypeMismatchException( "Collection loader for '" + persister.getRole()
+							+ "' returned an instance of '" + element.getClass().getName() + "'" );
+				}
+			}
+			collection.beforeInitialize( persister, resultList.size() );
+			collection.injectLoadedState( getLoadable(), resultList );
+			collection.afterInitialize();
+			session.getPersistenceContextInternal().getCollectionEntry( collection ).postInitialize( collection );
+			return collection;
+		}
 	}
 }

hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMappingImpl.java

@@ -360,7 +360,7 @@ public class ResultSetMappingImpl implements ResultSetMapping {
 					&& Objects.equals( legacyFetchBuilders, that.legacyFetchBuilders );
 		}
 		else {
-			return !that.isDynamic && mappingIdentifier.equals( that.mappingIdentifier );
+			return !that.isDynamic && mappingIdentifier != null && mappingIdentifier.equals( that.mappingIdentifier );
 		}
 	}
 }