Skip to content

Commit 13662e7

Browse files
mbelladembellade
committed
HHH-16594 Preserve consistent query parameter processing order
1 parent 8de77f0 commit 13662e7

File tree

6 files changed

+113
-35
lines changed

6 files changed

+113
-35
lines changed

‎hibernate-core/src/main/java/org/hibernate/internal/util/collections/LinkedIdentityHashMap.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Hibernate, Relational Persistence for Idiomatic Java
3+
*
4+
* License: GNU Lesser General Public License (LGPL), version 2.1 or later
5+
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
6+
*/
7+
package org.hibernate.internal.util.collections;
8+
9+
import java.util.ArrayList;
10+
import java.util.Collection;
11+
import java.util.Collections;
12+
import java.util.IdentityHashMap;
13+
import java.util.LinkedHashSet;
14+
import java.util.Map;
15+
import java.util.Set;
16+
import java.util.stream.Collectors;
17+
18+
/**
19+
* Utility {@link IdentityHashMap} implementation that maintains predictable iteration order,
20+
* with special care for keys iteration efficiency, and uses reference equality
21+
* (i.e. {@code ==}) in place of object-equality when comparing keys.
22+
* <p>
23+
* Note that the {@link #keySet()}, {@link #values()} and {@link #entrySet()} methods for this class
24+
* return unmodifiable collections and are only meant for iteration.
25+
*
26+
* @author Marco Belladelli
27+
*/
28+
public class LinkedIdentityHashMap<K, V> extends IdentityHashMap<K, V> {
29+
private final ArraySet<K> orderedKeys = new ArraySet<>();
30+
31+
@Override
32+
public V put(K key, V value) {
33+
orderedKeys.add( key );
34+
return super.put( key, value );
35+
}
36+
37+
@Override
38+
public V remove(Object key) {
39+
orderedKeys.remove( key );
40+
return super.remove( key );
41+
}
42+
43+
@Override
44+
public boolean remove(Object key, Object value) {
45+
final boolean removed = super.remove( key, value );
46+
if ( removed ) {
47+
orderedKeys.remove( key );
48+
}
49+
return removed;
50+
}
51+
52+
@Override
53+
public void clear() {
54+
orderedKeys.clear();
55+
super.clear();
56+
}
57+
58+
@Override
59+
public Set<K> keySet() {
60+
return Collections.unmodifiableSet( orderedKeys );
61+
}
62+
63+
@Override
64+
public Collection<V> values() {
65+
return orderedKeys.stream().map( this::get ).collect( Collectors.toUnmodifiableList() );
66+
}
67+
68+
@Override
69+
public Set<Entry<K, V>> entrySet() {
70+
return Collections.unmodifiableSet(
71+
(Set<? extends Entry<K, V>>) orderedKeys.stream()
72+
.map( key -> Map.entry( key, get( key ) ) )
73+
.collect( Collectors.toCollection( LinkedHashSet::new ) )
74+
);
75+
}
76+
77+
@Override
78+
public Object clone() {
79+
throw new UnsupportedOperationException();
80+
}
81+
82+
private static final class ArraySet<E> extends ArrayList<E> implements Set<E> {
83+
}
84+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,17 @@
99
import java.util.ArrayList;
1010
import java.util.Collections;
1111
import java.util.HashSet;
12-
import java.util.IdentityHashMap;
1312
import java.util.List;
1413
import java.util.Locale;
1514
import java.util.Map;
1615
import java.util.Set;
1716
import java.util.function.Consumer;
1817
import java.util.function.Predicate;
19-
import jakarta.persistence.Parameter;
2018

2119
import org.hibernate.QueryException;
2220
import org.hibernate.internal.util.StringHelper;
2321
import org.hibernate.internal.util.collections.CollectionHelper;
22+
import org.hibernate.internal.util.collections.LinkedIdentityHashMap;
2423
import org.hibernate.internal.util.compare.ComparableComparator;
2524
import org.hibernate.query.BindableType;
2625
import org.hibernate.query.QueryParameter;
@@ -29,6 +28,8 @@
2928
import org.hibernate.query.spi.QueryParameterImplementor;
3029
import org.hibernate.query.sqm.tree.expression.SqmParameter;
3130

31+
import jakarta.persistence.Parameter;
32+
3233
/**
3334
* Encapsulates metadata about parameters encountered within a query.
3435
*
@@ -98,7 +99,7 @@ public ParameterMetadataImpl(
9899
this.labels = Collections.emptySet();
99100
}
100101
else {
101-
this.queryParameters = new IdentityHashMap<>();
102+
this.queryParameters = new LinkedIdentityHashMap<>();
102103
if ( positionalQueryParameters != null ) {
103104
for ( QueryParameterImplementor<?> value : positionalQueryParameters.values() ) {
104105
this.queryParameters.put( value, Collections.emptyList() );

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import java.util.Iterator;
1313
import java.util.List;
1414
import java.util.Map;
15-
import java.util.Set;
1615
import java.util.concurrent.ConcurrentHashMap;
1716
import java.util.function.BiConsumer;
1817

@@ -167,29 +166,30 @@ public void visitBindings(@SuppressWarnings("rawtypes") BiConsumer action) {
167166
}
168167

169168
@Override
170-
public QueryKey.ParameterBindingsMemento generateQueryKeyMemento(SharedSessionContractImplementor persistenceContext) {
171-
final MutableCacheKeyImpl mutableCacheKey = new MutableCacheKeyImpl(parameterBindingMap.size());
169+
public QueryKey.ParameterBindingsMemento generateQueryKeyMemento(SharedSessionContractImplementor session) {
170+
final MutableCacheKeyImpl mutableCacheKey = new MutableCacheKeyImpl( parameterBindingMap.size() );
171+
// We know that parameters are consumed in processing order, this ensures consistency of generated cache keys
172+
parameterMetadata.visitParameters( queryParameter -> {
173+
final QueryParameterBinding<?> binding = parameterBindingMap.get( queryParameter );
174+
assert binding != null : "Found unbound query parameter while generating cache key";
172175

173-
for ( Map.Entry<QueryParameter<?>, QueryParameterBinding<?>> entry : parameterBindingMap.entrySet() ) {
174-
final QueryParameterBinding<?> binding = entry.getValue();
175176
final MappingModelExpressible<?> mappingType = determineMappingType(
176177
binding,
177-
entry.getKey(),
178-
persistenceContext
178+
queryParameter,
179+
session
179180
);
180-
181181
if ( binding.isMultiValued() ) {
182182
for ( Object bindValue : binding.getBindValues() ) {
183183
assert bindValue != null;
184-
mappingType.addToCacheKey( mutableCacheKey, bindValue, persistenceContext );
184+
mappingType.addToCacheKey( mutableCacheKey, bindValue, session );
185185
}
186186
}
187187
else {
188188
final Object bindValue = binding.getBindValue();
189-
mappingType.addToCacheKey( mutableCacheKey, bindValue, persistenceContext );
189+
mappingType.addToCacheKey( mutableCacheKey, bindValue, session );
190190
}
191-
}
192-
191+
} );
192+
// Finally, build the overall cache key
193193
return mutableCacheKey.build();
194194
}
195195

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.Map;
1414
import java.util.TreeMap;
1515

16+
import org.hibernate.internal.util.collections.LinkedIdentityHashMap;
1617
import org.hibernate.query.internal.QueryParameterNamedImpl;
1718
import org.hibernate.query.internal.QueryParameterPositionalImpl;
1819
import org.hibernate.query.spi.QueryParameterImplementor;
@@ -45,7 +46,7 @@ public static DomainParameterXref from(SqmStatement<?> sqmStatement) {
4546
return empty();
4647
}
4748

48-
final Map<QueryParameterImplementor<?>, List<SqmParameter<?>>> sqmParamsByQueryParam = new IdentityHashMap<>();
49+
final Map<QueryParameterImplementor<?>, List<SqmParameter<?>>> sqmParamsByQueryParam = new LinkedIdentityHashMap<>();
4950

5051
final int sqmParamCount = parameterResolutions.getSqmParameters().size();
5152
final Map<SqmParameter<?>, QueryParameterImplementor<?>> queryParamBySqmParam = new IdentityHashMap<>( sqmParamCount );
@@ -166,14 +167,6 @@ public int getNumberOfSqmParameters(QueryParameterImplementor<?> queryParameter)
166167
return sqmParameters.size();
167168
}
168169

169-
/**
170-
* Get the mapping of all QueryParameters to the List of its corresponding
171-
* SqmParameters
172-
*/
173-
public Map<QueryParameterImplementor<?>, List<SqmParameter<?>>> getSqmParamByQueryParam() {
174-
return sqmParamsByQueryParam;
175-
}
176-
177170
public SqmStatement.ParameterResolutions getParameterResolutions() {
178171
return parameterResolutions;
179172
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
import java.util.ArrayList;
1010
import java.util.Collection;
1111
import java.util.Collections;
12-
import java.util.HashSet;
1312
import java.util.IdentityHashMap;
1413
import java.util.Iterator;
14+
import java.util.LinkedHashSet;
1515
import java.util.List;
1616
import java.util.Locale;
1717
import java.util.Map;
@@ -31,12 +31,8 @@
3131
import org.hibernate.metamodel.mapping.JdbcMapping;
3232
import org.hibernate.metamodel.mapping.MappingModelExpressible;
3333
import org.hibernate.metamodel.mapping.PluralAttributeMapping;
34-
import org.hibernate.sql.exec.spi.JdbcParametersList;
35-
import org.hibernate.type.JavaObjectType;
36-
import org.hibernate.type.descriptor.converter.spi.BasicValueConverter;
3734
import org.hibernate.query.IllegalQueryOperationException;
3835
import org.hibernate.query.IllegalSelectQueryException;
39-
import org.hibernate.spi.NavigablePath;
4036
import org.hibernate.query.spi.QueryParameterBinding;
4137
import org.hibernate.query.spi.QueryParameterBindings;
4238
import org.hibernate.query.spi.QueryParameterImplementor;
@@ -50,12 +46,16 @@
5046
import org.hibernate.query.sqm.tree.expression.SqmParameter;
5147
import org.hibernate.query.sqm.tree.jpa.ParameterCollector;
5248
import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
49+
import org.hibernate.spi.NavigablePath;
5350
import org.hibernate.sql.ast.SqlTreeCreationException;
5451
import org.hibernate.sql.ast.tree.expression.JdbcParameter;
5552
import org.hibernate.sql.ast.tree.from.TableGroup;
5653
import org.hibernate.sql.exec.internal.JdbcParameterBindingImpl;
5754
import org.hibernate.sql.exec.internal.JdbcParameterBindingsImpl;
5855
import org.hibernate.sql.exec.spi.JdbcParameterBindings;
56+
import org.hibernate.sql.exec.spi.JdbcParametersList;
57+
import org.hibernate.type.JavaObjectType;
58+
import org.hibernate.type.descriptor.converter.spi.BasicValueConverter;
5959
import org.hibernate.type.spi.TypeConfiguration;
6060

6161
/**
@@ -118,7 +118,7 @@ public static Map<QueryParameterImplementor<?>, Map<SqmParameter<?>, List<JdbcPa
118118
final int queryParameterCount = domainParameterXref.getQueryParameterCount();
119119
final Map<QueryParameterImplementor<?>, Map<SqmParameter<?>, List<JdbcParametersList>>> result = new IdentityHashMap<>( queryParameterCount );
120120

121-
for ( Map.Entry<QueryParameterImplementor<?>, List<SqmParameter<?>>> entry : domainParameterXref.getSqmParamByQueryParam().entrySet() ) {
121+
for ( Map.Entry<QueryParameterImplementor<?>, List<SqmParameter<?>>> entry : domainParameterXref.getQueryParameters().entrySet() ) {
122122
final QueryParameterImplementor<?> queryParam = entry.getKey();
123123
final List<SqmParameter<?>> sqmParams = entry.getValue();
124124

@@ -211,7 +211,7 @@ public static JdbcParameterBindings createJdbcParameterBindings(
211211
);
212212

213213
for ( Map.Entry<QueryParameterImplementor<?>, List<SqmParameter<?>>> entry :
214-
domainParameterXref.getSqmParamByQueryParam().entrySet() ) {
214+
domainParameterXref.getQueryParameters().entrySet() ) {
215215
final QueryParameterImplementor<?> queryParam = entry.getKey();
216216
final List<SqmParameter<?>> sqmParameters = entry.getValue();
217217

@@ -480,7 +480,7 @@ private static class CriteriaParameterCollector {
480480

481481
public void process(SqmParameter<?> parameter) {
482482
if ( sqmParameters == null ) {
483-
sqmParameters = new HashSet<>();
483+
sqmParameters = new LinkedHashSet<>();
484484
}
485485

486486
if ( parameter instanceof SqmJpaCriteriaParameterWrapper<?> ) {

‎hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
package org.hibernate.query.sqm.tree.select;
88

99
import java.util.Collections;
10-
import java.util.HashSet;
10+
import java.util.LinkedHashSet;
1111
import java.util.List;
1212
import java.util.Map;
1313
import java.util.Set;
@@ -113,7 +113,7 @@ public SqmSelectStatement<T> copy(SqmCopyContext context) {
113113
parameters = null;
114114
}
115115
else {
116-
parameters = new HashSet<>( this.parameters.size() );
116+
parameters = new LinkedHashSet<>( this.parameters.size() );
117117
for ( SqmParameter<?> parameter : this.parameters ) {
118118
parameters.add( parameter.copy( context ) );
119119
}
@@ -225,7 +225,7 @@ public <X> X accept(SemanticQueryWalker<X> walker) {
225225
@Override
226226
public void addParameter(SqmParameter<?> parameter) {
227227
if ( parameters == null ) {
228-
parameters = new HashSet<>();
228+
parameters = new LinkedHashSet<>();
229229
}
230230

231231
parameters.add( parameter );

0 commit comments

Comments
 (0)