Change the way persisting works as needed for list semantics and throw

proper error on null list index

Author: beikov

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

@@ -13,6 +13,7 @@
 import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.internal.log.LoggingHelper;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
+import org.hibernate.query.SemanticException;
 import org.hibernate.query.spi.NavigablePath;
 import org.hibernate.sql.results.graph.DomainResultAssembler;
 import org.hibernate.sql.results.graph.FetchParentAccess;
@@ -66,7 +67,11 @@ protected void readCollectionRow(
 			CollectionKey collectionKey,
 			List<Object> loadingState,
 			RowProcessingState rowProcessingState) {
-		int index = listIndexAssembler.assemble( rowProcessingState );
+		final Integer indexValue = listIndexAssembler.assemble( rowProcessingState );
+		if ( indexValue == null ) {
+			throw new SemanticException( "Illegal null index value encountered while reading: " + getCollectionAttributeMapping().getNavigableRole() );
+		}
+		int index = indexValue;
 
 		if ( indexBase != 0 ) {
 			index -= indexBase;

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

@@ -13,6 +13,7 @@
 import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.internal.log.LoggingHelper;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
+import org.hibernate.query.SemanticException;
 import org.hibernate.query.spi.NavigablePath;
 import org.hibernate.sql.results.graph.DomainResultAssembler;
 import org.hibernate.sql.results.graph.FetchParentAccess;
@@ -68,7 +69,11 @@ protected void readCollectionRow(
 			CollectionKey collectionKey,
 			List<Object> loadingState,
 			RowProcessingState rowProcessingState) {
-		int index = listIndexAssembler.assemble( rowProcessingState );
+		final Integer indexValue = listIndexAssembler.assemble( rowProcessingState );
+		if ( indexValue == null ) {
+			throw new SemanticException( "Illegal null index value encountered while reading: " + getCollectionAttributeMapping().getNavigableRole() );
+		}
+		int index = indexValue;
 
 		if ( listIndexBase != 0 ) {
 			index -= listIndexBase;

hibernate-core/src/test/java/org/hibernate/orm/test/ops/OneToManyMappedByCascadeDeleteTest.java

@@ -55,6 +55,8 @@ public void testRemoveCascadeDelete(SessionFactoryScope scope) {
 			child.setId( 2 );
 			child.setParent( parent );
 			s.persist( child );
+
+			parent.children.add( child );
 		} );
 
 		getStatistics( scope ).clear();

hibernate-core/src/test/java/org/hibernate/orm/test/ops/OneToManyMappedByOrderColumnTest.java

@@ -0,0 +1,125 @@
+/*
+ * 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.ops;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.hibernate.annotations.Cascade;
+import org.hibernate.annotations.CascadeType;
+import org.hibernate.query.SemanticException;
+import org.hibernate.stat.spi.StatisticsImplementor;
+
+import org.hibernate.testing.orm.junit.DomainModel;
+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.Assertions;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.Id;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.OneToMany;
+import jakarta.persistence.OrderColumn;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+
+@SessionFactory
+@DomainModel(annotatedClasses = {
+		OneToManyMappedByOrderColumnTest.Parent.class,
+		OneToManyMappedByOrderColumnTest.Child.class
+})
+public class OneToManyMappedByOrderColumnTest {
+
+	@AfterEach
+	public void cleanup(SessionFactoryScope scope) {
+		scope.inTransaction( s -> {
+			s.createMutationQuery( "delete from child" ).executeUpdate();
+			s.createMutationQuery( "delete from parent" ).executeUpdate();
+		} );
+	}
+
+	@Test
+	public void testReadNullIndex(SessionFactoryScope scope) {
+		scope.inTransaction( s -> {
+			Parent parent = new Parent();
+			parent.setId( 1 );
+			s.persist( parent );
+
+			Child child = new Child();
+			child.setId( 2 );
+			child.setParent( parent );
+			s.persist( child );
+		} );
+
+		scope.inTransaction( s -> {
+			try {
+				s.get( Parent.class, 1 );
+				Assertions.fail( "Expected to fail because list index is null" );
+			}
+			catch (SemanticException ex) {
+				Assertions.assertTrue( ex.getMessage().contains( "children" ) );
+			}
+		} );
+	}
+
+	@Entity(name = "parent")
+	public static class Parent {
+
+		@Id
+		private Integer id;
+		@OrderColumn(name = "list_idx")
+		@OneToMany(targetEntity = Child.class, mappedBy = "parent", fetch = FetchType.EAGER)
+		@Cascade(CascadeType.DELETE)
+		private List<Child> children = new ArrayList<>();
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public List<Child> getChildren() {
+			return children;
+		}
+
+		public void setChildren(List<Child> children) {
+			this.children = children;
+		}
+	}
+
+	@Entity(name = "child")
+	public static class Child {
+
+		@Id
+		private Integer id;
+		@ManyToOne(targetEntity = Parent.class, fetch = FetchType.LAZY)
+		private Parent parent;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public Parent getParent() {
+			return parent;
+		}
+
+		public void setParent(Parent parent) {
+			this.parent = parent;
+		}
+	}
+}