HHH-12074 - order_inserts: flush during transaction causes incorrect insert ordering and subsequent constraint violation

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

@@ -1024,11 +1024,21 @@ public class ActionQueue {
 
 			private Set<String> childEntityNames = new HashSet<>( );
 
+			private BatchIdentifier parent;
+
 			BatchIdentifier(String entityName, String rootEntityName) {
 				this.entityName = entityName;
 				this.rootEntityName = rootEntityName;
 			}
 
+			public BatchIdentifier getParent() {
+				return parent;
+			}
+
+			public void setParent(BatchIdentifier parent) {
+				this.parent = parent;
+			}
+
 			@Override
 			public boolean equals(Object o) {
 				if ( this == o ) {
@@ -1070,6 +1080,18 @@ public class ActionQueue {
 			boolean hasAnyChildEntityNames(BatchIdentifier batchIdentifier) {
 				return childEntityNames.contains( batchIdentifier.getEntityName() );
 			}
+
+			/**
+			 * Check if the this {@link BatchIdentifier} has a parent or grand parent
+			 * matching the given {@link BatchIdentifier reference.
+			 *
+			 * @param batchIdentifier {@link BatchIdentifier} reference
+			 *
+			 * @return This {@link BatchIdentifier} has a parent matching the given {@link BatchIdentifier reference
+			 */
+			boolean hasParent(BatchIdentifier batchIdentifier) {
+				return parent == batchIdentifier || parent != null && parent.hasParent( batchIdentifier );
+			}
 		}
 
 		// the mapping of entity names to their latest batch numbers.
@@ -1095,7 +1117,11 @@ public class ActionQueue {
 			for ( AbstractEntityInsertAction action : insertions ) {
 				BatchIdentifier batchIdentifier = new BatchIdentifier(
 						action.getEntityName(),
-						action.getSession().getFactory().getMetamodel().entityPersister( action.getEntityName() ).getRootEntityName()
+						action.getSession()
+								.getFactory()
+								.getMetamodel()
+								.entityPersister( action.getEntityName() )
+								.getRootEntityName()
 				);
 
 				// the entity associated with the current action.
@@ -1114,7 +1140,34 @@ public class ActionQueue {
 			}
 			insertions.clear();
 
-			// Examine each entry in the batch list, sorting them based on parent/child associations.
+			// Examine each entry in the batch list, and build the dependency graph.
+			for ( int i = 0; i < latestBatches.size(); i++ ) {
+				BatchIdentifier batchIdentifier = latestBatches.get( i );
+
+				for ( int j = i - 1; j >= 0; j-- ) {
+					BatchIdentifier prevBatchIdentifier = latestBatches.get( j );
+					if ( prevBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) {
+						prevBatchIdentifier.parent = batchIdentifier;
+					}
+					if ( batchIdentifier.hasAnyChildEntityNames( prevBatchIdentifier ) ) {
+						prevBatchIdentifier.parent = batchIdentifier;
+					}
+				}
+
+				for ( int j = i + 1; j < latestBatches.size(); j++ ) {
+					BatchIdentifier nextBatchIdentifier = latestBatches.get( j );
+
+					if ( nextBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) {
+						nextBatchIdentifier.parent = batchIdentifier;
+					}
+					if ( batchIdentifier.hasAnyChildEntityNames( nextBatchIdentifier ) ) {
+						nextBatchIdentifier.parent = batchIdentifier;
+					}
+				}
+			}
+
+			// Examine each entry in the batch list, sorting them based on parent/child association
+			// as depicted by the dependency graph.
 			for ( int i = 0; i < latestBatches.size(); i++ ) {
 				BatchIdentifier batchIdentifier = latestBatches.get( i );
 
@@ -1124,7 +1177,7 @@ public class ActionQueue {
 				// batch.  If so, we reordered them.
 				for ( int j = i - 1; j >= 0; j-- ) {
 					BatchIdentifier prevBatchIdentifier = latestBatches.get( j );
-					if ( prevBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) {
+					if ( prevBatchIdentifier.hasParent( batchIdentifier ) ) {
 						latestBatches.remove( batchIdentifier );
 						latestBatches.add( j, batchIdentifier );
 					}
@@ -1137,19 +1190,14 @@ public class ActionQueue {
 				for ( int j = i + 1; j < latestBatches.size(); j++ ) {
 					BatchIdentifier nextBatchIdentifier = latestBatches.get( j );
 
-					final boolean nextBatchHasChild = nextBatchIdentifier.hasAnyChildEntityNames( batchIdentifier );
+					if ( batchIdentifier.hasParent( nextBatchIdentifier ) ) {
-					final boolean batchHasChild = batchIdentifier.hasAnyChildEntityNames( nextBatchIdentifier );
-					final boolean batchHasParent = batchIdentifier.hasAnyParentEntityNames( nextBatchIdentifier );
-
-					// Take care of unidirectional @OneToOne associations but exclude bidirectional @ManyToMany
-					if ( ( nextBatchHasChild && !batchHasChild ) || batchHasParent ) {
 						latestBatches.remove( batchIdentifier );
 						latestBatches.add( j, batchIdentifier );
 					}
 				}
 			}
 
-			// now rebuild the insertions list. There is a batch for each entry in the name list.
+			// Now, rebuild the insertions list. There is a batch for each entry in the name list.
 			for ( BatchIdentifier rootIdentifier : latestBatches ) {
 				List<AbstractEntityInsertAction> batch = actionBatches.get( rootIdentifier );
 				insertions.addAll( batch );

hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java

@@ -0,0 +1,161 @@
+/*
+ * 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.test.insertordering;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+import javax.persistence.OneToMany;
+import javax.persistence.SequenceGenerator;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static javax.persistence.CascadeType.PERSIST;
+import static javax.persistence.GenerationType.SEQUENCE;
+import static org.hibernate.cfg.AvailableSettings.ORDER_INSERTS;
+import static org.hibernate.cfg.AvailableSettings.STATEMENT_BATCH_SIZE;
+import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
+
+@TestForIssue(jiraKey = "HHH-12074")
+public class InsertOrderingWithBidirectionalOneToManyFlushProblem
+		extends BaseNonConfigCoreFunctionalTestCase {
+
+	@Test
+	public void testBatchingWithFlush() {
+		doInHibernate(
+			this::sessionFactory,
+			session -> {
+				Top top1 = new Top();
+
+				session.persist( top1 );
+
+				// InsertActionSorter#sort is invoked during this flush.
+				//
+				// input: [top1]
+				// output: [top1]
+				session.flush();
+
+				Middle middle1 = new Middle();
+
+				middle1.addBottom( new Bottom() );
+				top1.addMiddle( middle1 );
+				session.persist( middle1 );
+
+				Top top2 = new Top();
+
+				session.persist( top2 );
+
+				Middle middle2 = new Middle();
+
+				middle2.addBottom( new Bottom() );
+				top2.addMiddle( middle2 );
+				session.persist( middle2 );
+
+				// InsertActionSorter#sort is invoked during this flush
+				//
+				// input: [middle1,bottom1,top2,middle2,bottom2] output:
+				// [middle1,middle2,bottom1,bottom2,top2]
+				//
+				// This ordering causes a constraint violation during the flush
+				// when the attempt to insert middle2 before top2 is made.
+				//
+				// correct ordering is: [top2,middle1,middle2,bottom1,bottom2]
+			}
+		);
+	}
+
+	@Override
+	protected void addSettings(Map settings) {
+		settings.put( ORDER_INSERTS, "true" );
+		settings.put( STATEMENT_BATCH_SIZE, "10" );
+	}
+
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class[] { Top.class, Middle.class, Bottom.class };
+	}
+
+	@Entity(name = "Bottom")
+	public static class Bottom {
+
+		@Column(nullable = false)
+		@GeneratedValue(
+				strategy = SEQUENCE,
+				generator = "ID"
+		)
+		@Id
+		@SequenceGenerator(
+				name = "ID",
+				sequenceName = "BOTTOM_SEQ"
+		)
+		private Long id;
+
+		@ManyToOne(optional = false)
+		private Middle middle;
+	}
+
+	@Entity(name = "Middle")
+	public static class Middle {
+
+		@Column(nullable = false)
+		@GeneratedValue(
+				strategy = SEQUENCE,
+				generator = "ID"
+		)
+		@Id
+		@SequenceGenerator(
+				name = "ID",
+				sequenceName = "MIDDLE_SEQ"
+		)
+		private Long id;
+
+		@ManyToOne(optional = false)
+		private Top top;
+
+		@OneToMany(
+				cascade = PERSIST,
+				mappedBy = "middle"
+		)
+		private List<Bottom> bottoms = new ArrayList<>();
+
+		private void addBottom(Bottom bottom) {
+			bottoms.add( bottom );
+			bottom.middle = this;
+		}
+	}
+
+	@Entity(name = "Top")
+	public static class Top {
+
+		@Column(nullable = false)
+		@GeneratedValue(
+				strategy = SEQUENCE,
+				generator = "ID"
+		)
+		@Id
+		@SequenceGenerator(
+				name = "ID",
+				sequenceName = "TOP_SEQ"
+		)
+		private Long id;
+
+		@OneToMany(mappedBy = "top")
+		private List<Middle> middles = new ArrayList<>();
+
+		void addMiddle(Middle middle) {
+			middles.add( middle );
+			middle.top = this;
+		}
+	}
+}