Skip to content

Commit 64bf8af

Browse files
author
mhyeon-lee
committed
[DATAJDBC-493] Fix PostgresDialect Lock clause with Root table name
1 parent bf77aaa commit 64bf8af

File tree

11 files changed

+110
-52
lines changed

11 files changed

+110
-52
lines changed

spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ public CharSequence apply(Select select) {
211211
return "";
212212
}
213213

214-
return clause.getLock(new LockOptions(lockMode));
214+
return clause.getLock(new LockOptions(lockMode, select.getFrom()));
215215
}
216216
}
217217

spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@
1717

1818
import lombok.RequiredArgsConstructor;
1919

20+
import org.springframework.data.relational.core.sql.From;
2021
import org.springframework.data.relational.core.sql.IdentifierProcessing;
2122
import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing;
2223
import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting;
2324
import org.springframework.data.relational.core.sql.LockOptions;
25+
import org.springframework.data.relational.core.sql.Table;
2426
import org.springframework.util.Assert;
2527
import org.springframework.util.ClassUtils;
2628

29+
import java.util.List;
30+
2731
/**
2832
* An SQL dialect for Postgres.
2933
*
@@ -79,37 +83,6 @@ public Position getClausePosition() {
7983
}
8084
};
8185

82-
private static final LockClause LOCK_CLAUSE = new LockClause() {
83-
84-
/*
85-
* (non-Javadoc)
86-
* @see org.springframework.data.relational.core.dialect.LockClause#getLock(LockOptions)
87-
*/
88-
@Override
89-
public String getLock(LockOptions lockOptions) {
90-
switch (lockOptions.getLockMode()) {
91-
92-
case PESSIMISTIC_WRITE:
93-
return "FOR UPDATE";
94-
95-
case PESSIMISTIC_READ:
96-
return "FOR SHARE";
97-
98-
default:
99-
return "";
100-
}
101-
}
102-
103-
/*
104-
* (non-Javadoc)
105-
* @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition()
106-
*/
107-
@Override
108-
public Position getClausePosition() {
109-
return Position.AFTER_ORDER_BY;
110-
}
111-
};
112-
11386
private final PostgresArrayColumns ARRAY_COLUMNS = new PostgresArrayColumns();
11487

