HHH-16984 Disable use of arrays for batch and multi-loader on H2

hibernate · Jul 25, 2023 · 949397f · 949397f
1 parent 7eba1b4
commit 949397f
Show file tree

Hide file tree

Showing 13 changed files with 58 additions and 28 deletions.
diff --git a/...ate-community-dialects/src/main/java/org/hibernate/community/dialect/H2LegacyDialect.java b/...ate-community-dialects/src/main/java/org/hibernate/community/dialect/H2LegacyDialect.java
@@ -207,6 +207,12 @@ public boolean supportsStandardArrays() {
 		return getVersion().isSameOrAfter( 2 );
 	}
 
+	@Override
+	public boolean useArrayForMultiValuedParameters() {
+		// Performance is worse than the in-predicate version
+		return false;
+	}
+
 	@Override
 	protected String columnType(int sqlTypeCode) {
 		switch ( sqlTypeCode ) {

diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java
@@ -4151,6 +4151,17 @@ public boolean supportsStandardArrays() {
 		return false;
 	}
 
+	/**
+	 * Does this database prefer to use array types for multi-valued parameters.
+	 *
+	 * @return boolean
+	 *
+	 * @since 6.2.8
+	 */
+	public boolean useArrayForMultiValuedParameters() {
+		return supportsStandardArrays() && getPreferredSqlTypeCodeForArray() == SqlTypes.ARRAY;
+	}
+
 	/**
 	 * The SQL type name for the array type with elements of the given type name.
 	 * <p>

diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java
@@ -193,6 +193,12 @@ public boolean supportsStandardArrays() {
 		return getVersion().isSameOrAfter( 2 );
 	}
 
+	@Override
+	public boolean useArrayForMultiValuedParameters() {
+		// Performance is worse than the in-predicate version
+		return false;
+	}
+
 	@Override
 	protected String columnType(int sqlTypeCode) {
 		switch ( sqlTypeCode ) {

diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiKeyLoadHelper.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiKeyLoadHelper.java
@@ -12,7 +12,6 @@
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.metamodel.mapping.JdbcMapping;
 import org.hibernate.type.BasicType;
-import org.hibernate.type.SqlTypes;
 import org.hibernate.type.descriptor.java.BasicPluralJavaType;
 import org.hibernate.type.descriptor.java.JavaType;
 import org.hibernate.type.descriptor.java.spi.JavaTypeRegistry;
@@ -26,8 +25,7 @@ private MultiKeyLoadHelper() {
 	}
 
 	public static boolean supportsSqlArrayType(Dialect dialect) {
-		return dialect.supportsStandardArrays()
-				&& dialect.getPreferredSqlTypeCodeForArray() == SqlTypes.ARRAY;
+		return dialect.useArrayForMultiValuedParameters();
 	}
 
 	public static JdbcMapping resolveArrayJdbcMapping(

diff --git a/...nate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java b/...nate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java
@@ -62,9 +62,7 @@ public CollectionBatchLoader createCollectionBatchLoader(
 			SessionFactoryImplementor factory) {
 		final Dialect dialect = factory.getJdbcServices().getDialect();
 		final int columnCount = attributeMapping.getKeyDescriptor().getJdbcTypeCount();
-		if ( columnCount == 1
-				&& dialect.supportsStandardArrays()
-				&& dialect.getPreferredSqlTypeCodeForArray() == SqlTypes.ARRAY ) {
+		if ( columnCount == 1 && MultiKeyLoadHelper.supportsSqlArrayType( dialect ) ) {
 			// we can use a single ARRAY parameter to send all the ids
 			return new CollectionBatchLoaderArrayParam( domainBatchSize, influencers, attributeMapping, factory );
 		}

diff --git a/...core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java b/...core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java
@@ -119,6 +119,7 @@ public HqlInterpretation resolveHqlInterpretation(
 			Class<?> expectedResultType,
 			HqlTranslator translator) {
 		log.tracef( "QueryPlan#resolveHqlInterpretation( `%s` )", queryString );
+		final StatisticsImplementor statistics = statisticsSupplier.get();
 
 		final Object cacheKey;
 		if ( expectedResultType != null ) {
@@ -129,30 +130,35 @@ public HqlInterpretation resolveHqlInterpretation(
 		}
 		final HqlInterpretation existing = hqlInterpretationCache.get( cacheKey );
 		if ( existing != null ) {
-			final StatisticsImplementor statistics = statisticsSupplier.get();
 			if ( statistics.isStatisticsEnabled() ) {
 				statistics.queryPlanCacheHit( queryString );
 			}
 			return existing;
 		}
-		else {
-			final HqlInterpretation hqlInterpretation = createHqlInterpretation(
-					queryString,
-					expectedResultType,
-					translator,
-					statisticsSupplier
-			);
-			hqlInterpretationCache.put( cacheKey, hqlInterpretation );
-			return hqlInterpretation;
+		else if ( expectedResultType != null ) {
+			final HqlInterpretation existingQueryOnly = hqlInterpretationCache.get( queryString );
+			if ( existingQueryOnly != null ) {
+				if ( statistics.isStatisticsEnabled() ) {
+					statistics.queryPlanCacheHit( queryString );
+				}
+				return existingQueryOnly;
+			}
 		}
+		final HqlInterpretation hqlInterpretation = createHqlInterpretation(
+				queryString,
+				expectedResultType,
+				translator,
+				statistics
+		);
+		hqlInterpretationCache.put( cacheKey, hqlInterpretation );
+		return hqlInterpretation;
 	}
 
 	protected static HqlInterpretation createHqlInterpretation(
 			String queryString,
 			Class<?> expectedResultType,
 			HqlTranslator translator,
-			Supplier<StatisticsImplementor> statisticsSupplier) {
-		final StatisticsImplementor statistics = statisticsSupplier.get();
+			StatisticsImplementor statistics) {
 		final boolean stats = statistics.isStatisticsEnabled();
 		final long startTime = ( stats ) ? System.nanoTime() : 0L;
 

diff --git a/...core/src/test/java/org/hibernate/orm/test/batch/BatchAndClassIdAndLazyCollectionTest.java b/...core/src/test/java/org/hibernate/orm/test/batch/BatchAndClassIdAndLazyCollectionTest.java
@@ -103,7 +103,7 @@ public void testSelectChild(SessionFactoryScope scope){
 					}
 					statementInspector.assertExecutedCount( 2 );
 
-					if ( scope.getSessionFactory().getJdbcServices().getDialect().supportsStandardArrays() ) {
+					if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 0 ) ).containsOnlyOnce( "?" );
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 1 ) ).containsOnlyOnce( "?" );
 					}

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndClassIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndClassIdTest.java
@@ -95,7 +95,7 @@ public void testSelectChild(SessionFactoryScope scope){
 					statementInspector.clear();
 					List<Child> children = session.createQuery( "select c from Child c", Child.class ).getResultList();
 					statementInspector.assertExecutedCount( 3 );
-					if ( scope.getSessionFactory().getJdbcServices().getDialect().supportsStandardArrays() ) {
+					if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 1 ) ).containsOnlyOnce( "?" );
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 2 ) ).containsOnlyOnce( "?" );
 					}

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdId2Test.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdId2Test.java
@@ -95,7 +95,7 @@ public void testSelectChild(SessionFactoryScope scope) {
 					statementInspector.clear();
 					List<Child> children = session.createQuery( "select c from Child c", Child.class ).getResultList();
 					statementInspector.assertExecutedCount( 3 );
-					if ( scope.getSessionFactory().getJdbcServices().getDialect().supportsStandardArrays() ) {
+					if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 1 ) ).containsOnlyOnce( "?" );
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 2 ) ).containsOnlyOnce( "?" );
 					}

diff --git a/...src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdIdAndLazyCollectionTest.java b/...src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdIdAndLazyCollectionTest.java
@@ -103,7 +103,7 @@ public void testSelectChild(SessionFactoryScope scope) {
 					}
 					statementInspector.assertExecutedCount( 2 );
 
-					if ( scope.getSessionFactory().getJdbcServices().getDialect().supportsStandardArrays() ) {
+					if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 0 ) ).containsOnlyOnce( "?" );
 						Assertions.assertThat( statementInspector.getSqlQueries().get( 1 ) ).containsOnlyOnce( "?" );
 					}

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batch/BatchAndEmbeddedIdIdTest.java
@@ -93,7 +93,7 @@ public void testSelectChild(SessionFactoryScope scope) {
 			statementInspector.clear();
 			List<Child> children = session.createQuery( "select c from Child c", Child.class ).getResultList();
 			statementInspector.assertExecutedCount( 3 );
-			if ( scope.getSessionFactory().getJdbcServices().getDialect().supportsStandardArrays() ) {
+			if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 				assertThat( statementInspector.getSqlQueries().get( 1 ) ).containsOnlyOnce( "?" );
 				assertThat( statementInspector.getSqlQueries().get( 2 ) ).containsOnlyOnce( "?" );
 			}

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/batchfetch/NestedLazyManyToOneTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/batchfetch/NestedLazyManyToOneTest.java
@@ -126,7 +126,7 @@ public void testGetNestedChildrenLessThanBatchSize(SessionFactoryScope scope) {
 			// 1 for Entity1, 1 for Entity2, 1 for Entity3
 			statementInspector.assertExecutedCount( 3 );
 			statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 1, QUESTION_MARK, 1 );
-			if ( MultiKeyLoadHelper.supportsSqlArrayType( scope.getSessionFactory().getJdbcServices().getDialect() ) ) {
+			if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 				assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 2 ), '?' ) ).isEqualTo( 1 );
 			}
 			else {
@@ -156,7 +156,7 @@ public void testGetNestedChildrenMoreThanBatchSize(SessionFactoryScope scope) {
 			// 1 for Entity1, 1 for Entity2, 2 for Entity3
 			assertThat( statementInspector.getSqlQueries() ).hasSize( 4 );
 			assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 1 ), '?' ) ).isEqualTo( 1 );
-			if ( MultiKeyLoadHelper.supportsSqlArrayType( scope.getSessionFactory().getJdbcServices().getDialect() ) ) {
+			if ( scope.getSessionFactory().getJdbcServices().getDialect().useArrayForMultiValuedParameters() ) {
 				assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 2 ), '?' ) ).isEqualTo( 1 );
 				assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 3 ), '?' ) ).isEqualTo( 1 );
 			}

diff --git a/...ibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java b/...ibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionDialectWithLimitTest.java
@@ -109,6 +109,7 @@ public void after(SessionFactoryScope scope) {
 	@TestForIssue(jiraKey = "HHH-12740")
 	public void testSubselect(SessionFactoryScope scope) {
 		final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
+		final Dialect dialect = scope.getSessionFactory().getFastSessionServices().jdbcServices.getDialect();
 		statementInspector.clear();
 
 		scope.inTransaction(
@@ -117,7 +118,12 @@ public void testSubselect(SessionFactoryScope scope) {
 					assertEquals( 56, list.size() );
 
 					// None of the collections should be loaded yet
-					assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+					if ( dialect.useArrayForMultiValuedParameters() ) {
+						assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+					}
+					else {
+						assertThat( statementInspector.getSqlQueries() ).hasSize( 2 );
+					}
 					for ( Parent p : list ) {
 						assertFalse( Hibernate.isInitialized( p.children ) );
 					}
@@ -128,8 +134,7 @@ public void testSubselect(SessionFactoryScope scope) {
 					Hibernate.initialize( list.get( 0 ).children );
 
 					// exactly how depends on whether the Dialect supports use of SQL ARRAY
-					final Dialect dialect = scope.getSessionFactory().getFastSessionServices().jdbcServices.getDialect();
-					if ( MultiKeyLoadHelper.supportsSqlArrayType( dialect ) ) {
+					if ( dialect.useArrayForMultiValuedParameters() ) {
 						assertThat( Hibernate.isInitialized( list.get( 0 ).children ) ).isTrue();
 						assertThat( Hibernate.isInitialized( list.get( 50 ).children ) ).isTrue();
 						assertThat( Hibernate.isInitialized( list.get( 52 ).children ) ).isTrue();