From 9ae9810f855daa2a65a589c5a431f9ab47e635b5 Mon Sep 17 00:00:00 2001 From: Anders Wallgren Date: Thu, 9 Nov 2017 20:09:04 -0500 Subject: [PATCH] HHH-12086 - Batch order_inserts: flush during transaction causes incorrect insert ordering and subsequent constraint violation --- .../org/hibernate/engine/spi/ActionQueue.java | 63 +++++++++------ ...ithBidirectionalOneToManyFlushProblem.java | 81 ++++++++++++++++++- 2 files changed, 117 insertions(+), 27 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index ee43a5f27b..e4377a94e8 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -1166,36 +1166,47 @@ public void sort(List insertions) { } } - // 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 ); + boolean stored = false; - // Iterate previous batches and make sure that parent types are before children - // Since the outer loop looks at each batch entry individually, we need to verify that any - // prior batches in the list are not considered children (or have a parent) of the current - // batch. If so, we reordered them. - for ( int j = i - 1; j >= 0; j-- ) { - BatchIdentifier prevBatchIdentifier = latestBatches.get( j ); - if ( prevBatchIdentifier.hasParent( batchIdentifier ) ) { - latestBatches.remove( batchIdentifier ); - latestBatches.add( j, batchIdentifier ); + sort: + do { + // 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 ); + + // Iterate previous batches and make sure that parent types are before children + // Since the outer loop looks at each batch entry individually, we need to verify that any + // prior batches in the list are not considered children (or have a parent) of the current + // batch. If so, we reordered them. + for ( int j = i - 1; j >= 0; j-- ) { + BatchIdentifier prevBatchIdentifier = latestBatches.get( j ); + if ( prevBatchIdentifier.hasParent( batchIdentifier ) && !batchIdentifier.hasParent( prevBatchIdentifier ) ) { + latestBatches.remove( batchIdentifier ); + latestBatches.add( j, batchIdentifier ); + + continue sort; + } + } + + // Iterate next batches and make sure that children types are after parents. + // Since the outer loop looks at each batch entry individually and the prior loop will reorder + // entries as well, we need to look and verify if the current batch is a child of the next + // batch or if the current batch is seen as a parent or child of the next batch. + for ( int j = i + 1; j < latestBatches.size(); j++ ) { + BatchIdentifier nextBatchIdentifier = latestBatches.get( j ); + + if ( batchIdentifier.hasParent( nextBatchIdentifier ) && !nextBatchIdentifier.hasParent( batchIdentifier ) ) { + latestBatches.remove( batchIdentifier ); + latestBatches.add( j, batchIdentifier ); + + continue sort; + } } } - // Iterate next batches and make sure that children types are after parents. - // Since the outer loop looks at each batch entry individually and the prior loop will reorder - // entries as well, we need to look and verify if the current batch is a child of the next - // batch or if the current batch is seen as a parent or child of the next batch. - for ( int j = i + 1; j < latestBatches.size(); j++ ) { - BatchIdentifier nextBatchIdentifier = latestBatches.get( j ); - - if ( batchIdentifier.hasParent( nextBatchIdentifier ) ) { - latestBatches.remove( batchIdentifier ); - latestBatches.add( j, batchIdentifier ); - } - } - } + stored = true; + } while ( !stored ); // Now, rebuild the insertions list. There is a batch for each entry in the name list. for ( BatchIdentifier rootIdentifier : latestBatches ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java b/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java index 968596d992..f1c7fe9ba0 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java +++ b/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java @@ -75,6 +75,55 @@ public void testBatchingWithFlush() { ); } + @Test + @TestForIssue(jiraKey = "HHH-12086") + public void testBatchingWithFlush2() { + doInHibernate( + this::sessionFactory, + session -> { + TopEntity top1 = new TopEntity(); + + session.persist( top1 ); + + // InsertActionSorter#sort is invoked during this flush. + // + // input: [top1] + // output: [top1] + session.flush(); + + MiddleEntity middle1 = new MiddleEntity(); + + middle1.addBottom( new BottomEntity() ); + middle1.addBottom2( new BottomEntity2() ); + top1.addMiddle( middle1 ); + session.persist( middle1 ); + + TopEntity top2 = new TopEntity(); + + session.persist( top2 ); + + MiddleEntity middle2 = new MiddleEntity(); + + middle2.addBottom( new BottomEntity() ); + middle2.addBottom2( new BottomEntity2() ); + top2.addMiddle( middle2 ); + session.persist( middle2 ); + + session.persist(new TopEntity()); + + // 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" ); @@ -83,7 +132,7 @@ protected void addSettings(Map settings) { @Override protected Class[] getAnnotatedClasses() { - return new Class[] { TopEntity.class, MiddleEntity.class, BottomEntity.class }; + return new Class[] { TopEntity.class, MiddleEntity.class, BottomEntity.class, BottomEntity2.class }; } @Entity(name = "BottomEntity") @@ -105,6 +154,25 @@ public static class BottomEntity { private MiddleEntity middle; } + @Entity(name = "BottomEntity2") + public static class BottomEntity2 { + + @Column(nullable = false) + @GeneratedValue( + strategy = SEQUENCE, + generator = "ID" + ) + @Id + @SequenceGenerator( + name = "ID", + sequenceName = "BOTTOM2_SEQ" + ) + private Long id; + + @ManyToOne(optional = false) + private MiddleEntity middle; + } + @Entity(name = "MiddleEntity") public static class MiddleEntity { @@ -129,10 +197,21 @@ public static class MiddleEntity { ) private List bottoms = new ArrayList<>(); + @OneToMany( + cascade = PERSIST, + mappedBy = "middle" + ) + private List bottom2s = new ArrayList<>(); + private void addBottom(BottomEntity bottom) { bottoms.add( bottom ); bottom.middle = this; } + + private void addBottom2(BottomEntity2 bottom2) { + bottom2s.add( bottom2 ); + bottom2.middle = this; + } } @Entity(name = "TopEntity")