Skip to content

Commit 9ab496f

Browse files
victornoelvictornoel
committed
HHH-19331: query plan cache should take parameter type into accound for cast
1 parent 28bcb12 commit 9ab496f

File tree

4 files changed

+123
-55
lines changed

4 files changed

+123
-55
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5667,7 +5667,7 @@ protected void renderCasted(Expression expression) {
56675667
if ( expression instanceof SqlTypedMappingJdbcParameter parameter ) {
56685668
final SqlTypedMapping sqlTypedMapping = parameter.getSqlTypedMapping();
56695669
castTarget = new CastTarget(
5670-
parameter.getJdbcMapping(),
5670+
sqlTypedMapping.getJdbcMapping(),
56715671
sqlTypedMapping.getColumnDefinition(),
56725672
sqlTypedMapping.getLength(),
56735673
sqlTypedMapping.getTemporalPrecision() != null
@@ -5680,6 +5680,12 @@ protected void renderCasted(Expression expression) {
56805680
castTarget = new CastTarget( expression.getExpressionType().getSingleJdbcMapping() );
56815681
}
56825682
arguments.add( castTarget );
5683+
if ( expression instanceof JdbcParameter ) {
5684+
// the value itself is not important, but its type,
5685+
// to improve performances, we could store that information
5686+
// and use it in JdbcOperationQuery.isCompatibleWith instead
5687+
addAppliedParameterBinding( (JdbcParameter) expression, null );
5688+
}
56835689
castFunction().render( this, arguments, (ReturnableType<?>) castTarget.getJdbcMapping(), this );
56845690
}
56855691

‎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
@@ -78,16 +78,37 @@ public boolean isCompatibleWith(JdbcParameterBindings jdbcParameterBindings, Que
7878
return false;
7979
}
8080
for ( Map.Entry<JdbcParameter, JdbcParameterBinding> entry : appliedParameters.entrySet() ) {
81-
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( entry.getKey() );
81+
final JdbcParameter parameter = entry.getKey();
82+
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( parameter );
8283
final JdbcParameterBinding appliedBinding = entry.getValue();
83-
//noinspection unchecked
84-
if ( binding == null || !appliedBinding.getBindType()
85-
.getJavaTypeDescriptor()
86-
.areEqual( binding.getBindValue(), appliedBinding.getBindValue() ) ) {
84+
if ( !isCompatible( parameter, appliedBinding, binding, queryOptions ) ) {
8785
return false;
8886
}
8987
}
9088
}
9189
return true;
9290
}
91+
92+
protected boolean isCompatible(
93+
JdbcParameter parameter,
94+
JdbcParameterBinding appliedBinding,
95+
JdbcParameterBinding binding,
96+
QueryOptions queryOptions) {
97+
// This is a special case where the rendered SQL depends on the presence of the parameter,
98+
// but not specifically on the value. Some example of such situation are when generating
99+
// SQL that depends on the type of the parameter (e.g., cast). See also HHH-19331
100+
// and QueryPlanCachingTest, as well as subclass overrides of this method.
101+
if ( appliedBinding == null ) {
102+
// We could optionally optimize this by identifying only the type and checking it in the same
103+
// way we check the value below.
104+
return false;
105+
}
106+
//noinspection unchecked
107+
if ( binding == null || !appliedBinding.getBindType()
108+
.getJavaTypeDescriptor()
109+
.areEqual( binding.getBindValue(), appliedBinding.getBindValue() ) ) {
110+
return false;
111+
}
112+
return true;
113+
}
93114
}

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

Lines changed: 30 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.hibernate.query.spi.QueryOptions;
1414
import org.hibernate.sql.ast.tree.expression.JdbcParameter;
1515
import org.hibernate.sql.results.jdbc.spi.JdbcValuesMappingProducer;
16-
import org.hibernate.type.descriptor.java.JavaType;
1716

