From a26b19d93d347de28b4af042f665dfca0a1ef677 Mon Sep 17 00:00:00 2001 From: Jan Schatteman Date: Mon, 27 Mar 2023 16:49:56 +0200 Subject: [PATCH] HHH-16386 - Disable batching for dynamic-insert and dynamic-update Signed-off-by: Jan Schatteman --- .../entity/mutation/InsertCoordinator.java | 8 +- .../mutation/UpdateCoordinatorStandard.java | 10 +- ...atchedMultiTableDynamicStatementTests.java | 181 +++++++++++++++++ .../DynamicMutationsAndBatchingTest.java | 182 ++++++++++++++++++ 4 files changed, 372 insertions(+), 9 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchedMultiTableDynamicStatementTests.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/batch/DynamicMutationsAndBatchingTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java index 424a7c39fd..2631a1a4c9 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java @@ -152,7 +152,7 @@ public class InsertCoordinator extends AbstractMutationCoordinator { final TableInclusionChecker tableInclusionChecker = getTableInclusionChecker( insertValuesAnalysis ); - final MutationExecutor mutationExecutor = executor( session, staticInsertGroup ); + final MutationExecutor mutationExecutor = executor( session, staticInsertGroup, false ); decomposeForInsert( mutationExecutor, @@ -271,7 +271,7 @@ public class InsertCoordinator extends AbstractMutationCoordinator { final boolean[] insertability = getPropertiesToInsert( values ); final MutationOperationGroup insertGroup = generateDynamicInsertSqlGroup( insertability ); - final MutationExecutor mutationExecutor = executor( session, insertGroup ); + final MutationExecutor mutationExecutor = executor( session, insertGroup, true ); final InsertValuesAnalysis insertValuesAnalysis = new InsertValuesAnalysis( entityPersister(), values ); @@ -301,11 +301,11 @@ public class InsertCoordinator extends AbstractMutationCoordinator { } } - private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group) { + private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) { return session.getFactory() .getServiceRegistry() .getService( MutationExecutorService.class ) - .createExecutor( ( session.getTransactionCoordinator() != null && + .createExecutor( ( !dynamicUpdate && session.getTransactionCoordinator() != null && session.getTransactionCoordinator().isTransactionActive() ? () -> batchKey : () -> null ), group, session ); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java index 9d6bdac060..42223e4995 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java @@ -447,7 +447,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple final EntityTableMapping mutatingTableDetails = (EntityTableMapping) versionUpdateGroup.getSingleOperation().getTableDetails(); - final MutationExecutor mutationExecutor = executor( session, versionUpdateGroup ); + final MutationExecutor mutationExecutor = executor( session, versionUpdateGroup, false ); final EntityVersionMapping versionMapping = entityPersister().getVersionMapping(); @@ -703,7 +703,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple UpdateValuesAnalysisImpl valuesAnalysis, SharedSessionContractImplementor session) { - final MutationExecutor mutationExecutor = executor( session, staticUpdateGroup ); + final MutationExecutor mutationExecutor = executor( session, staticUpdateGroup, false ); decomposeForUpdate( id, @@ -921,7 +921,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple // and then execute them - final MutationExecutor mutationExecutor = executor( session, dynamicUpdateGroup ); + final MutationExecutor mutationExecutor = executor( session, dynamicUpdateGroup, true ); decomposeForUpdate( id, @@ -966,11 +966,11 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple } } - private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group) { + private MutationExecutor executor(SharedSessionContractImplementor session, MutationOperationGroup group, boolean dynamicUpdate) { return session.getSessionFactory() .getServiceRegistry() .getService( MutationExecutorService.class ) - .createExecutor( ( session.getTransactionCoordinator() != null && + .createExecutor( ( !dynamicUpdate && session.getTransactionCoordinator() != null && session.getTransactionCoordinator().isTransactionActive() ? () -> batchKey : () -> null ), group, session ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchedMultiTableDynamicStatementTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchedMultiTableDynamicStatementTests.java new file mode 100644 index 0000000000..f4d42b274e --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchedMultiTableDynamicStatementTests.java @@ -0,0 +1,181 @@ +/* + * 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.batch; + +import java.util.List; + +import org.hibernate.annotations.DynamicInsert; +import org.hibernate.annotations.DynamicUpdate; + +import org.hibernate.testing.jdbc.SQLStatementInspector; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.FailureExpected; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Inheritance; +import jakarta.persistence.PrimaryKeyJoinColumn; +import jakarta.persistence.Table; + +import static jakarta.persistence.InheritanceType.JOINED; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hibernate.cfg.AvailableSettings.STATEMENT_BATCH_SIZE; + +/** + * @author Steve Ebersole + */ +public class BatchedMultiTableDynamicStatementTests { + + @Test + @ServiceRegistry( settings = @Setting( name = STATEMENT_BATCH_SIZE, value = "2" ) ) + @DomainModel( annotatedClasses = { Payment.class, CheckPayment.class } ) + @SessionFactory( useCollectingStatementInspector = true ) + public void testBatched(SessionFactoryScope scope) { + final SQLStatementInspector statementCollector = scope.getCollectingStatementInspector(); + statementCollector.clear(); + + createData( scope ); + + assertThat( statementCollector.getSqlQueries() ).hasSize( 6 ); + + scope.inTransaction( (session) -> { + final List payments = session.createSelectionQuery( "from Payment", Payment.class ).list(); + assertThat( payments ).hasSize( 3 ); + } ); + } + + @Test + @ServiceRegistry( settings = @Setting( name = STATEMENT_BATCH_SIZE, value = "-1" ) ) + @DomainModel( annotatedClasses = { Payment.class, CheckPayment.class } ) + @SessionFactory( useCollectingStatementInspector = true ) + public void testNonBatched(SessionFactoryScope scope) { + final SQLStatementInspector statementCollector = scope.getCollectingStatementInspector(); + statementCollector.clear(); + + createData( scope ); + + assertThat( statementCollector.getSqlQueries() ).hasSize( 6 ); + + scope.inTransaction( (session) -> { + final List payments = session.createSelectionQuery( "from Payment", Payment.class ).list(); + assertThat( payments ).hasSize( 3 ); + } ); + } + + private static void createData(SessionFactoryScope scope) { + final CheckPayment payment = new CheckPayment(); + payment.setId( 1 ); + payment.setAmount( 123.00 ); + payment.setRoute( "0123-45-6789" ); + payment.setAccount( "0089654321" ); + + final CheckPayment payment2 = new CheckPayment(); + payment2.setId( 2 ); + payment2.setAmount( 230.00 ); + payment2.setRoute( "0123-45-6789" ); + payment2.setAccount( "0089654321" ); + payment2.setMemo( "Car Loan" ); + + final CheckPayment payment3 = new CheckPayment(); + payment3.setId( 3 ); + payment3.setAmount( 1234.00 ); + payment3.setRoute( "0123-45-6789" ); + payment3.setAccount( "0089654321" ); + + scope.inTransaction( (session) -> { + session.persist( payment ); + session.persist( payment2 ); + session.persist( payment3 ); + } ); + } + + @AfterEach + public void dropTestData(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + session.createMutationQuery( "delete Payment" ).executeUpdate(); + } ); + } + + @Entity( name = "Payment" ) + @Table( name = "payments" ) + @Inheritance( strategy = JOINED ) + @DynamicInsert @DynamicUpdate + public static class Payment { + @Id + private Integer id; + private double amount; + private String comment; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public double getAmount() { + return amount; + } + + public void setAmount(double amount) { + this.amount = amount; + } + + public String getComment() { + return comment; + } + + public void setComment(String comment) { + this.comment = comment; + } + } + + @Entity( name = "CheckPayment") + @Table( name = "check_payments" ) + @PrimaryKeyJoinColumn( name = "payment_fk" ) + @DynamicInsert + @DynamicUpdate + public static class CheckPayment extends Payment { + @Basic(optional = false) + private String route; + @Basic(optional = false) + private String account; + private String memo; + + public String getRoute() { + return route; + } + + public void setRoute(String route) { + this.route = route; + } + + public String getAccount() { + return account; + } + + public void setAccount(String account) { + this.account = account; + } + + public String getMemo() { + return memo; + } + + public void setMemo(String memo) { + this.memo = memo; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/DynamicMutationsAndBatchingTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/DynamicMutationsAndBatchingTest.java new file mode 100644 index 0000000000..b558804e99 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/DynamicMutationsAndBatchingTest.java @@ -0,0 +1,182 @@ +/* + * 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.batch; + +import java.util.List; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +import org.hibernate.Transaction; +import org.hibernate.annotations.DynamicInsert; +import org.hibernate.annotations.DynamicUpdate; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.query.Query; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * @author Jan Schatteman + */ +@ServiceRegistry( + settings = { + @Setting(name = AvailableSettings.STATEMENT_BATCH_SIZE, value = "2"), + } +) +@DomainModel( + annotatedClasses = { DynamicMutationsAndBatchingTest.EntityA.class } +) +@SessionFactory +@TestForIssue( jiraKey = "HHH-16352") +public class DynamicMutationsAndBatchingTest { + + @AfterEach + public void cleanup( SessionFactoryScope scope ) { + scope.inTransaction( + s -> { + s.createMutationQuery( "delete from EntityA" ).executeUpdate(); + } + ); + } + + @Test + public void testDynamicInserts( SessionFactoryScope scope ) { + scope.inTransaction( + s -> { + EntityA entityA1 = new EntityA(1); + entityA1.propertyA = 1; + + EntityA entityA2 = new EntityA(2); + entityA2.propertyB = 2; + entityA2.propertyC = 3; + + EntityA entityA3 = new EntityA(3); + entityA3.propertyA = 4; + entityA3.propertyC = 5; + + s.persist(entityA1); + s.persist(entityA2); + s.persist(entityA3); + } + ); + + scope.inTransaction( + s -> { + Query query = s.createQuery( "select e from EntityA e where id = :id", EntityA.class); + EntityA a1 = query.setParameter( "id", 1 ).uniqueResult(); + assertNotNull( a1 ); + assertNull(a1.propertyB); + assertNull(a1.propertyC); + assertEquals( 1, a1.propertyA ); + + EntityA a2 = query.setParameter( "id", 2 ).uniqueResult(); + assertNotNull( a2 ); + assertNull( a2.propertyA ); + assertEquals( 2, a2.propertyB ); + assertEquals( 3, a2.propertyC ); + + EntityA a3 = query.setParameter( "id", 3 ).uniqueResult(); + assertNotNull( a3 ); + assertNull( a3.propertyB ); + assertEquals( 4, a3.propertyA ); + assertEquals( 5, a3.propertyC ); + } + ); + } + + @Test + public void testDynamicUpdates( SessionFactoryScope scope ) { + scope.inTransaction( + s -> { + EntityA entityA1 = new EntityA(1); + EntityA entityA2 = new EntityA(2); + EntityA entityA3 = new EntityA(3); + s.persist(entityA1); + s.persist(entityA2); + s.persist(entityA3); + } + ); + + scope.inSession( + s -> { + Query query = s.createQuery( "select e from EntityA e order by id asc", EntityA.class); + Transaction tx = s.beginTransaction(); + List actual = query.list(); + actual.get(0).propertyA = 1; + actual.get(1).propertyA = 2; + actual.get(1).propertyB = 2; + actual.get(2).propertyA = 4; + s.flush(); + tx.commit(); + } + ); + + scope.inTransaction( + s -> { + Query query = s.createQuery( "select e from EntityA e where id = :id", EntityA.class); + EntityA a1 = query.setParameter( "id", 1 ).uniqueResult(); + assertNotNull( a1 ); + assertEquals( 1, a1.propertyA ); + assertNull(a1.propertyB); + assertNull(a1.propertyC); + + EntityA a2 = query.setParameter( "id", 2 ).uniqueResult(); + assertNotNull( a2 ); + assertNull( a2.propertyC ); + assertEquals( 2, a2.propertyA ); + assertEquals( 2, a2.propertyB ); + + EntityA a3 = query.setParameter( "id", 3 ).uniqueResult(); + assertNotNull( a3 ); + assertNull( a3.propertyB ); + assertNull( a3.propertyC ); + assertEquals( 4, a3.propertyA ); + } + ); + } + + @DynamicInsert + @DynamicUpdate + @Entity(name="EntityA") + @Table(name = "ENTITY_A") + public static class EntityA { + + @Id + @Column(name = "ID") + Integer id; + + @Column(name = "PROPERTY_A") + Integer propertyA; + + @Column(name = "PROPERTY_B") + Integer propertyB; + + @Column(name = "PROPERTY_C") + Integer propertyC; + + public EntityA() { + } + + public EntityA(Integer id) { + this.id = id; + } + } +}