From 9c9a326ae6b2ce0ca74e35ebe3bcf29db32e2b75 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Fri, 5 Nov 2021 13:38:39 +0100 Subject: [PATCH] Fix Dialect#BatchLoadSizingStrategy#determineOptimalBatchLoadSize taking into account org.hibernate.cfg.AvailableSettings#IN_CLAUSE_PARAMETER_PADDING --- .../java/org/hibernate/dialect/Dialect.java | 12 +- .../loader/BatchLoadSizingStrategy.java | 2 +- .../ast/internal/MultiIdLoaderStandard.java | 6 +- .../MultiNaturalIdLoaderStandard.java | 3 +- ...bSelectCollectionDialectWithLimitTest.java | 206 ++++++++++++++++++ .../MultiLoadSubSelectCollectionTest.java | 33 ++- .../orm/junit/ServiceRegistryExtension.java | 2 +- .../testing/orm/junit/SettingProvider.java | 2 +- 8 files changed, 240 insertions(+), 26 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index 9c70d72fa0..506e33b6b0 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -3068,8 +3068,16 @@ public abstract class Dialect implements ConversionContext { protected final BatchLoadSizingStrategy STANDARD_DEFAULT_BATCH_LOAD_SIZING_STRATEGY = new BatchLoadSizingStrategy() { @Override - public int determineOptimalBatchLoadSize(int numberOfKeyColumns, int numberOfKeys) { - int paddedSize = MathHelper.ceilingPowerOfTwo( numberOfKeys ); + public int determineOptimalBatchLoadSize(int numberOfKeyColumns, int numberOfKeys, boolean inClauseParameterPaddingEnabled) { + final int paddedSize; + + if ( inClauseParameterPaddingEnabled ) { + paddedSize = MathHelper.ceilingPowerOfTwo( numberOfKeys ); + } + else { + paddedSize = numberOfKeys; + } + // For tuples, there is no limit, so we can just use the power of two padding approach if ( numberOfKeyColumns > 1 ) { return paddedSize; diff --git a/hibernate-core/src/main/java/org/hibernate/loader/BatchLoadSizingStrategy.java b/hibernate-core/src/main/java/org/hibernate/loader/BatchLoadSizingStrategy.java index 66a055a880..66b82b1215 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/BatchLoadSizingStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/BatchLoadSizingStrategy.java @@ -13,5 +13,5 @@ package org.hibernate.loader; */ @FunctionalInterface public interface BatchLoadSizingStrategy { - int determineOptimalBatchLoadSize(int numberOfKeyColumns, int numberOfKeys); + int determineOptimalBatchLoadSize(int numberOfKeyColumns, int numberOfKeys, boolean inClauseParameterPaddingEnabled); } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdLoaderStandard.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdLoaderStandard.java index 6c41b1248b..eb63ea0376 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdLoaderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdLoaderStandard.java @@ -118,7 +118,8 @@ public class MultiIdLoaderStandard implements MultiIdEntityLoader { else { maxBatchSize = dialect.getDefaultBatchLoadSizingStrategy().determineOptimalBatchLoadSize( idJdbcTypeCount, - ids.length + ids.length, + sessionFactory.getSessionFactoryOptions().inClauseParameterPaddingEnabled() ); } @@ -448,7 +449,8 @@ public class MultiIdLoaderStandard implements MultiIdEntityLoader { else { maxBatchSize = session.getJdbcServices().getJdbcEnvironment().getDialect().getDefaultBatchLoadSizingStrategy().determineOptimalBatchLoadSize( entityDescriptor.getIdentifierType().getColumnSpan( session.getFactory() ), - numberOfIdsLeft + numberOfIdsLeft, + sessionFactory.getSessionFactoryOptions().inClauseParameterPaddingEnabled() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiNaturalIdLoaderStandard.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiNaturalIdLoaderStandard.java index 1d80c611d5..378d9a1e77 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiNaturalIdLoaderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiNaturalIdLoaderStandard.java @@ -57,7 +57,8 @@ public class MultiNaturalIdLoaderStandard implements MultiNaturalIdLoader else { maxBatchSize = session.getJdbcServices().getJdbcEnvironment().getDialect().getDefaultBatchLoadSizingStrategy().determineOptimalBatchLoadSize( entityDescriptor.getNaturalIdMapping().getJdbcTypeCount(), - naturalIds.length + naturalIds.length, + sessionFactory.getSessionFactoryOptions().inClauseParameterPaddingEnabled() ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java new file mode 100644 index 0000000000..17b1d7b333 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java @@ -0,0 +1,206 @@ +package org.hibernate.orm.test.loading.multiLoad; + +import java.util.ArrayList; +import java.util.List; + +import org.hibernate.CacheMode; +import org.hibernate.Hibernate; +import org.hibernate.annotations.BatchSize; +import org.hibernate.annotations.Fetch; +import org.hibernate.annotations.FetchMode; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.dialect.H2Dialect; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.RequiresDialect; +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.SettingProvider; +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.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +import jakarta.persistence.Table; + +import static jakarta.persistence.GenerationType.AUTO; +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 = { + MultiLoadSubSelectCollectionDialectWithLimitTest.Parent.class, + MultiLoadSubSelectCollectionDialectWithLimitTest.Child.class +}) +@ServiceRegistry( + settingProviders = @SettingProvider( provider = MultiLoadSubSelectCollectionDialectWithLimitTest.TestSettingProvider.class, settingName = AvailableSettings.DIALECT) +) +@SessionFactory(generateStatistics = true) +@RequiresDialect( H2Dialect.class ) +public class MultiLoadSubSelectCollectionDialectWithLimitTest { + + + public static class TestSettingProvider implements SettingProvider.Provider { + + @Override + public String getSetting() { + return TestDialect.class.getName(); + } + } + + public static class TestDialect extends H2Dialect{ + @Override + public int getInExpressionCountLimit() { + return 50; + } + } + + @BeforeEach + public void before(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.setCacheMode( CacheMode.IGNORE ); + for ( int i = 1; i <= 60; i++ ) { + final Parent p = new Parent( i, "Entity #" + i ); + for ( int j = 0; j < i; j++ ) { + Child child = new Child(); + child.setParent( p ); + p.getChildren().add( child ); + } + session.persist( p ); + } + } ); + } + + @AfterEach + public void after(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.createQuery( "delete Child" ).executeUpdate(); + session.createQuery( "delete Parent" ).executeUpdate(); + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-12740") + public void testSubselect(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + List list = session.byMultipleIds( Parent.class ).multiLoad( ids( 56 ) ); + assertEquals( 56, list.size() ); + + // None of the collections should be loaded yet + for ( Parent p : list ) { + assertFalse( Hibernate.isInitialized( p.children ) ); + } + + // When the first collection is loaded, the full batch of 50 collections + // should be loaded. + Hibernate.initialize( list.get( 0 ).children ); + + for ( int i = 0; i < 50; i++ ) { + assertTrue( Hibernate.isInitialized( list.get( i ).children ) ); + assertEquals( i + 1, list.get( i ).children.size() ); + } + + // The collections for the 51st through 56th entities should still be uninitialized + for ( int i = 50; i < 56; i++ ) { + assertFalse( Hibernate.isInitialized( list.get( i ).children ) ); + } + + // When the 51st collection gets initialized, the remaining collections should + // also be initialized. + Hibernate.initialize( list.get( 50 ).children ); + + for ( int i = 50; i < 56; i++ ) { + assertTrue( Hibernate.isInitialized( list.get( i ).children ) ); + assertEquals( i + 1, list.get( i ).children.size() ); + } + } + ); + } + + private Integer[] ids(int count) { + Integer[] ids = new Integer[count]; + for ( int i = 1; i <= count; i++ ) { + ids[i - 1] = i; + } + return ids; + } + + @Entity(name = "Parent") + @Table(name = "Parent") + @BatchSize(size = 15) + public static class Parent { + Integer id; + String text; + private List children = new ArrayList<>(); + + public Parent() { + } + + public Parent(Integer id, String text) { + this.id = id; + this.text = text; + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getText() { + return text; + } + + public void setText(String text) { + this.text = text; + } + + @OneToMany(mappedBy = "parent", fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) + @Fetch(FetchMode.SUBSELECT) + public List getChildren() { + return children; + } + + public void setChildren(List children) { + this.children = children; + } + } + + @Entity(name = "Child") + public static class Child { + + @Id + @GeneratedValue(strategy = AUTO) + private int id; + + @ManyToOne(fetch = FetchType.LAZY, optional = true) + private Parent parent; + + public Child() { + } + + public Parent getParent() { + return parent; + } + + public void setParent(Parent parent) { + this.parent = parent; + } + + public int getId() { + return id; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java index f7e668ecc8..1179353373 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java @@ -8,7 +8,21 @@ package org.hibernate.orm.test.loading.multiLoad; import java.util.ArrayList; import java.util.List; -import java.util.Map; + +import org.hibernate.CacheMode; +import org.hibernate.Hibernate; +import org.hibernate.annotations.BatchSize; +import org.hibernate.annotations.Fetch; +import org.hibernate.annotations.FetchMode; + +import org.hibernate.testing.TestForIssue; +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.BeforeEach; +import org.junit.jupiter.api.Test; + import jakarta.persistence.CascadeType; import jakarta.persistence.Entity; import jakarta.persistence.FetchType; @@ -18,22 +32,6 @@ import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; -import org.hibernate.CacheMode; -import org.hibernate.Hibernate; -import org.hibernate.annotations.BatchSize; -import org.hibernate.annotations.Fetch; -import org.hibernate.annotations.FetchMode; -import org.hibernate.cfg.AvailableSettings; - -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.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - import static jakarta.persistence.GenerationType.AUTO; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -47,7 +45,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; MultiLoadSubSelectCollectionTest.Parent.class, MultiLoadSubSelectCollectionTest.Child.class }) -@ServiceRegistry @SessionFactory(generateStatistics = true) public class MultiLoadSubSelectCollectionTest { diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/ServiceRegistryExtension.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/ServiceRegistryExtension.java index a5a6f385d3..9ffe185a50 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/ServiceRegistryExtension.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/ServiceRegistryExtension.java @@ -183,7 +183,7 @@ public class ServiceRegistryExtension } for ( SettingProvider providerAnn : serviceRegistryAnn.settingProviders() ) { - final Class> providerImpl = providerAnn.provider(); + final Class providerImpl = providerAnn.provider(); final SettingProvider.Provider provider = providerImpl.getConstructor().newInstance(); ssrb.applySetting( providerAnn.settingName(), provider.getSetting() ); } diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/SettingProvider.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/SettingProvider.java index 51c2befaef..f5c95efd3b 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/SettingProvider.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/SettingProvider.java @@ -15,5 +15,5 @@ public @interface SettingProvider { } String settingName(); - Class> provider(); + Class provider(); }