Skip to content

Commit 455811c

Browse files
SanneSanne
committed
HHH-19646 Improve validation of configuration settings for the Query Plan Cache
1 parent 50177c5 commit 455811c

File tree

2 files changed

+153
-7
lines changed

2 files changed

+153
-7
lines changed

‎hibernate-core/src/main/java/org/hibernate/query/internal/QueryEngineImpl.java‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.hibernate.engine.jdbc.spi.JdbcServices;
1414
import org.hibernate.engine.query.spi.NativeQueryInterpreter;
1515
import org.hibernate.internal.CoreLogging;
16+
import org.hibernate.internal.util.config.ConfigurationException;
1617
import org.hibernate.internal.util.config.ConfigurationHelper;
1718
import org.hibernate.metamodel.MappingMetamodel;
1819
import org.hibernate.metamodel.model.domain.JpaMetamodel;
@@ -171,9 +172,9 @@ private static List<FunctionContributor> sortedFunctionContributors(ServiceRegis
171172
return contributors;
172173
}
173174

174-
private static QueryInterpretationCache buildInterpretationCache(
175+
public static QueryInterpretationCache buildInterpretationCache(
175176
ServiceRegistry serviceRegistry, Map<String, Object> properties) {
176-
final boolean explicitUseCache = ConfigurationHelper.getBoolean(
177+
final boolean useCache = ConfigurationHelper.getBoolean(
177178
AvailableSettings.QUERY_PLAN_CACHE_ENABLED,
178179
properties,
179180
// enabled by default
@@ -185,12 +186,20 @@ private static QueryInterpretationCache buildInterpretationCache(
185186
properties
186187
);
187188

188-
if ( explicitUseCache || explicitMaxPlanSize != null && explicitMaxPlanSize > 0 ) {
189-
final int size = explicitMaxPlanSize != null
190-
? explicitMaxPlanSize
191-
: QueryEngine.DEFAULT_QUERY_PLAN_MAX_COUNT;
189+
//Let's avoid some confusion and check settings consistency:
190+
final int appliedMaxPlanSize = explicitMaxPlanSize == null
191+
? QueryEngine.DEFAULT_QUERY_PLAN_MAX_COUNT
192+
: explicitMaxPlanSize;
193+
if ( !useCache && explicitMaxPlanSize != null && appliedMaxPlanSize > 0 ) {
194+
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" );
195+
}
196+
197+
if ( appliedMaxPlanSize < 0 ) {
198+
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'" );
199+
}
192200

193-
return new QueryInterpretationCacheStandardImpl( size, serviceRegistry );
201+
if ( useCache ) {
202+
return new QueryInterpretationCacheStandardImpl( appliedMaxPlanSize, serviceRegistry );
194203
}
195204
else {
196205
// disabled

‎hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryEngineImplConfigValidationTest.java‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.querycache;
6+
7+
import org.hibernate.boot.registry.StandardServiceRegistry;
8+
import org.hibernate.cfg.AvailableSettings;
9+
import org.hibernate.internal.util.config.ConfigurationException;
10+
import org.hibernate.query.internal.QueryInterpretationCacheDisabledImpl;
11+
import org.hibernate.query.internal.QueryInterpretationCacheStandardImpl;
12+
import org.hibernate.query.spi.QueryInterpretationCache;
13+
import org.hibernate.service.ServiceRegistry;
14+
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
15+
import org.hibernate.query.internal.QueryEngineImpl;
16+
import org.junit.jupiter.api.Test;
17+
18+
import java.util.HashMap;
19+
import java.util.Map;
20+
21+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
22+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
24+
import static org.junit.jupiter.api.Assertions.assertTrue;
25+
26+
/**
27+
* Test for configuration validation in QueryEngineImpl.
28+
* Tests the consistency checks between QUERY_PLAN_CACHE_ENABLED and QUERY_PLAN_CACHE_MAX_SIZE.
29+
*/
30+
public class QueryEngineImplConfigValidationTest {
31+
32+
@Test
33+
public void testCacheEnabledWithValidMaxSize() {
34+
Map<String, Object> settings = new HashMap<>();
35+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true );
36+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 100 );
37+
try (ServiceRegistry serviceRegistry = newRegistry()) {
38+
QueryInterpretationCache interpretationCache = assertDoesNotThrow( () ->
39+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
40+
);
41+
testCacheEnabled( interpretationCache );
42+
}
43+
}
44+
45+
@Test
46+
public void testCacheEnabledWithDefaultMaxSize() {
47+
Map<String, Object> settings = new HashMap<>();
48+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true );
49+
// No explicit max size - should use default
50+
try (ServiceRegistry serviceRegistry = newRegistry()) {
51+
QueryInterpretationCache interpretationCache = assertDoesNotThrow( () ->
52+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
53+
);
54+
testCacheEnabled( interpretationCache );
55+
}
56+
}
57+
58+
@Test
59+
public void testCacheDisabledWithNoMaxSize() {
60+
Map<String, Object> settings = new HashMap<>();
61+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false );
62+
// No explicit max size - should work fine
63+
try (ServiceRegistry serviceRegistry = newRegistry()) {
64+
QueryInterpretationCache interpretationCache = assertDoesNotThrow( () ->
65+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
66+
);
67+
testCacheDisabled( interpretationCache );
68+
}
69+
}
70+
71+
@Test
72+
public void testCacheDisabledWithPositiveMaxSize() {
73+
Map<String, Object> settings = new HashMap<>();
74+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false );
75+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 100 );
76+
//Explicit max size, with cache explicitly disabled is an inconsistency we want to flag
77+
try (ServiceRegistry serviceRegistry = newRegistry()) {
78+
ConfigurationException exception = assertThrows( ConfigurationException.class, () ->
79+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
80+
);
81+
assertTrue( exception.getMessage().matches(
82+
"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" ) );
83+
}
84+
}
85+
86+
@Test
87+
public void testCacheDisabledWithZeroMaxSize() {
88+
Map<String, Object> settings = new HashMap<>();
89+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, false );
90+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, 0 );
91+
try (ServiceRegistry serviceRegistry = newRegistry()) {
92+
QueryInterpretationCache interpretationCache = assertDoesNotThrow( () ->
93+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
94+
);
95+
testCacheDisabled( interpretationCache );
96+
}
97+
}
98+
99+
@Test
100+
public void testNegativeMaxSize() {
101+
Map<String, Object> settings = new HashMap<>();
102+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_ENABLED, true );
103+
settings.put( AvailableSettings.QUERY_PLAN_CACHE_MAX_SIZE, -1 );
104+
try (ServiceRegistry serviceRegistry = newRegistry()) {
105+
ConfigurationException exception = assertThrows( ConfigurationException.class, () ->
106+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
107+
);
108+
assertTrue( exception.getMessage().contains( "can't be set to a negative value" ) );
109+
}
110+
}
111+
112+
@Test
113+
public void testDefaultConfigurationWorks() {
114+
Map<String, Object> settings = new HashMap<>();
115+
// No explicit settings - should use defaults and work fine
116+
try (ServiceRegistry serviceRegistry = newRegistry()) {
117+
QueryInterpretationCache interpretationCache = assertDoesNotThrow( () ->
118+
QueryEngineImpl.buildInterpretationCache( serviceRegistry, settings )
119+
);
120+
testCacheEnabled( interpretationCache );
121+
}
122+
}
123+
124+
private static StandardServiceRegistry newRegistry() {
125+
return new StandardServiceRegistryBuilder().build();
126+
}
127+
128+
private void testCacheEnabled(QueryInterpretationCache interpretationCache) {
129+
assertInstanceOf( QueryInterpretationCacheStandardImpl.class, interpretationCache,
130+
"Default interpretation cache should be used" );
131+
}
132+
133+
private void testCacheDisabled(QueryInterpretationCache interpretationCache) {
134+
assertInstanceOf( QueryInterpretationCacheDisabledImpl.class, interpretationCache,
135+
"Cache should have been disabled" );
136+
}
137+
}

0 commit comments

Comments
 (0)