diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryEngineImpl.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryEngineImpl.java index 6d366ead6cae..e9a6b62b471f 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryEngineImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryEngineImpl.java @@ -13,6 +13,7 @@ import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.query.spi.NativeQueryInterpreter; import org.hibernate.internal.CoreLogging; +import org.hibernate.internal.util.config.ConfigurationException; import org.hibernate.internal.util.config.ConfigurationHelper; import org.hibernate.metamodel.MappingMetamodel; import org.hibernate.metamodel.model.domain.JpaMetamodel; @@ -171,9 +172,9 @@ private static List sortedFunctionContributors(ServiceRegis return contributors; } - private static QueryInterpretationCache buildInterpretationCache( + public static QueryInterpretationCache buildInterpretationCache( ServiceRegistry serviceRegistry, Map properties) { - final boolean explicitUseCache = ConfigurationHelper.getBoolean( + final boolean useCache = ConfigurationHelper.getBoolean( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, properties, // enabled by default @@ -185,12 +186,20 @@ private static QueryInterpretationCache buildInterpretationCache( properties ); - if ( explicitUseCache || explicitMaxPlanSize != null && explicitMaxPlanSize > 0 ) { - final int size = explicitMaxPlanSize != null - ? explicitMaxPlanSize - : QueryEngine.DEFAULT_QUERY_PLAN_MAX_COUNT; + //Let's avoid some confusion and check settings consistency: + final int appliedMaxPlanSize = explicitMaxPlanSize == null + ? QueryEngine.DEFAULT_QUERY_PLAN_MAX_COUNT + : explicitMaxPlanSize; + if ( !useCache && explicitMaxPlanSize != null && appliedMaxPlanSize > 0 ) { + throw new ConfigurationException( "Inconsistent configuration: '" + AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE + "' can only be set to a greater than zero value when '" + AvailableSettings.QUERY_PLAN_CACHE_ENABLED + "' is enabled" ); + } + + if ( appliedMaxPlanSize < 0 ) { + throw new ConfigurationException( "Inconsistent configuration: '" + AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE + "' can't be set to a negative value. To disable the query plan cache set '" + AvailableSettings.QUERY_PLAN_CACHE_ENABLED + "' to 'false'" ); + } - return new QueryInterpretationCacheStandardImpl( size, serviceRegistry ); + if ( useCache ) { + return new QueryInterpretationCacheStandardImpl( appliedMaxPlanSize, serviceRegistry ); } else { // disabled diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryEngineImplConfigValidationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryEngineImplConfigValidationTest.java new file mode 100644 index 000000000000..a2c93f538bf2 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryEngineImplConfigValidationTest.java @@ -0,0 +1,139 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.querycache; + +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.internal.util.config.ConfigurationException; +import org.hibernate.query.internal.QueryInterpretationCacheDisabledImpl; +import org.hibernate.query.internal.QueryInterpretationCacheStandardImpl; +import org.hibernate.query.spi.QueryInterpretationCache; +import org.hibernate.service.ServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.query.internal.QueryEngineImpl; +import org.hibernate.testing.orm.junit.Jira; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test for configuration validation in QueryEngineImpl. + * Tests the consistency checks between QUERY_PLAN_CACHE_ENABLED and QUERY_PLAN_CACHE_MAX_SIZE. + */ +@Jira("HHH-19646") +public class QueryEngineImplConfigValidationTest { + + @Test + public void testCacheEnabledWithValidMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true ); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 100 ); + try (ServiceRegistry serviceRegistry = newRegistry()) { + QueryInterpretationCache interpretationCache = assertDoesNotThrow( () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + testCacheEnabled( interpretationCache ); + } + } + + @Test + public void testCacheEnabledWithDefaultMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true ); + // No explicit max size - should use default + try (ServiceRegistry serviceRegistry = newRegistry()) { + QueryInterpretationCache interpretationCache = assertDoesNotThrow( () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + testCacheEnabled( interpretationCache ); + } + } + + @Test + public void testCacheDisabledWithNoMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false ); + // No explicit max size - should work fine + try (ServiceRegistry serviceRegistry = newRegistry()) { + QueryInterpretationCache interpretationCache = assertDoesNotThrow( () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + testCacheDisabled( interpretationCache ); + } + } + + @Test + public void testCacheDisabledWithPositiveMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false ); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 100 ); + //Explicit max size, with cache explicitly disabled is an inconsistency we want to flag + try (ServiceRegistry serviceRegistry = newRegistry()) { + ConfigurationException exception = assertThrows( ConfigurationException.class, () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + assertTrue( exception.getMessage().matches( + "Inconsistent configuration: '" + AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE + "' can only be set to a greater than zero value when '" + AvailableSettings.QUERY_PLAN_CACHE_ENABLED + "' is enabled" ) ); + } + } + + @Test + public void testCacheDisabledWithZeroMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false ); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 0 ); + try (ServiceRegistry serviceRegistry = newRegistry()) { + QueryInterpretationCache interpretationCache = assertDoesNotThrow( () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + testCacheDisabled( interpretationCache ); + } + } + + @Test + public void testNegativeMaxSize() { + Map settings = new HashMap<>(); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true ); + settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, -1 ); + try (ServiceRegistry serviceRegistry = newRegistry()) { + ConfigurationException exception = assertThrows( ConfigurationException.class, () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + assertTrue( exception.getMessage().contains( "can't be set to a negative value" ) ); + } + } + + @Test + public void testDefaultConfigurationWorks() { + Map settings = new HashMap<>(); + // No explicit settings - should use defaults and work fine + try (ServiceRegistry serviceRegistry = newRegistry()) { + QueryInterpretationCache interpretationCache = assertDoesNotThrow( () -> + QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings ) + ); + testCacheEnabled( interpretationCache ); + } + } + + private static StandardServiceRegistry newRegistry() { + return new StandardServiceRegistryBuilder().build(); + } + + private void testCacheEnabled(QueryInterpretationCache interpretationCache) { + assertInstanceOf( QueryInterpretationCacheStandardImpl.class, interpretationCache, + "Default interpretation cache should be used" ); + } + + private void testCacheDisabled(QueryInterpretationCache interpretationCache) { + assertInstanceOf( QueryInterpretationCacheDisabledImpl.class, interpretationCache, + "Cache should have been disabled" ); + } +}