Skip to content

Commit f326433

Browse files
sebersolesebersole
committed
HHH-19336 - Proper implementation for JPA extended locking scope
HHH-19459 - LockScope, FollowOnLocking
1 parent 8e40d28 commit f326433

File tree

14 files changed

+99
-91
lines changed

14 files changed

+99
-91
lines changed

‎hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/OracleLegacyDialect.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1383,7 +1383,7 @@ public boolean supportsSkipLocked() {
13831383

13841384
@Override
13851385
public RowLockStrategy getWriteRowLockStrategy() {
1386-
return RowLockStrategy.COLUMN_NAME;
1386+
return RowLockStrategy.COLUMN;
13871387
}
13881388

13891389
@Override

‎hibernate-core/src/main/java/org/hibernate/LockMode.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import jakarta.persistence.FindOption;
88
import jakarta.persistence.LockModeType;
99
import jakarta.persistence.RefreshOption;
10+
import org.hibernate.jpa.HibernateHints;
1011
import org.hibernate.jpa.internal.util.LockModeTypeHelper;
1112

1213
import java.util.Locale;
@@ -158,10 +159,15 @@ public enum LockMode implements FindOption, RefreshOption {
158159
* as {@link #PESSIMISTIC_WRITE}. If the lock is not immediately
159160
* available, an exception occurs.
160161
*
161-
* @apiNote To be removed in a future version. A different approach to
162-
* specifying handling for locked rows will be introduced.
162+
* @apiNote This lock-mode is intended for use as a JPA
163+
* {@linkplain HibernateHints#HINT_NATIVE_LOCK_MODE query hint}.
164+
* Other cases should use the combination of
165+
* {@linkplain #PESSIMISTIC_WRITE} and {@linkplain Timeouts#NO_WAIT}
166+
* as find/refresh options - e.g. {@code session.find(Book.class, 1, PESSIMISTIC_WRITE, NO_WAIT)}
167+
*
168+
* @see #PESSIMISTIC_WRITE
169+
* @see Timeouts#NO_WAIT
163170
*/
164-
@Remove
165171
UPGRADE_NOWAIT,
166172

167173
/**
@@ -172,10 +178,15 @@ public enum LockMode implements FindOption, RefreshOption {
172178
* immediately available, no exception occurs, but the locked
173179
* row is not returned from the database.
174180
*
175-
* @apiNote To be removed in a future version. A different approach to
176-
* specifying handling for locked rows will be introduced.
181+
* @apiNote This lock-mode is intended for use as a JPA
182+
* {@linkplain HibernateHints#HINT_NATIVE_LOCK_MODE query hint}.
183+
* Other cases should use the combination of
184+
* {@linkplain #PESSIMISTIC_WRITE} and {@linkplain Timeouts#SKIP_LOCKED}
185+
* as find/refresh options - e.g. {@code session.find(Book.class, 1, PESSIMISTIC_WRITE, SKIP_LOCKED)}
186+
*
187+
* @see #PESSIMISTIC_WRITE
188+
* @see Timeouts#NO_WAIT
177189
*/
178-
@Remove
179190
UPGRADE_SKIPLOCKED;
180191

181192
/**

‎hibernate-core/src/main/java/org/hibernate/LockOptions.java

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public class LockOptions implements Serializable {
8888
*
8989
* @see LockMode#NONE
9090
* @see Timeouts#WAIT_FOREVER
91+
* @see Locking.Scope#ROOT_ONLY
92+
* @see Locking.FollowOn#ALLOW
9193
*/
9294
public LockOptions() {
9395
immutable = false;
@@ -104,6 +106,8 @@ public LockOptions() {
104106
* @param lockMode The initial lock mode
105107
*
106108
* @see Timeouts#WAIT_FOREVER
109+
* @see Locking.Scope#ROOT_ONLY
110+
* @see Locking.FollowOn#ALLOW
107111
*/
108112
public LockOptions(LockMode lockMode) {
109113
immutable = false;
@@ -119,6 +123,9 @@ public LockOptions(LockMode lockMode) {
119123
*
120124
* @param lockMode The initial lock mode
121125
* @param timeout The initial timeout, in milliseconds
126+
*
127+
* @see Locking.Scope#ROOT_ONLY
128+
* @see Locking.FollowOn#ALLOW
122129
*/
123130
public LockOptions(LockMode lockMode, Timeout timeout) {
124131
immutable = false;
@@ -135,6 +142,8 @@ public LockOptions(LockMode lockMode, Timeout timeout) {
135142
* @param lockMode The initial lock mode
136143
* @param timeout The initial timeout
137144
* @param jpaScope The initial lock scope
145+
*
146+
* @see Locking.FollowOn#ALLOW
138147
*/
139148
public LockOptions(LockMode lockMode, Timeout timeout, PessimisticLockScope jpaScope) {
140149
immutable = false;
@@ -146,6 +155,10 @@ public LockOptions(LockMode lockMode, Timeout timeout, PessimisticLockScope jpaS
146155

147156
/**
148157
* Internal operation used to create immutable global instances.
158+
*
159+
* @see Timeouts#WAIT_FOREVER
160+
* @see Locking.Scope#ROOT_ONLY
161+
* @see Locking.FollowOn#ALLOW
149162
*/
150163
protected LockOptions(boolean immutable, LockMode lockMode) {
151164
this.immutable = immutable;
@@ -288,6 +301,7 @@ public Locking.FollowOn getFollowOnStrategy() {
288301
* @see #getFollowOnStrategy()
289302
*/
290303
public LockOptions setFollowOnStrategy(Locking.FollowOn followOnStrategy) {
304+
assert followOnStrategy != null;
291305
this.followOnStrategy = followOnStrategy;
292306
return this;
293307
}
@@ -393,14 +407,16 @@ public LockOptions setLockScope(PessimisticLockScope jpaScope) {
393407
* cases where Hibernate determines it would need to use follow-on
394408
* locking.
395409
*
410+
* @see Locking.FollowOn#asLegacyValue()
396411
* @see org.hibernate.jpa.HibernateHints#HINT_FOLLOW_ON_LOCKING
397412
* @see org.hibernate.dialect.Dialect#useFollowOnLocking(String, org.hibernate.query.spi.QueryOptions)
398413
*
399414
* @deprecated Use {@linkplain #getFollowOnStrategy()} instead.
400415
*/
401416
@Deprecated(since = "7.1")
402417
public Boolean getFollowOnLocking() {
403-
return followOnStrategy == Locking.FollowOn.ALLOW;
418+
assert followOnStrategy != null;
419+
return followOnStrategy.asLegacyValue();
404420
}
405421

406422
/**
@@ -410,21 +426,15 @@ public Boolean getFollowOnLocking() {
410426
* @param followOnLocking The new follow-on locking setting
411427
* @return {@code this} for method chaining
412428
*
429+
* @see org.hibernate.Locking.FollowOn#fromLegacyValue
413430
* @see org.hibernate.jpa.HibernateHints#HINT_FOLLOW_ON_LOCKING
414431
* @see org.hibernate.dialect.Dialect#useFollowOnLocking(String, org.hibernate.query.spi.QueryOptions)
415432
*
416433
* @deprecated Use {@linkplain #setFollowOnStrategy} instead.
417434
*/
418435
@Deprecated(since = "7.1")
419436
public LockOptions setFollowOnLocking(Boolean followOnLocking) {
420-
if ( followOnLocking == null ) {
421-
this.followOnStrategy = Locking.FollowOn.ALLOW;
422-
}
423-
else {
424-
this.followOnStrategy = followOnLocking
425-
? Locking.FollowOn.FORCE
426-
: Locking.FollowOn.DISALLOW;
427-
}
437+
followOnStrategy = Locking.FollowOn.fromLegacyValue( followOnLocking );
428438
return this;
429439
}
430440

‎hibernate-core/src/main/java/org/hibernate/Locking.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,41 @@ enum FollowOn implements FindOption, LockOption, RefreshOption {
132132
*
133133
* @apiNote This may lead to exceptions from the database.
134134
*/
135-
FORCE
135+
FORCE;
136+
137+
/**
138+
* Interprets the follow-on strategy into the legacy boolean values.
139+
*
140+
* @return {@code true} if {@linkplain #FORCE}; {@code false} if {@linkplain #DISALLOW};
141+
* {@code null} otherwise.
142+
*
143+
* @see #fromLegacyValue
144+
*/
145+
public Boolean asLegacyValue() {
146+
return switch ( this ) {
147+
case FORCE -> true;
148+
case DISALLOW -> false;
149+
default -> null;
150+
};
151+
}
152+
153+
/**
154+
* Given a legacy boolean value, interpret the follow-on strategy.
155+
*
156+
* @return {@linkplain #FORCE} if {@code true};
157+
* {@linkplain #DISALLOW} if {@code false};
158+
* {@linkplain #ALLOW} otherwise.
159+
*
160+
* @see #asLegacyValue()
161+
*/
162+
public static FollowOn fromLegacyValue(Boolean value) {
163+
if ( value == Boolean.TRUE ) {
164+
return FORCE;
165+
}
166+
if ( value == Boolean.FALSE ) {
167+
return DISALLOW;
168+
}
169+
return ALLOW;
170+
}
136171
}
137172
}

‎hibernate-core/src/main/java/org/hibernate/boot/model/internal/QueryHintDefinition.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,7 @@ private Locking.FollowOn followOnStrategy() {
179179
}
180180

181181
final Boolean lockingValue = getBooleanWrapper( HibernateHints.HINT_FOLLOW_ON_LOCKING );
182-
if ( lockingValue == null || lockingValue == Boolean.TRUE ) {
183-
return Locking.FollowOn.ALLOW;
184-
}
185-
else {
186-
return Locking.FollowOn.DISALLOW;
187-
}
182+
return Locking.FollowOn.fromLegacyValue( lockingValue );
188183
}
189184

190185
private LockOptions determineLockOptions(

‎hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1438,7 +1438,7 @@ public boolean supportsSkipLocked() {
14381438

14391439
@Override
14401440
public RowLockStrategy getWriteRowLockStrategy() {
1441-
return RowLockStrategy.COLUMN_NAME;
1441+
return RowLockStrategy.COLUMN;
14421442
}
14431443

14441444
@Override

‎hibernate-core/src/main/java/org/hibernate/dialect/RowLockStrategy.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@ public enum RowLockStrategy {
1515
* Use the column reference (column name qualified by the table alias).
1616
*/
1717
COLUMN,
18-
/**
19-
* Use the column name.
20-
*/
21-
COLUMN_NAME,
2218
/**
2319
* Use the table alias.
2420
*/

‎hibernate-core/src/main/java/org/hibernate/jpa/QueryHints.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public final class QueryHints {
8484
public static final String JAKARTA_HINT_LOADGRAPH = SpecHints.HINT_SPEC_LOAD_GRAPH;
8585

8686
/**
87-
* @see HibernateHints#HINT_FOLLOW_ON_LOCKING
87+
* @see HibernateHints#HINT_FOLLOW_ON_STRATEGY
8888
*/
8989
public static final String HINT_FOLLOW_ON_STRATEGY = HibernateHints.HINT_FOLLOW_ON_STRATEGY;
9090

‎hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import java.util.Map;
6161
import java.util.Set;
6262

63-
import static java.lang.Boolean.FALSE;
6463
import static java.util.Arrays.asList;
6564
import static java.util.Locale.ROOT;
6665
import static org.hibernate.internal.log.DeprecationLogger.DEPRECATION_LOGGER;
@@ -424,7 +423,6 @@ private boolean applyLockingHint(String hintName, Object value) {
424423
}
425424
return true;
426425
case HINT_FOLLOW_ON_LOCKING:
427-
DEPRECATION_LOGGER.deprecatedHint( HINT_FOLLOW_ON_LOCKING, HINT_FOLLOW_ON_STRATEGY );
428426
applyFollowOnLockingHint( getBoolean( value ) );
429427
return true;
430428
case HINT_NATIVE_LOCKMODE:
@@ -489,12 +487,8 @@ protected void applyFollowOnStrategyHint(Locking.FollowOn followOnStrategy) {
489487
}
490488

491489
protected void applyFollowOnLockingHint(Boolean followOnLocking) {
492-
if ( followOnLocking == FALSE ) {
493-
applyFollowOnStrategyHint( Locking.FollowOn.DISALLOW );
494-
}
495-
else {
496-
applyFollowOnStrategyHint( Locking.FollowOn.ALLOW );
497-
}
490+
DEPRECATION_LOGGER.deprecatedHint( HINT_FOLLOW_ON_LOCKING, HINT_FOLLOW_ON_STRATEGY );
491+
applyFollowOnStrategyHint( Locking.FollowOn.fromLegacyValue( followOnLocking ) );
498492
}
499493

500494

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
import static org.hibernate.jpa.HibernateHints.HINT_CACHE_REGION;
9898
import static org.hibernate.jpa.HibernateHints.HINT_FETCH_SIZE;
9999
import static org.hibernate.jpa.HibernateHints.HINT_FOLLOW_ON_LOCKING;
100+
import static org.hibernate.jpa.HibernateHints.HINT_FOLLOW_ON_STRATEGY;
100101
import static org.hibernate.jpa.HibernateHints.HINT_READ_ONLY;
101102
import static org.hibernate.jpa.LegacySpecHints.HINT_JAVAEE_CACHE_RETRIEVE_MODE;
102103
import static org.hibernate.jpa.LegacySpecHints.HINT_JAVAEE_CACHE_STORE_MODE;
@@ -826,6 +827,7 @@ protected void collectHints(Map<String, Object> hints) {
826827
hints.put( appliedGraph.getSemantic().getJpaHintName(), appliedGraph );
827828
}
828829

830+
putIfNotNull( hints, HINT_FOLLOW_ON_STRATEGY, getQueryOptions().getLockOptions().getFollowOnStrategy() );
829831
putIfNotNull( hints, HINT_FOLLOW_ON_LOCKING, getQueryOptions().getLockOptions().getFollowOnLocking() );
830832
}
831833

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import static org.hibernate.jpa.HibernateHints.HINT_CACHE_REGION;
7575
import static org.hibernate.jpa.HibernateHints.HINT_FETCH_SIZE;
7676
import static org.hibernate.jpa.HibernateHints.HINT_FOLLOW_ON_LOCKING;
77+
import static org.hibernate.jpa.HibernateHints.HINT_FOLLOW_ON_STRATEGY;
7778
import static org.hibernate.jpa.HibernateHints.HINT_READ_ONLY;
7879
import static org.hibernate.jpa.LegacySpecHints.HINT_JAVAEE_CACHE_RETRIEVE_MODE;
7980
import static org.hibernate.jpa.LegacySpecHints.HINT_JAVAEE_CACHE_STORE_MODE;
@@ -677,6 +678,7 @@ protected void collectHints(Map<String, Object> hints) {
677678
hints.put( appliedGraph.getSemantic().getJpaHintName(), appliedGraph );
678679
}
679680

681+
putIfNotNull( hints, HINT_FOLLOW_ON_STRATEGY, getQueryOptions().getLockOptions().getFollowOnStrategy() );
680682
putIfNotNull( hints, HINT_FOLLOW_ON_LOCKING, getQueryOptions().getLockOptions().getFollowOnLocking() );
681683
}
682684

‎hibernate-core/src/main/java/org/hibernate/sql/ast/internal/StandardLockingClauseStrategy.java

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55
package org.hibernate.sql.ast.internal;
66

7-
import org.hibernate.HibernateException;
87
import org.hibernate.Locking;
98
import org.hibernate.dialect.Dialect;
109
import org.hibernate.dialect.RowLockStrategy;
@@ -198,9 +197,6 @@ private void collectLockItems(TableGroup tableGroup, List<String> lockItems) {
198197
else if ( rowLockStrategy == RowLockStrategy.COLUMN ) {
199198
addColumnRefs( tableGroup, lockItems );
200199
}
201-
else if ( rowLockStrategy == RowLockStrategy.COLUMN_NAME ) {
202-
addColumnNames( tableGroup, lockItems );
203-
}
204200
}
205201

206202
private void addTableAliases(TableGroup tableGroup, List<String> lockItems) {
@@ -223,6 +219,10 @@ private String[] determineKeyColumnRefs(TableGroup tableGroup) {
223219
final String[] result = determineKeyColumnNames( tableGroup.getModelPart() );
224220
final String tableAlias = tableGroup.getPrimaryTableReference().getIdentificationVariable();
225221
for ( int i = 0; i < result.length; i++ ) {
222+
// NOTE: in some tests with Oracle, the qualifiers are being applied twice;
223+
// still need to track that down. possibly, unexpected calls to
224+
// `Dialect#applyLocksToSql`?
225+
assert !result[i].contains( "." );
226226
result[i] = tableAlias + "." + result[i];
227227
}
228228
return result;
@@ -233,8 +233,6 @@ private String[] determineKeyColumnNames(ModelPart modelPart) {
233233
return entityPersister.getIdentifierColumnNames();
234234
}
235235
else if ( modelPart instanceof PluralAttributeMapping pluralAttributeMapping ) {
236-
// todo : seems like this ought to return the column name(s)
237-
// to then be qualified with the table alias
238236
return pluralAttributeMapping.getCollectionDescriptor().getKeyColumnAliases( null );
239237
}
240238
else if ( modelPart instanceof EntityAssociationMapping entityAssociationMapping ) {
@@ -244,31 +242,4 @@ else if ( modelPart instanceof EntityAssociationMapping entityAssociationMapping
244242
return null;
245243
}
246244
}
247-
248-
private void addColumnNames(TableGroup tableGroup, List<String> lockItems) {
249-
final ModelPart keyColumnPart = determineKeyPart( tableGroup.getModelPart() );
250-
if ( keyColumnPart == null ) {
251-
throw new HibernateException( "Could not determine ModelPart defining key columns - " + tableGroup.getModelPart() );
252-
}
253-
keyColumnPart.forEachSelectable( (selectionIndex, selectableMapping) -> {
254-
if ( !selectableMapping.isFormula() ) {
255-
lockItems.add( selectableMapping.getSelectableName() );
256-
}
257-
} );
258-
}
259-
260-
private ModelPart determineKeyPart(ModelPart modelPart) {
261-
if ( modelPart instanceof EntityPersister entityPersister ) {
262-
return entityPersister.getIdentifierMapping();
263-
}
264-
else if ( modelPart instanceof PluralAttributeMapping pluralAttributeMapping ) {
265-
return pluralAttributeMapping.getKeyDescriptor();
266-
}
267-
else if ( modelPart instanceof EntityAssociationMapping entityAssociationMapping ) {
268-
return entityAssociationMapping.getForeignKeyDescriptor();
269-
}
270-
else {
271-
return null;
272-
}
273-
}
274245
}

0 commit comments

Comments
 (0)