Skip to content

Commit b5fb34d

Browse files
SanneSanne
committed
HHH-19646 Improve validation of configuration settings for the Query Plan Cache
1 parent 007d2f1 commit b5fb34d

File tree

2 files changed

+155
-7
lines changed

2 files changed

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

0 commit comments

Comments
 (0)