11588
/*
@@ -121,6 +94,8 @@ public LimitClause limit() {
12194
return LIMIT_CLAUSE;
12295
}
12396

97+
private final PostgresLockClause LOCK_CLAUSE = new PostgresLockClause(this.getIdentifierProcessing());
98+
12499
/*
125100
* (non-Javadoc)
126101
* @see org.springframework.data.relational.core.dialect.Dialect#lock()
@@ -139,6 +114,50 @@ public ArrayColumns getArraySupport() {
139114
return ARRAY_COLUMNS;
140115
}
141116

117+
static class PostgresLockClause implements LockClause {
118+
private final IdentifierProcessing identifierProcessing;
119+
120+
PostgresLockClause(IdentifierProcessing identifierProcessing) {
121+
this.identifierProcessing = identifierProcessing;
122+
}
123+
124+
/*
125+
* (non-Javadoc)
126+
* @see org.springframework.data.relational.core.dialect.LockClause#getLock(LockOptions)
127+
*/
128+
@Override
129+
public String getLock(LockOptions lockOptions) {
130+
131+
List<Table> tables = lockOptions.getFrom().getTables();
132+
if (tables.isEmpty()) {
133+
return "";
134+
}
135+
136+
String tableName = tables.get(0).getName().toSql(this.identifierProcessing);
137+
138+
switch (lockOptions.getLockMode()) {
139+
140+
case PESSIMISTIC_WRITE:
141+
return "FOR UPDATE OF " + tableName;
142+
143+
case PESSIMISTIC_READ:
144+
return "FOR SHARE OF " + tableName;
145+
146+
default:
147+
return "";
148+
}
149+
}
150+
151+
/*
152+
* (non-Javadoc)
153+
* @see org.springframework.data.relational.core.dialect.LockClause#getClausePosition()
154+
*/
155+
@Override
156+
public Position getClausePosition() {
157+
return Position.AFTER_ORDER_BY;
158+
}
159+
};
160+
142161
@RequiredArgsConstructor
143162
static class PostgresArrayColumns implements ArrayColumns {
144163

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/DefaultSelect.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@ class DefaultSelect implements Select {
5656
this.lockMode = lockMode;
5757
}
5858

59+
/*
60+
* (non-Javadoc)
61+
* @see org.springframework.data.relational.core.sql.Select#getFrom()
62+
*/
63+
@Override
64+
public From getFrom() {
65+
return this.from;
66+
}
67+
5968
/*
6069
* (non-Javadoc)
6170
* @see org.springframework.data.relational.core.sql.Select#getOrderBy()

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/From.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.relational.core.sql;
1717

1818
import java.util.Arrays;
19+
import java.util.Collections;
1920
import java.util.List;
2021

2122
import org.springframework.util.StringUtils;
@@ -38,7 +39,11 @@ public class From extends AbstractSegment {
3839

3940
super(tables.toArray(new Table[] {}));
4041

41-
this.tables = tables;
42+
this.tables = Collections.unmodifiableList(tables);
43+
}
44+
45+
public List<Table> getTables() {
46+
return this.tables;
4247
}
4348

4449
/*

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/LockOptions.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,18 @@
2323
*/
2424
public class LockOptions {
2525
private final LockMode lockMode;
26+
private final From from;
2627

27-
public LockOptions(LockMode lockMode) {
28+
public LockOptions(LockMode lockMode, From from) {
2829
this.lockMode = lockMode;
30+
this.from = from;
2931
}
3032

3133
public LockMode getLockMode() {
3234
return this.lockMode;
3335
}
36+
37+
public From getFrom() {
38+
return this.from;
39+
}
3440
}

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Select.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ static SelectBuilder builder() {
4949
return new DefaultSelectBuilder();
5050
}
5151

52+
From getFrom();
53+
5254
/**
5355
* @return the {@link List} of {@link OrderByField ORDER BY} fields.
5456
*/

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/HsqlDbDialectUnitTests.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,17 @@
1515
*/
1616
package org.springframework.data.relational.core.dialect;
1717

18-
import static org.assertj.core.api.Assertions.*;
19-
2018
import org.junit.Test;
19+
import org.springframework.data.relational.core.sql.From;
2120
import org.springframework.data.relational.core.sql.LockMode;
2221
import org.springframework.data.relational.core.sql.LockOptions;
2322

23+
import static org.assertj.core.api.Assertions.assertThat;
24+
import static org.mockito.Mockito.mock;
25+
2426
/**
2527
* Unit tests for the {@link HsqlDbDialect}.
26-
*
28+
*
2729
* @author Jens Schauder
2830
* @author Myeonghyeon Lee
2931
*/
@@ -74,7 +76,8 @@ public void shouldQuoteIdentifiersUsingBackticks() {
7476
public void shouldRenderLock() {
7577

7678
LockClause limit = HsqlDbDialect.INSTANCE.lock();
77-
LockOptions lockOptions = new LockOptions(LockMode.PESSIMISTIC_WRITE);
79+
From from = mock(From.class);
80+
LockOptions lockOptions = new LockOptions(LockMode.PESSIMISTIC_WRITE, from);
7881

7982
assertThat(limit.getLock(lockOptions)).isEqualTo("FOR UPDATE");
8083
assertThat(limit.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY);

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
*/
1616
package org.springframework.data.relational.core.dialect;
1717

18-
import static org.assertj.core.api.Assertions.*;
19-
2018
import org.junit.Test;
19+
import org.springframework.data.relational.core.sql.From;
2120
import org.springframework.data.relational.core.sql.LockMode;
2221
import org.springframework.data.relational.core.sql.LockOptions;
2322

23+
import static org.assertj.core.api.Assertions.assertThat;
24+
import static org.mockito.Mockito.mock;
25+
2426
/**
2527
* Unit tests for {@link MySqlDialect}.
2628
*
@@ -75,9 +77,10 @@ public void shouldQuoteIdentifiersUsingBackticks() {
7577
public void shouldRenderLock() {
7678

7779
LockClause lock = MySqlDialect.INSTANCE.lock();
80+
From from = mock(From.class);
7881

79-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE");
80-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("LOCK IN SHARE MODE");
82+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("FOR UPDATE");
83+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("LOCK IN SHARE MODE");
8184
assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY);
8285
}
8386
}

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public void shouldRenderSelectWithLockWrite() {
109109

110110
String sql = SqlRenderer.create(factory.createRenderContext()).render(select);
111111

112-
assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE");
112+
assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR UPDATE OF foo");
113113
}
114114

115115
@Test // DATAJDBC-498
@@ -121,7 +121,7 @@ public void shouldRenderSelectWithLockRead() {
121121

122122
String sql = SqlRenderer.create(factory.createRenderContext()).render(select);
123123

124-
assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR SHARE");
124+
assertThat(sql).isEqualTo("SELECT foo.* FROM foo FOR SHARE OF foo");
125125
}
126126

127127
@Test // DATAJDBC-498
@@ -133,7 +133,7 @@ public void shouldRenderSelectWithLimitWithLockWrite() {
133133

134134
String sql = SqlRenderer.create(factory.createRenderContext()).render(select);
135135

136-
assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE");
136+
assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR UPDATE OF foo");
137137
}
138138

139139
@Test // DATAJDBC-498
@@ -145,6 +145,6 @@ public void shouldRenderSelectWithLimitWithLockRead() {
145145

146146
String sql = SqlRenderer.create(factory.createRenderContext()).render(select);
147147

148-
assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE");
148+
assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE OF foo");
149149
}
150150
}

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectUnitTests.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@
1717

1818
import static org.assertj.core.api.Assertions.*;
1919
import static org.assertj.core.api.SoftAssertions.*;
20+
import static org.mockito.Mockito.*;
2021

2122
import org.junit.Test;
23+
import org.springframework.data.relational.core.sql.From;
2224
import org.springframework.data.relational.core.sql.LockMode;
2325
import org.springframework.data.relational.core.sql.LockOptions;
26+
import org.springframework.data.relational.core.sql.Table;
27+
28+
import java.util.Collections;
2429

2530
/**
2631
* Unit tests for {@link PostgresDialect}.
@@ -79,9 +84,11 @@ public void shouldRenderLimitOffset() {
7984
public void shouldRenderLock() {
8085

8186
LockClause lock = PostgresDialect.INSTANCE.lock();
87+
From from = mock(From.class);
88+
when(from.getTables()).thenReturn(Collections.singletonList(Table.create("dummy_table")));
8289

83-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("FOR UPDATE");
84-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("FOR SHARE");
90+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("FOR UPDATE OF dummy_table");
91+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("FOR SHARE OF dummy_table");
8592
assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_ORDER_BY);
8693
}
8794
}

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectUnitTests.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
*/
1616
package org.springframework.data.relational.core.dialect;
1717

18-
import static org.assertj.core.api.Assertions.*;
19-
2018
import org.junit.Test;
19+
import org.springframework.data.relational.core.sql.From;
2120
import org.springframework.data.relational.core.sql.LockMode;
2221
import org.springframework.data.relational.core.sql.LockOptions;
2322

23+
import static org.assertj.core.api.Assertions.assertThat;
24+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
25+
import static org.mockito.Mockito.mock;
26+
2427
/**
2528
* Unit tests for {@link SqlServerDialect}.
2629
*
@@ -67,9 +70,10 @@ public void shouldRenderLimitOffset() {
6770
public void shouldRenderLock() {
6871

6972
LockClause lock = SqlServerDialect.INSTANCE.lock();
73+
From from = mock(From.class);
7074

71-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE))).isEqualTo("WITH (UPDLOCK, ROWLOCK)");
72-
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ))).isEqualTo("WITH (HOLDLOCK, ROWLOCK)");
75+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_WRITE, from))).isEqualTo("WITH (UPDLOCK, ROWLOCK)");
76+
assertThat(lock.getLock(new LockOptions(LockMode.PESSIMISTIC_READ, from))).isEqualTo("WITH (HOLDLOCK, ROWLOCK)");
7377
assertThat(lock.getClausePosition()).isEqualTo(LockClause.Position.AFTER_FROM_TABLE);
7478
}
7579
}

0 commit comments

Comments
 (0)