Skip to content

Commit 92a4962

Browse files
sebersolesebersole
committed
HHH-19925 - Locking root(s) should be based on select-clause, not from-clause
1 parent b4898dd commit 92a4962

File tree

17 files changed

+243
-309
lines changed

17 files changed

+243
-309
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ protected void renderTableGroupJoin(TableGroupJoin tableGroupJoin, List<TableGro
118118
}
119119
appendSql( COMMA_SEPARATOR_CHAR );
120120

121-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
121+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
122122
if ( tableGroupJoin.getPredicate() != null && !tableGroupJoin.getPredicate().isEmpty() ) {
123123
addAdditionalWherePredicate( tableGroupJoin.getPredicate() );
124124
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,15 +166,15 @@ protected void renderTableGroupJoin(TableGroupJoin tableGroupJoin, List<TableGro
166166
// We have to inject the lateral predicate into the sub-query
167167
final Predicate lateralPredicate = this.lateralPredicate;
168168
this.lateralPredicate = predicate;
169-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
169+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
170170
this.lateralPredicate = lateralPredicate;
171171
}
172172
else {
173-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), predicate, tableGroupJoinCollector );
173+
renderJoinedTableGroup( tableGroupJoin, predicate, tableGroupJoinCollector );
174174
}
175175
}
176176
else {
177-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
177+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
178178
}
179179
}
180180

‎hibernate-core/src/main/java/org/hibernate/dialect/sql/ast/DB2SqlAstTranslator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ protected void renderTableGroupJoin(TableGroupJoin tableGroupJoin, List<TableGro
116116
}
117117
appendSql( COMMA_SEPARATOR_CHAR );
118118

119-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
119+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
120120
if ( tableGroupJoin.getPredicate() != null && !tableGroupJoin.getPredicate().isEmpty() ) {
121121
addAdditionalWherePredicate( tableGroupJoin.getPredicate() );
122122
}

‎hibernate-core/src/main/java/org/hibernate/dialect/sql/ast/SQLServerSqlAstTranslator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,15 @@ protected void renderTableGroupJoin(TableGroupJoin tableGroupJoin, List<TableGro
167167
// We have to inject the lateral predicate into the sub-query
168168
final Predicate lateralPredicate = this.lateralPredicate;
169169
this.lateralPredicate = predicate;
170-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
170+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
171171
this.lateralPredicate = lateralPredicate;
172172
}
173173
else {
174-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), predicate, tableGroupJoinCollector );
174+
renderJoinedTableGroup( tableGroupJoin, predicate, tableGroupJoinCollector );
175175
}
176176
}
177177
else {
178-
renderJoinedTableGroup( tableGroupJoin.getJoinedGroup(), null, tableGroupJoinCollector );
178+
renderJoinedTableGroup( tableGroupJoin, null, tableGroupJoinCollector );
179179
}
180180
}
181181

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

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.hibernate.sql.ast.tree.from.TableGroup;
1313
import org.hibernate.sql.ast.tree.from.TableGroupJoin;
1414

15+
import java.util.Collection;
1516
import java.util.HashSet;
1617
import java.util.Set;
1718

@@ -32,15 +33,42 @@ public AbstractLockingClauseStrategy(
3233
}
3334