1817
/**
1918
* Executable JDBC command
@@ -97,52 +96,43 @@ public JdbcLockStrategy getLockStrategy() {
9796

9897
@Override
9998
public boolean isCompatibleWith(JdbcParameterBindings jdbcParameterBindings, QueryOptions queryOptions) {
100-
if ( !appliedParameters.isEmpty() ) {
101-
if ( jdbcParameterBindings == null ) {
102-
return false;
103-
}
104-
for ( Map.Entry<JdbcParameter, JdbcParameterBinding> entry : appliedParameters.entrySet() ) {
105-
final JdbcParameter parameter = entry.getKey();
106-
final JdbcParameterBinding appliedBinding = entry.getValue();
107-
// This is a special case where the rendered SQL depends on the presence of the parameter,
108-
// but not specifically on the value. In this case we have to re-generate the SQL if we can't find a binding
109-
// The need for this can be tested with the OracleFollowOnLockingTest#testPessimisticLockWithMaxResultsThenNoFollowOnLocking
110-
// Since the Limit is not part of the query plan cache key, but this has an effect on follow on locking,
111-
// we must treat the absence of Limit parameters, when they were considered for locking, as incompatible
112-
if ( appliedBinding == null ) {
113-
if ( parameter == offsetParameter ) {
114-
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getFirstRowJpa() == 0 ) {
115-
return false;
116-
}
117-
}
118-
else if ( parameter == limitParameter ) {
119-
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getMaxRowsJpa() == Integer.MAX_VALUE ) {
120-
return false;
121-
}
122-
}
123-
else if ( jdbcParameterBindings.getBinding( parameter ) == null ) {
124-
return false;
125-
}
126-
}
127-
// We handle limit and offset parameters below
128-
if ( parameter != offsetParameter && parameter != limitParameter ) {
129-
final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( parameter );
130-
// TODO: appliedBinding can be null here, resulting in NPE
131-
if ( binding == null || !areEqualBindings( appliedBinding, binding ) ) {
132-
return false;
133-
}
134-
}
135-
}
99+
if ( !super.isCompatibleWith( jdbcParameterBindings, queryOptions ) ) {
100+
return false;
136101
}
137102
final Limit limit = queryOptions.getLimit();
138103
return ( offsetParameter != null || limitParameter != null || limit == null || limit.isEmpty() )
139104
&& isCompatible( offsetParameter, limit == null ? null : limit.getFirstRow(), 0 )
140105
&& isCompatible( limitParameter, limit == null ? null : limit.getMaxRows(), Integer.MAX_VALUE );
141106
}
142107

143-
private static boolean areEqualBindings(JdbcParameterBinding appliedBinding, JdbcParameterBinding binding) {
144-
final JavaType<Object> javaTypeDescriptor = appliedBinding.getBindType().getJavaTypeDescriptor();
145-
return javaTypeDescriptor.areEqual( binding.getBindValue(), appliedBinding.getBindValue() );
108+
@Override
109+
protected boolean isCompatible(
110+
JdbcParameter parameter,
111+
JdbcParameterBinding appliedBinding,
112+
JdbcParameterBinding binding,
113+
QueryOptions queryOptions) {
114+
// This is a special case where the rendered SQL depends on the presence of the parameter,
115+
// but not specifically on the value. In this case we have to re-generate the SQL if we can't find a binding
116+
// The need for this can be tested with the OracleFollowOnLockingTest#testPessimisticLockWithMaxResultsThenNoFollowOnLocking
117+
// Since the Limit is not part of the query plan cache key, but this has an effect on follow on locking,
118+
// we must treat the absence of Limit parameters, when they were considered for locking, as incompatible.
119+
if ( appliedBinding == null ) {
120+
if ( parameter == offsetParameter ) {
121+
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getFirstRowJpa() == 0 ) {
122+
return false;
123+
}
124+
}
125+
else if ( parameter == limitParameter ) {
126+
if ( queryOptions.getLimit() == null || queryOptions.getLimit().getMaxRowsJpa() == Integer.MAX_VALUE ) {
127+
return false;
128+
}
129+
}
130+
}
131+
// We handle limit and offset parameters in isCompatibleWith
132+
if ( parameter != offsetParameter && parameter != limitParameter ) {
133+
return super.isCompatible( parameter, appliedBinding, binding, queryOptions );
134+
}
135+
return true;
146136
}
147137

148138
private boolean isCompatible(JdbcParameter parameter, Integer requestedValue, int defaultValue) {

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

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,47 @@
55
package org.hibernate.orm.test.query.hql;
66

77
import org.hibernate.engine.spi.SessionImplementor;
8-
import org.hibernate.orm.test.mapping.SmokeTests;
9-
108
import org.hibernate.query.criteria.HibernateCriteriaBuilder;
119
import org.hibernate.query.criteria.JpaCriteriaQuery;
1210
import org.hibernate.query.criteria.JpaRoot;
11+
import org.hibernate.testing.orm.domain.gambit.BasicEntity;
1312
import org.hibernate.testing.orm.junit.DomainModel;
13+
import org.hibernate.testing.orm.junit.Jira;
1414
import org.hibernate.testing.orm.junit.ServiceRegistry;
1515
import org.hibernate.testing.orm.junit.SessionFactory;
1616
import org.hibernate.testing.orm.junit.SessionFactoryScope;
17+
import org.junit.jupiter.api.AfterAll;
18+
import org.junit.jupiter.api.BeforeAll;
1719
import org.junit.jupiter.api.Test;
1820

21+
import java.math.BigDecimal;
22+
23+
import static org.assertj.core.api.Assertions.assertThat;
24+
1925
/**
2026
* @author Steve Ebersole
2127
*/
22-
@DomainModel( annotatedClasses = SmokeTests.SimpleEntity.class )
28+
@DomainModel(annotatedClasses = BasicEntity.class)
2329
@ServiceRegistry
24-
@SessionFactory( exportSchema = true )
30+
@SessionFactory(exportSchema = true)
2531
public class QueryPlanCachingTest {
32+
33+
@BeforeAll
34+
public void setUp(SessionFactoryScope scope) {
35+
scope.inTransaction( session -> session.persist( new BasicEntity( 1, "entity_1" ) ) );
36+
}
37+
38+
@AfterAll
39+
public void tearDown(SessionFactoryScope scope) {
40+
scope.getSessionFactory().getSchemaManager().truncateMappedObjects();
41+
}
42+
2643
@Test
2744
public void testHqlTranslationCaching(SessionFactoryScope scope) {
2845
scope.inTransaction(
2946
session -> {
30-
session.createQuery( "select e from SimpleEntity e" ).list();
31-
session.createQuery( "select e from SimpleEntity e" ).list();
47+
session.createQuery( "select e from BasicEntity e" ).list();
48+
session.createQuery( "select e from BasicEntity e" ).list();
3249
}
3350
);
3451
}
@@ -44,11 +61,45 @@ public void testCriteriaTranslationCaching(SessionFactoryScope scope) {
4461
);
4562
}
4663

