Skip to content

Commit 4af6898

Browse files
victornoelvictornoel
committed
HHH-19331: query plan cache should take parameter type into accound for cast
1 parent bb931c7 commit 4af6898

File tree

4 files changed

+122
-50
lines changed

4 files changed

+122
-50
lines changed

‎hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5804,10 +5804,9 @@ protected void renderCasted(Expression expression) {
58045804
arguments.add( expression );
58055805
final CastTarget castTarget;
58065806
if ( expression instanceof SqlTypedMappingJdbcParameter ) {
5807-
final SqlTypedMappingJdbcParameter parameter = (SqlTypedMappingJdbcParameter) expression;
5808-
final SqlTypedMapping sqlTypedMapping = parameter.getSqlTypedMapping();
5807+
final SqlTypedMapping sqlTypedMapping = ( (SqlTypedMappingJdbcParameter) expression ).getSqlTypedMapping();
58095808
castTarget = new CastTarget(
5810-
parameter.getJdbcMapping(),
5809+
sqlTypedMapping.getJdbcMapping(),
58115810
sqlTypedMapping.getColumnDefinition(),
58125811
sqlTypedMapping.getLength(),
58135812
sqlTypedMapping.getTemporalPrecision() != null
@@ -5820,6 +5819,12 @@ protected void renderCasted(Expression expression) {
58205819
castTarget = new CastTarget( expression.getExpressionType().getSingleJdbcMapping() );
58215820
}
58225821
arguments.add( castTarget );
5822+
if ( expression instanceof JdbcParameter ) {
5823+
// the value itself is not important, but its type,
5824+
// to improve performances, we could store that information
5825+
// and use it in JdbcOperationQuery.isCompatibleWith instead
5826+
addAppliedParameterBinding( (JdbcParameter) expression, null );
5827+
}
58235828
castFunction().render( this, arguments, (ReturnableType<?>) castTarget.getJdbcMapping(), this );
58245829
}
58255830

‎hibernate-core/src/main/java/org/hibernate/sql/exec/spi/AbstractJdbcOperationQuery.java‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,37 @@ public boolean isCompatibleWith(JdbcParameterBindings jdbcParameterBindings, Que
110110
return false;
111111
}
112112
for ( Map.Entry<JdbcParameter, JdbcParameterBinding> entry : appliedParameters.entrySet() ) {
113-
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( entry.getKey() );
113+
final JdbcParameter parameter = entry.getKey();
114+
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( parameter );
114115
final JdbcParameterBinding appliedBinding = entry.getValue();
115-
//noinspection unchecked
116-
if ( binding == null || !appliedBinding.getBindType()
117-
.getJavaTypeDescriptor()
118-
.areEqual( binding.getBindValue(), appliedBinding.getBindValue() ) ) {
116+
if ( !isCompatible( parameter, appliedBinding, binding, queryOptions ) ) {
119117
return false;
120118
}
121119
}
122120
}
123121
return true;
124122
}
123+
124+
protected boolean isCompatible(
125+
JdbcParameter parameter,
126+
JdbcParameterBinding appliedBinding,
127+
JdbcParameterBinding binding,
128+
QueryOptions queryOptions) {
129+
// This is a special case where the rendered SQL depends on the presence of the parameter,
130+
// but not specifically on the value. Some example of such situation are when generating
131+
// SQL that depends on the type of the parameter (e.g., cast). See also HHH-19331
132+
// and QueryPlanCachingTest, as well as subclass overrides of this method.
133+
if ( appliedBinding == null ) {
134+
// We could optionally optimize this by identifying only the type and checking it in the same
135+
// way we check the value below.
136+
return false;
137+
}
138+
//noinspection unchecked
139+
if ( binding == null || !appliedBinding.getBindType()
140+
.getJavaTypeDescriptor()
141+
.areEqual( binding.getBindValue(), appliedBinding.getBindValue() ) ) {
142+
return false;
143+
}
144+
return true;
145+
}
125146
}

