HHH-18493 Resolving already initialized collection elements leads to assertion error

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java

@@ -6,6 +6,7 @@
  */
 package org.hibernate.sql.results.graph.collection.internal;
 
+import java.lang.reflect.Array;
 import java.util.Iterator;
 import java.util.List;
 import java.util.function.BiConsumer;
@@ -131,9 +132,11 @@ public class ArrayInitializer extends AbstractImmediateCollectionInitializer<Abs
 		final Initializer<?> initializer = elementAssembler.getInitializer();
 		if ( initializer != null ) {
 			final RowProcessingState rowProcessingState = data.getRowProcessingState();
-			final Iterator iter = getCollectionInstance( data ).elements();
+			final Integer index = listIndexAssembler.assemble( rowProcessingState );
-			while ( iter.hasNext() ) {
+			if ( index != null ) {
-				initializer.resolveInstance( iter.next(), rowProcessingState );
+				final PersistentArrayHolder<?> arrayHolder = getCollectionInstance( data );
+				assert arrayHolder != null;
+				initializer.resolveInstance( Array.get( arrayHolder.getArray(), index ), rowProcessingState );
 			}
 		}
 	}

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java

@@ -123,19 +123,7 @@ public class BagInitializer extends AbstractImmediateCollectionInitializer<Abstr
 	protected void resolveInstanceSubInitializers(ImmediateCollectionInitializerData data) {
 		final Initializer<?> initializer = elementAssembler.getInitializer();
 		if ( initializer != null ) {
-			final RowProcessingState rowProcessingState = data.getRowProcessingState();
+			initializer.resolveKey( data.getRowProcessingState() );
-			final PersistentCollection<?> persistentCollection = getCollectionInstance( data );
-			assert persistentCollection != null;
-			if ( persistentCollection instanceof PersistentBag<?> ) {
-				for ( Object element : ( (PersistentBag<?>) persistentCollection ) ) {
-					initializer.resolveInstance( element, rowProcessingState );
-				}
-			}
-			else {
-				for ( Object element : ( (PersistentIdentifierBag<?>) persistentCollection ) ) {
-					initializer.resolveInstance( element, rowProcessingState );
-				}
-			}
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java

@@ -123,10 +123,11 @@ public class ListInitializer extends AbstractImmediateCollectionInitializer<Abst
 		final Initializer<?> initializer = elementAssembler.getInitializer();
 		if ( initializer != null ) {
 			final RowProcessingState rowProcessingState = data.getRowProcessingState();
-			final PersistentList<?> list = getCollectionInstance( data );
+			final Integer index = listIndexAssembler.assemble( rowProcessingState );
-			assert list != null;
+			if ( index != null ) {
-			for ( Object element : list ) {
+				final PersistentList<?> list = getCollectionInstance( data );
-				initializer.resolveInstance( element, rowProcessingState );
+				assert list != null;
+				initializer.resolveInstance( list.get( index ), rowProcessingState );
 			}
 		}
 	}

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java

@@ -121,17 +121,27 @@ public class MapInitializer extends AbstractImmediateCollectionInitializer<Abstr
 	protected void resolveInstanceSubInitializers(ImmediateCollectionInitializerData data) {
 		final Initializer<?> keyInitializer = mapKeyAssembler.getInitializer();
 		final Initializer<?> valueInitializer = mapValueAssembler.getInitializer();
-		if ( keyInitializer != null || valueInitializer != null ) {
+		final RowProcessingState rowProcessingState = data.getRowProcessingState();
-			final RowProcessingState rowProcessingState = data.getRowProcessingState();
+		if ( keyInitializer == null && valueInitializer != null ) {
-			final PersistentMap<?, ?> map = getCollectionInstance( data );
+			// For now, we only support resolving the value initializer instance when keys have no initializer,
-			assert map != null;
+			// though we could also support map keys with an initializer given that the initialized java type:
-			for ( Map.Entry<?, ?> entry : map.entrySet() ) {
+			// * is an entity that uses only the primary key in equals/hashCode.
-				if ( keyInitializer != null ) {
+			//   If the primary key type is an embeddable, the next condition must hold for that
-					keyInitializer.resolveInstance( entry.getKey(), rowProcessingState );
+			// * or is an embeddable that has no initializers for fields being used in the equals/hashCode
-				}
+			//   which violate this same requirement (recursion)
-				if ( valueInitializer != null ) {
+			final Object key = mapKeyAssembler.assemble( rowProcessingState );
-					valueInitializer.resolveInstance( entry.getValue(), rowProcessingState );
+			if ( key != null ) {
-				}
+				final PersistentMap<?, ?> map = getCollectionInstance( data );
+				assert map != null;
+				valueInitializer.resolveInstance( map.get( key ), rowProcessingState );
+			}
+		}
+		else {
+			if ( keyInitializer != null ) {
+				keyInitializer.resolveKey( rowProcessingState );
+			}
+			if ( valueInitializer != null ) {
+				valueInitializer.resolveKey( rowProcessingState );
 			}
 		}
 	}

hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java

@@ -97,12 +97,7 @@ public class SetInitializer extends AbstractImmediateCollectionInitializer<Abstr
 	protected void resolveInstanceSubInitializers(ImmediateCollectionInitializerData data) {
 		final Initializer<?> initializer = elementAssembler.getInitializer();
 		if ( initializer != null ) {
-			final RowProcessingState rowProcessingState = data.getRowProcessingState();
+			initializer.resolveKey( data.getRowProcessingState() );
-			final PersistentSet<?> set = getCollectionInstance( data );
-			assert set != null;
-			for ( Object element : set ) {
-				initializer.resolveInstance( element, rowProcessingState );
-			}
 		}
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/ReloadMultipleCollectionElementsTest.java

@@ -0,0 +1,307 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.orm.test.inheritance.join;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.hibernate.Hibernate;
+
+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.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.CascadeType;
+import jakarta.persistence.Column;
+import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.GenerationType;
+import jakarta.persistence.Id;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.ManyToMany;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.OneToMany;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@DomainModel( annotatedClasses = {
+		ReloadMultipleCollectionElementsTest.Flight.class,
+		ReloadMultipleCollectionElementsTest.Ticket.class,
+		ReloadMultipleCollectionElementsTest.Customer.class,
+		ReloadMultipleCollectionElementsTest.Company.class
+} )
+@SessionFactory
+@Jira("https://hibernate.atlassian.net/browse/HHH-18493")
+public class ReloadMultipleCollectionElementsTest {
+
+	@BeforeEach
+	public void setup(SessionFactoryScope scope) {
+		scope.inTransaction( s -> {
+			Flight f2 = new Flight();
+			f2.setId(1L);
+			f2.setName("Flight two");
+
+			Company us = new Company();
+			us.setName("Airline 2");
+			f2.setCompany(us);
+
+			Customer c1 = new Customer();
+			c1.setId( 1L );
+			c1.setName("Tom");
+
+			Customer c2 = new Customer();
+			c2.setId( 2L );
+			c2.setName("Pete");
+
+			Ticket t1 = new Ticket();
+			t1.setId(1L);
+			t1.setCustomer(c2);
+			t1.setNumber( "123" );
+
+			Ticket t2 = new Ticket();
+			t2.setId(2L);
+			t2.setCustomer(c2);
+			t2.setNumber( "456" );
+
+			f2.setCustomers(Set.of(c1, c2));
+
+			s.persist(c1);
+			s.persist(c2);
+			s.persist(f2);
+			s.persist(t1);
+			s.persist(t2);
+		} );
+	}
+
+	@AfterEach
+	public void cleanup(SessionFactoryScope scope) {
+		scope.inTransaction( s -> {
+			s.createMutationQuery( "delete from Ticket" ).executeUpdate();
+			s.createMutationQuery( "delete from Flight" ).executeUpdate();
+			s.createMutationQuery( "delete from Customer" ).executeUpdate();
+			s.createMutationQuery( "delete from Company" ).executeUpdate();
+		} );
+	}
+
+	@Test
+	public void testResolveElementOfInitializedCollection(SessionFactoryScope scope) {
+		scope.inTransaction( s -> {
+			// First load all customers with their flights collection and corresponding customers
+			List<Customer> customers = s.createQuery(
+					"from Customer c join fetch c.flights f join fetch f.customers order by c.id",
+					Customer.class
+			).getResultList();
+			assertEquals( 2, customers.size() );
+			assertFalse( Hibernate.isInitialized( customers.get( 0 ).getTickets() ) );
+			assertFalse( Hibernate.isInitialized( customers.get( 1 ).getTickets() ) );
+
+			// Then load all flights with their customers collection, but in addition, also the customers tickets
+			// This will trigger resolveInstance(Object, Data) with the existing collection and will
+			// fetch tickets data into existing customers
+			s.createQuery( "from Flight f join fetch f.customers c left join fetch c.tickets", Flight.class ).getResultList();
+
+			assertTrue( Hibernate.isInitialized( customers.get( 0 ).getTickets() ) );
+			assertTrue( Hibernate.isInitialized( customers.get( 1 ).getTickets() ) );
+			assertEquals( 0, customers.get( 0 ).getTickets().size() );
+			assertEquals( 2, customers.get( 1 ).getTickets().size() );
+		} );
+	}
+
+	@Entity( name = "Flight" )
+	public static class Flight {
+		private Long id;
+		private String name;
+		private Company company;
+		private Set<Customer> customers;
+
+		public Flight() {
+		}
+
+		@Id
+		@Column(name = "flight_id")
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long long1) {
+			id = long1;
+		}
+
+		@Column(updatable = false, name = "flight_name", nullable = false, length = 50)
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String string) {
+			name = string;
+		}
+
+
+		@ManyToOne(cascade = {CascadeType.ALL})
+		@JoinColumn(name = "comp_id")
+		public Company getCompany() {
+			return company;
+		}
+
+		public void setCompany(Company company) {
+			this.company = company;
+		}
+
+		@ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, fetch = FetchType.EAGER)
+		public Set<Customer> getCustomers() {
+			return customers;
+		}
+
+		public void setCustomers(Set<Customer> customers) {
+			this.customers = customers;
+		}
+	}
+
+	@Entity( name = "Ticket" )
+	public static class Ticket {
+
+		Long id;
+		String number;
+		Customer customer;
+
+		public Ticket() {
+		}
+
+		@Id
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long long1) {
+			id = long1;
+		}
+
+		@Column(name = "nr")
+		public String getNumber() {
+			return number;
+		}
+
+		public void setNumber(String string) {
+			number = string;
+		}
+
+		@ManyToOne(cascade = CascadeType.ALL)
+		@JoinColumn(name = "CUSTOMER_ID")
+		public Customer getCustomer() {
+			return customer;
+		}
+
+		public void setCustomer(Customer customer) {
+			this.customer = customer;
+		}
+	}
+
+	@Entity( name = "Customer" )
+	public static class Customer {
+		private Long id;
+		private String name;
+		private String address;
+		private Set<Ticket> tickets;
+		private Set<Flight> flights;
+
+		// Address address;
+
+		public Customer() {
+		}
+
+		@Id
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+
+		public String getAddress() {
+			return address;
+		}
+
+		public void setAddress(String address) {
+			this.address = address;
+		}
+
+		@OneToMany(cascade = CascadeType.ALL, mappedBy = "customer")
+		public Set<Ticket> getTickets() {
+			return tickets;
+		}
+
+		public void setTickets(Set<Ticket> tickets) {
+			this.tickets = tickets;
+		}
+
+		@ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, mappedBy = "customers")
+		public Set<Flight> getFlights() {
+			return flights;
+		}
+
+		public void setFlights(Set<Flight> flights) {
+			this.flights = flights;
+		}
+	}
+
+	@Entity( name = "Company" )
+	public static class Company {
+
+		private Long id;
+		private String name;
+		private Set<Flight> flights = new HashSet<Flight>();
+
+		public Company() {
+		}
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.AUTO)
+		@Column(name = "comp_id")
+		public Long getId() {
+			return id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setId(Long newId) {
+			id = newId;
+		}
+
+		public void setName(String string) {
+			name = string;
+		}
+
+		@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, mappedBy = "company")
+		@Column(name = "flight_id")
+		public Set<Flight> getFlights() {
+			return flights;
+		}
+
+		public void setFlights(Set<Flight> flights) {
+			this.flights = flights;
+		}
+	}
+}