47-
private static JpaCriteriaQuery<SmokeTests.SimpleEntity> constructCriteriaQuery(SessionImplementor session) {
64+
private static JpaCriteriaQuery<BasicEntity> constructCriteriaQuery(SessionImplementor session) {
4865
final HibernateCriteriaBuilder cb = session.getCriteriaBuilder();
49-
final JpaCriteriaQuery<SmokeTests.SimpleEntity> query = cb.createQuery( SmokeTests.SimpleEntity.class );
50-
final JpaRoot<SmokeTests.SimpleEntity> root = query.from( SmokeTests.SimpleEntity.class );
66+
final JpaCriteriaQuery<BasicEntity> query = cb.createQuery( BasicEntity.class );
67+
final JpaRoot<BasicEntity> root = query.from( BasicEntity.class );
5168
query.select( root );
5269
return query;
5370
}
71+
72+
@Test
73+
@Jira("https://hibernate.atlassian.net/browse/HHH-19331")
74+
public void hhh19331_selectionquery(SessionFactoryScope scope) {
75+
scope.inTransaction( session -> {
76+
assertThat(
77+
session.createSelectionQuery( "select :p0 from BasicEntity", Object[].class )
78+
.setParameter( "p0", 1 )
79+
.getSingleResult()
80+
).containsExactly( 1 );
81+
assertThat(
82+
session.createSelectionQuery( "select :p0 from BasicEntity", Object[].class )
83+
.setParameter( "p0", BigDecimal.valueOf( 3.14 ) )
84+
.getSingleResult()
85+
).containsExactly( BigDecimal.valueOf( 3.14 ) );
86+
} );
87+
}
88+
89+
@Test
90+
@Jira("https://hibernate.atlassian.net/browse/HHH-19331")
91+
public void hhh19331_query(SessionFactoryScope scope) {
92+
scope.inTransaction( session -> {
93+
assertThat(
94+
session.createQuery( "select :p0 from BasicEntity", Object[].class )
95+
.setParameter( "p0", 1 )
96+
.getSingleResult()
97+
).containsExactly( 1 );
98+
assertThat(
99+
session.createQuery( "select :p0 from BasicEntity", Object[].class )
100+
.setParameter( "p0", BigDecimal.valueOf( 3.14 ) )
101+
.getSingleResult()
102+
).containsExactly( BigDecimal.valueOf( 3.14 ) );
103+
} );
104+
}
54105
}

0 commit comments

Comments
 (0)