‎hibernate-core/src/main/java/org/hibernate/sql/exec/spi/JdbcOperationQuerySelect.java‎

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -148,43 +148,8 @@ public JdbcLockStrategy getLockStrategy() {
148148

149149
@Override
150150
public boolean isCompatibleWith(JdbcParameterBindings jdbcParameterBindings, QueryOptions queryOptions) {
151-
if ( !appliedParameters.isEmpty() ) {
152-
if ( jdbcParameterBindings == null ) {
153-
return false;
154-
}
155-
for ( Map.Entry<JdbcParameter, JdbcParameterBinding> entry : appliedParameters.entrySet() ) {
156-
final JdbcParameter parameter = entry.getKey();
157-
final JdbcParameterBinding appliedBinding = entry.getValue();
158-
// This is a special case where the rendered SQL depends on the presence of the parameter,
159-
// but not specifically on the value. In this case we have to re-generate the SQL if we can't find a binding
160-
// The need for this can be tested with the OracleFollowOnLockingTest#testPessimisticLockWithMaxResultsThenNoFollowOnLocking
161-
// Since the Limit is not part of the query plan cache key, but this has an effect on follow on locking,
162-
// we must treat the absence of Limit parameters, when they were considered for locking, as incompatible
163-
if ( appliedBinding == null ) {
164-
if ( parameter == offsetParameter ) {
165-
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getFirstRowJpa() == 0 ) {
166-
return false;
167-
}
168-
}
169-
else if ( parameter == limitParameter ) {
170-
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getMaxRowsJpa() == Integer.MAX_VALUE ) {
171-
return false;
172-
}
173-
}
174-
else if ( jdbcParameterBindings.getBinding( parameter ) == null ) {
175-
return false;
176-
}
177-
}
178-
// We handle limit and offset parameters below
179-
if ( parameter != offsetParameter && parameter != limitParameter ) {
180-
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( parameter );
181-
if ( binding == null || !appliedBinding.getBindType()
182-
.getJavaTypeDescriptor()
183-
.areEqual( binding.getBindValue(), appliedBinding.getBindValue() ) ) {
184-
return false;
185-
}
186-
}
187-
}
151+
if ( !super.isCompatibleWith( jdbcParameterBindings, queryOptions ) ) {
152+
return false;
188153
}
189154
final Limit limit = queryOptions.getLimit();
190155
if ( offsetParameter == null && limitParameter == null ) {
@@ -201,6 +166,36 @@ else if ( jdbcParameterBindings.getBinding( parameter ) == null ) {
201166
return true;
202167
}
203168

169+
@Override
170+
protected boolean isCompatible(
171+
JdbcParameter parameter,
172+
JdbcParameterBinding appliedBinding,
173+
JdbcParameterBinding binding,
174+
QueryOptions queryOptions) {
175+
// This is a special case where the rendered SQL depends on the presence of the parameter,
176+
// but not specifically on the value. In this case we have to re-generate the SQL if we can't find a binding
177+
// The need for this can be tested with the OracleFollowOnLockingTest#testPessimisticLockWithMaxResultsThenNoFollowOnLocking
178+
// Since the Limit is not part of the query plan cache key, but this has an effect on follow on locking,
179+
// we must treat the absence of Limit parameters, when they were considered for locking, as incompatible.
180+
if ( appliedBinding == null ) {
181+
if ( parameter == offsetParameter ) {
182+
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getFirstRowJpa() == 0 ) {
183+
return false;
184+
}
185+
}
186+
else if ( parameter == limitParameter ) {
187+
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getMaxRowsJpa() == Integer.MAX_VALUE ) {
188+
return false;
189+
}
190+
}
191+
}
192+
// We handle limit and offset parameters above and in isCompatibleWith
193+
if ( parameter != offsetParameter && parameter != limitParameter ) {
194+
return super.isCompatible( parameter, appliedBinding, binding, queryOptions );
195+
}
196+
return true;
197+
}
198+
204199
private boolean isCompatible(JdbcParameter parameter, Integer requestedValue, int defaultValue) {
205200
if ( parameter == null ) {
206201
return requestedValue == null;

‎hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/QueryPlanCachingTest.java‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,79 @@
66
*/
77
package org.hibernate.orm.test.query.hql;
88

9-
import org.hibernate.orm.test.mapping.SmokeTests;
9+
import java.math.BigDecimal;
1010

11+
import org.hibernate.testing.orm.domain.gambit.BasicEntity;
1112
import org.hibernate.testing.orm.junit.DomainModel;
13+
import org.hibernate.testing.orm.junit.Jira;
1214
import org.hibernate.testing.orm.junit.ServiceRegistry;
1315
import org.hibernate.testing.orm.junit.SessionFactory;
1416
import org.hibernate.testing.orm.junit.SessionFactoryScope;
17+
import org.junit.jupiter.api.AfterAll;
18+
import org.junit.jupiter.api.BeforeAll;
1519
import org.junit.jupiter.api.Test;
1620

21+
import static org.assertj.core.api.Assertions.assertThat;
22+
1723
/**
1824
* @author Steve Ebersole
1925
*/
20-
@DomainModel( annotatedClasses = SmokeTests.SimpleEntity.class )
26+
@DomainModel(annotatedClasses = BasicEntity.class)
2127
@ServiceRegistry
22-
@SessionFactory( exportSchema = true )
28+
@SessionFactory(exportSchema = true)
2329
public class QueryPlanCachingTest {
30+
31+
@BeforeAll
32+
public void setUp(SessionFactoryScope scope) {
33+
scope.inTransaction( session -> session.persist( new BasicEntity( 1, "entity_1" ) ) );
34+
}
35+
36+
@AfterAll
37+
public void tearDown(SessionFactoryScope scope) {
38+
scope.getSessionFactory().getSchemaManager().truncateMappedObjects();
39+
}
40+
2441
@Test
2542
public void testHqlTranslationCaching(SessionFactoryScope scope) {
2643
scope.inTransaction(
2744
session -> {
28-
session.createQuery( "select e from SimpleEntity e" ).list();
29-
session.createQuery( "select e from SimpleEntity e" ).list();
45+
session.createQuery( "select e from BasicEntity e" ).list();
46+
session.createQuery( "select e from BasicEntity e" ).list();
3047
}
3148
);
3249
}
50+
51+
@Test
52+
@Jira("https://hibernate.atlassian.net/browse/HHH-19331")
53+
public void hhh19331_selectionquery(SessionFactoryScope scope) {
54+
scope.inTransaction( session -> {
55+
assertThat(
56+
session.createSelectionQuery( "select :p0 from BasicEntity", Object[].class )
57+
.setParameter( "p0", 1 )
58+
.getSingleResult()
59+
).containsExactly( 1 );
60+
assertThat(
61+
session.createSelectionQuery( "select :p0 from BasicEntity", Object[].class )
62+
.setParameter( "p0", BigDecimal.valueOf( 3.14 ) )
63+
.getSingleResult()
64+
).containsExactly( BigDecimal.valueOf( 3.14 ) );
65+
} );
66+
}
67+
68+
@Test
69+
@Jira("https://hibernate.atlassian.net/browse/HHH-19331")
70+
public void hhh19331_query(SessionFactoryScope scope) {
71+
scope.inTransaction( session -> {
72+
assertThat(
73+
session.createQuery( "select :p0 from BasicEntity", Object[].class )
74+
.setParameter( "p0", 1 )
75+
.getSingleResult()
76+
).containsExactly( 1 );
77+
assertThat(
78+
session.createQuery( "select :p0 from BasicEntity", Object[].class )
79+
.setParameter( "p0", BigDecimal.valueOf( 3.14 ) )
80+
.getSingleResult()
81+
).containsExactly( BigDecimal.valueOf( 3.14 ) );
82+
} );
83+
}
3384
}

0 commit comments

Comments
 (0)