3435
@Override
35-
public boolean shouldLockRoot(TableGroup root) {
36+
public boolean registerRoot(TableGroup root) {
37+
if ( shouldLockRoot( root ) ) {
38+
trackRoot( root );
39+
return true;
40+
}
41+
else {
42+
return false;
43+
}
44+
}
45+
46+
protected boolean shouldLockRoot(TableGroup root) {
3647
// NOTE : the NavigablePath can be null in some cases.
3748
// we don't care about these cases, so easier to just
3849
// handle the nullness here
3950
return root.getNavigablePath() != null && rootsForLocking.contains( root.getNavigablePath() );
4051
}
4152

53+
protected void trackRoot(TableGroup root) {
54+
if ( pathsToLock == null ) {
55+
pathsToLock = new HashSet<>();
56+
}
57+
pathsToLock.add( root.getNavigablePath() );
58+
}
59+
4260
@Override
43-
public boolean shouldLockJoin(TableGroup joinedGroup) {
61+
public boolean registerJoin(TableGroupJoin join) {
62+
if ( shouldLockJoin( join.getJoinedGroup() ) ) {
63+
trackJoin( join );
64+
return true;
65+
}
66+
else {
67+
return false;
68+
}
69+
}
70+
71+
protected boolean shouldLockJoin(TableGroup joinedGroup) {
4472
// we only want to consider applying locks to joins in 2 cases:
4573
// 1) It is a root path for locking (aka occurs in the domain select-clause)
4674
// 2) It's left-hand side is to be locked
@@ -72,39 +100,22 @@ protected boolean isRootForLocking(TableGroup joinedGroup) {
72100

73101
protected boolean isLhsLocked(TableGroup joinedGroup) {
74102
// todo (pessimistic-locking) : The use of NavigablePath#parent for LHS here is not ideal.
75-
// However, the only alternative is to change the method signature to pass the
103+
// However, the alternative is to change the method signature to pass the
76104
// join's LHS which would have a broad impact on Dialects and translators.
77-
// I'm sure this will miss some cases, but let's start here fow now and deal with
78-
// these other cases as they come up.
105+
// This will possibly miss some cases, but let's start here fow now.
79106
return pathsToLock != null
80107
&& pathsToLock.contains( joinedGroup.getNavigablePath().getParent() );
81108
}
82109

83-
@Override
84-
public void registerRoot(TableGroup root) {
85-
if ( shouldLockRoot( root ) ) {
86-
trackRoot( root );
87-
}
88-
}
89-
90-
protected void trackRoot(TableGroup root) {
110+
protected void trackJoin(TableGroupJoin join) {
91111
if ( pathsToLock == null ) {
92112
pathsToLock = new HashSet<>();
93113
}
94-
pathsToLock.add( root.getNavigablePath() );
114+
pathsToLock.add( join.getNavigablePath() );
95115
}
96116

97117
@Override
98-
public void registerJoin(TableGroupJoin joinedGroup) {
99-
if ( shouldLockJoin( joinedGroup.getJoinedGroup() ) ) {
100-
trackJoin( joinedGroup );
101-
}
102-
}
103-
104-
protected void trackJoin(TableGroupJoin join) {
105-
if ( pathsToLock == null ) {
106-
pathsToLock = new HashSet<>();
107-
}
108-
pathsToLock.add( join.getNavigablePath() );
118+
public Collection<NavigablePath> getPathsToLock() {
119+
return pathsToLock;
109120
}
110121
}

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

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

7+
import org.hibernate.spi.NavigablePath;
78
import org.hibernate.sql.ast.spi.LockingClauseStrategy;
89
import org.hibernate.sql.ast.spi.SqlAppender;
910
import org.hibernate.sql.ast.tree.from.TableGroup;
@@ -23,23 +24,15 @@ public class NonLockingClauseStrategy implements LockingClauseStrategy {
2324
public static final NonLockingClauseStrategy NON_CLAUSE_STRATEGY = new NonLockingClauseStrategy();
2425

2526
@Override
26-
public boolean shouldLockRoot(TableGroup root) {
27-
return false;
28-
}
29-
30-
@Override
31-
public boolean shouldLockJoin(TableGroup joinedGroup) {
32-
return false;
33-
}
34-
35-
@Override
36-
public void registerRoot(TableGroup root) {
27+
public boolean registerRoot(TableGroup root) {
3728
// nothing to do
29+
return false;
3830
}
3931

4032
@Override
41-
public void registerJoin(TableGroupJoin joinedGroup) {
33+
public boolean registerJoin(TableGroupJoin join) {
4234
// nothing to do
35+
return false;
4336
}
4437

4538
@Override
@@ -53,12 +46,7 @@ public void render(SqlAppender sqlAppender) {
5346
}
5447

5548
@Override
56-
public Collection<TableGroup> getRootsToLock() {
57-
return List.of();
58-
}
59-
60-
@Override
61-
public Collection<TableGroupJoin> getJoinsToLock() {
49+
public Collection<NavigablePath> getPathsToLock() {
6250
return List.of();
6351
}
6452
}

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

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.hibernate.sql.model.TableMapping;
3030

3131
import java.util.ArrayList;
32-
import java.util.Collection;
3332
import java.util.HashSet;
3433
import java.util.LinkedHashSet;
3534
import java.util.List;
@@ -69,25 +68,15 @@ public StandardLockingClauseStrategy(
6968
}
7069

7170
@Override
72-
public Collection<TableGroup> getRootsToLock() {
73-
return rootsToLock;
74-
}
75-
76-
@Override
77-
public Collection<TableGroupJoin> getJoinsToLock() {
78-
return joinsToLock;
79-
}
80-
81-
@Override
82-
public void registerRoot(TableGroup root) {
71+
public boolean registerRoot(TableGroup root) {
8372
if ( !queryHasOuterJoins ) {
8473
if ( CollectionHelper.isNotEmpty( root.getTableReferenceJoins() ) ) {
8574
// joined inheritance and/or secondary tables - inherently has outer joins
8675
queryHasOuterJoins = true;
8776
}
8877
}
8978

90-
super.registerRoot( root );
79+
return super.registerRoot( root );
9180
}
9281

9382
@Override
@@ -100,9 +89,9 @@ protected void trackRoot(TableGroup root) {
10089
}
10190

10291
@Override
103-
public void registerJoin(TableGroupJoin joinedGroup) {
104-
checkForOuterJoins( joinedGroup );
105-
super.registerJoin( joinedGroup );
92+
public boolean registerJoin(TableGroupJoin join) {
93+
checkForOuterJoins( join );
94+
return super.registerJoin( join );
10695
}
10796

10897
@Override
@@ -172,13 +161,13 @@ private String collectLockItems() {
172161
}
173162

174163
final List<String> lockItems = new ArrayList<>();
175-
if ( getRootsToLock() != null ) {
176-
for ( TableGroup root : getRootsToLock() ) {
164+
if ( rootsToLock != null ) {
165+
for ( TableGroup root : rootsToLock ) {
177166
collectLockItems( root, lockItems );
178167
}
179168
}
180-
if ( getJoinsToLock() != null ) {
181-
for ( TableGroupJoin join : getJoinsToLock() ) {
169+
if ( joinsToLock != null ) {
170+
for ( TableGroupJoin join : joinsToLock ) {
182171
collectLockItems( join.getJoinedGroup(), lockItems );
183172
}
184173
}

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@
77
import org.hibernate.Locking;
88
import org.hibernate.spi.NavigablePath;
99
import org.hibernate.sql.ast.spi.SqlAppender;
10-
import org.hibernate.sql.ast.tree.from.TableGroup;
11-
import org.hibernate.sql.ast.tree.from.TableGroupJoin;
1210

13-
import java.util.Collection;
14-
import java.util.List;
1511
import java.util.Set;
1612

13+
/// LockingClauseStrategy implementation for T-SQL (SQL Server and Sybase)
14+
///
1715
/// @author Steve Ebersole
1816
public class TransactSQLLockingClauseStrategy extends AbstractLockingClauseStrategy {
1917

@@ -31,14 +29,4 @@ public boolean containsOuterJoins() {
3129
public void render(SqlAppender sqlAppender) {
3230
// not used for T-SQL dialects
3331
}
34-
35-
@Override
36-
public Collection<TableGroup> getRootsToLock() {
37-
return List.of();
38-
}
39-
40-
@Override
41-
public Collection<TableGroupJoin> getJoinsToLock() {
42-
return List.of();
43-
}
4432
}

0 commit comments

Comments
 (0)