Skip to content

Commit cf7fe15

Browse files
fix: FOR UPDATE clause should come after the select body
- fixes #1995 - disable faulty Oracle test `for_update08` Signed-off-by: Andreas Reichel <andreas@manticore-projects.com>
1 parent f417c8f commit cf7fe15

8 files changed

Lines changed: 165 additions & 133 deletions

File tree

src/main/java/net/sf/jsqlparser/statement/select/PlainSelect.java

Lines changed: 8 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
*/
1010
package net.sf.jsqlparser.statement.select;
1111

12-
import static java.util.stream.Collectors.joining;
12+
import net.sf.jsqlparser.expression.Alias;
13+
import net.sf.jsqlparser.expression.Expression;
14+
import net.sf.jsqlparser.expression.OracleHierarchicalExpression;
15+
import net.sf.jsqlparser.expression.OracleHint;
16+
import net.sf.jsqlparser.expression.WindowDefinition;
17+
import net.sf.jsqlparser.schema.Table;
1318

1419
import java.util.ArrayList;
1520
import java.util.Arrays;
@@ -18,12 +23,8 @@
1823
import java.util.Iterator;
1924
import java.util.List;
2025
import java.util.Optional;
21-
import net.sf.jsqlparser.expression.Alias;
22-
import net.sf.jsqlparser.expression.Expression;
23-
import net.sf.jsqlparser.expression.OracleHierarchicalExpression;
24-
import net.sf.jsqlparser.expression.OracleHint;
25-
import net.sf.jsqlparser.expression.WindowDefinition;
26-
import net.sf.jsqlparser.schema.Table;
26+
27+
import static java.util.stream.Collectors.joining;
2728

2829
@SuppressWarnings({"PMD.CyclomaticComplexity"})
2930
public class PlainSelect extends Select {
@@ -45,15 +46,10 @@ public class PlainSelect extends Select {
4546
private Top top;
4647
private OracleHierarchicalExpression oracleHierarchical = null;
4748
private OracleHint oracleHint = null;
48-
private ForMode forMode = null;
49-
private Table forUpdateTable = null;
50-
private boolean skipLocked;
51-
private Wait wait;
5249
private boolean mySqlSqlCalcFoundRows = false;
5350
private MySqlSqlCacheFlags mySqlCacheFlag = null;
5451
private String forXmlPath;
5552
private KSQLWindow ksqlWindow = null;
56-
private boolean noWait = false;
5753
private boolean emitChanges = false;
5854

5955
private List<WindowDefinition> windowDefinitions;
@@ -360,22 +356,6 @@ public void setOracleHierarchical(OracleHierarchicalExpression oracleHierarchica
360356
this.oracleHierarchical = oracleHierarchical;
361357
}
362358

363-
public ForMode getForMode() {
364-
return forMode;
365-
}
366-
367-
public void setForMode(ForMode forMode) {
368-
this.forMode = forMode;
369-
}
370-
371-
public Table getForUpdateTable() {
372-
return forUpdateTable;
373-
}
374-
375-
public void setForUpdateTable(Table forUpdateTable) {
376-
this.forUpdateTable = forUpdateTable;
377-
}
378-
379359
public OracleHint getOracleHint() {
380360
return oracleHint;
381361
}
@@ -384,24 +364,6 @@ public void setOracleHint(OracleHint oracleHint) {
384364
this.oracleHint = oracleHint;
385365
}
386366

387-
/**
388-
* Sets the {@link Wait} for this SELECT
389-
*
390-
* @param wait the {@link Wait} for this SELECT
391-
*/
392-
public void setWait(final Wait wait) {
393-
this.wait = wait;
394-
}
395-
396-
/**
397-
* Returns the value of the {@link Wait} set for this SELECT
398-
*
399-
* @return the value of the {@link Wait} set for this SELECT
400-
*/
401-
public Wait getWait() {
402-
return wait;
403-
}
404-
405367
public String getForXmlPath() {
406368
return forXmlPath;
407369
}
@@ -434,14 +396,6 @@ public void setWindowDefinitions(List<WindowDefinition> windowDefinitions) {
434396
this.windowDefinitions = windowDefinitions;
435397
}
436398

437-
public boolean isSkipLocked() {
438-
return skipLocked;
439-
}
440-
441-
public void setSkipLocked(boolean skipLocked) {
442-
this.skipLocked = skipLocked;
443-
}
444-
445399
@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.ExcessiveMethodLength",
446400
"PMD.NPathComplexity"})
447401
public StringBuilder appendSelectBodyTo(StringBuilder builder) {
@@ -538,25 +492,6 @@ public StringBuilder appendSelectBodyTo(StringBuilder builder) {
538492
if (emitChanges) {
539493
builder.append(" EMIT CHANGES");
540494
}
541-
if (getForMode() != null) {
542-
builder.append(" FOR ");
543-
builder.append(getForMode().getValue());
544-
545-
if (forUpdateTable != null) {
546-
builder.append(" OF ").append(forUpdateTable);
547-
}
548-
549-
if (wait != null) {
550-
// Wait's toString will do the formatting for us
551-
builder.append(wait);
552-
}
553-
554-
if (isNoWait()) {
555-
builder.append(" NOWAIT");
556-
} else if (isSkipLocked()) {
557-
builder.append(" SKIP LOCKED");
558-
}
559-
}
560495
} else {
561496
// without from
562497
if (where != null) {
@@ -616,14 +551,6 @@ public MySqlSqlCacheFlags getMySqlSqlCacheFlag() {
616551
return this.mySqlCacheFlag;
617552
}
618553

619-
public void setNoWait(boolean noWait) {
620-
this.noWait = noWait;
621-
}
622-
623-
public boolean isNoWait() {
624-
return this.noWait;
625-
}
626-
627554
public PlainSelect withDistinct(Distinct distinct) {
628555
this.setDistinct(distinct);
629556
return this;
@@ -679,16 +606,6 @@ public PlainSelect withOracleSiblings(boolean oracleSiblings) {
679606
return this;
680607
}
681608

682-
public PlainSelect withForMode(ForMode forMode) {
683-
this.setForMode(forMode);
684-
return this;
685-
}
686-
687-
public PlainSelect withForUpdateTable(Table forUpdateTable) {
688-
this.setForUpdateTable(forUpdateTable);
689-
return this;
690-
}
691-
692609
public PlainSelect withForXmlPath(String forXmlPath) {
693610
this.setForXmlPath(forXmlPath);
694611
return this;
@@ -704,21 +621,11 @@ public PlainSelect withNoWait(boolean noWait) {
704621
return this;
705622
}
706623

707-
public PlainSelect withSkipLocked(boolean skipLocked) {
708-
this.setSkipLocked(skipLocked);
709-
return this;
710-
}
711-
712624
public PlainSelect withHaving(Expression having) {
713625
this.setHaving(having);
714626
return this;
715627
}
716628

717-
public PlainSelect withWait(Wait wait) {
718-
this.setWait(wait);
719-
return this;
720-
}
721-
722629
public PlainSelect addSelectItems(Collection<? extends SelectItem<?>> selectItems) {
723630
List<SelectItem<?>> collection =
724631
Optional.ofNullable(getSelectItems()).orElseGet(ArrayList::new);

src/main/java/net/sf/jsqlparser/statement/select/Select.java

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import net.sf.jsqlparser.expression.Expression;
1313
import net.sf.jsqlparser.expression.ExpressionVisitor;
1414
import net.sf.jsqlparser.parser.ASTNodeAccessImpl;
15+
import net.sf.jsqlparser.schema.Table;
1516
import net.sf.jsqlparser.statement.Statement;
1617
import net.sf.jsqlparser.statement.StatementVisitor;
1718

@@ -23,6 +24,7 @@
2324
import java.util.Optional;
2425

2526
public abstract class Select extends ASTNodeAccessImpl implements Statement, Expression {
27+
protected Table forUpdateTable = null;
2628
List<WithItem> withItemsList;
2729
Limit limitBy;
2830
Limit limit;
@@ -34,6 +36,10 @@ public abstract class Select extends ASTNodeAccessImpl implements Statement, Exp
3436
ForClause forClause = null;
3537

3638
List<OrderByElement> orderByElements;
39+
ForMode forMode = null;
40+
private boolean skipLocked;
41+
private Wait wait;
42+
private boolean noWait = false;
3743

3844
public static String orderByToString(List<OrderByElement> orderByElements) {
3945
return orderByToString(false, orderByElements);
@@ -52,8 +58,8 @@ public static String getFormattedList(List<?> list, String expression, boolean u
5258
boolean useBrackets) {
5359
String sql = getStringList(list, useComma, useBrackets);
5460

55-
if (sql.length() > 0) {
56-
if (expression.length() > 0) {
61+
if (!sql.isEmpty()) {
62+
if (!expression.isEmpty()) {
5763
sql = " " + expression + " " + sql;
5864
} else {
5965
sql = " " + sql;
@@ -152,6 +158,14 @@ public void setOracleSiblings(boolean oracleSiblings) {
152158
this.oracleSiblings = oracleSiblings;
153159
}
154160

161+
public void setNoWait(boolean noWait) {
162+
this.noWait = noWait;
163+
}
164+
165+
public boolean isNoWait() {
166+
return this.noWait;
167+
}
168+
155169
public Select withOracleSiblings(boolean oracleSiblings) {
156170
this.setOracleSiblings(oracleSiblings);
157171
return this;
@@ -255,6 +269,48 @@ public Select withIsolation(WithIsolation isolation) {
255269
return this;
256270
}
257271

272+
public ForMode getForMode() {
273+
return this.forMode;
274+
}
275+
276+
public void setForMode(ForMode forMode) {
277+
this.forMode = forMode;
278+
}
279+
280+
public Table getForUpdateTable() {
281+
return this.forUpdateTable;
282+
}
283+
284+
public void setForUpdateTable(Table forUpdateTable) {
285+
this.forUpdateTable = forUpdateTable;
286+
}
287+
288+
/**
289+
* Sets the {@link Wait} for this SELECT
290+
*
291+
* @param wait the {@link Wait} for this SELECT
292+
*/
293+
public void setWait(final Wait wait) {
294+
this.wait = wait;
295+
}
296+
297+
/**
298+
* Returns the value of the {@link Wait} set for this SELECT
299+
*
300+
* @return the value of the {@link Wait} set for this SELECT
301+
*/
302+
public Wait getWait() {
303+
return wait;
304+
}
305+
306+
public boolean isSkipLocked() {
307+
return skipLocked;
308+
}
309+
310+
public void setSkipLocked(boolean skipLocked) {
311+
this.skipLocked = skipLocked;
312+
}
313+
258314
public abstract StringBuilder appendSelectBodyTo(StringBuilder builder);
259315

260316
@SuppressWarnings({"PMD.CyclomaticComplexity"})
@@ -294,6 +350,25 @@ public StringBuilder appendTo(StringBuilder builder) {
294350
if (isolation != null) {
295351
builder.append(isolation);
296352
}
353+
if (forMode != null) {
354+
builder.append(" FOR ");
355+
builder.append(forMode.getValue());
356+
357+
if (getForUpdateTable() != null) {
358+
builder.append(" OF ").append(forUpdateTable);
359+
}
360+
361+
if (wait != null) {
362+
// Wait's toString will do the formatting for us
363+
builder.append(wait);
364+
}
365+
366+
if (isNoWait()) {
367+
builder.append(" NOWAIT");
368+
} else if (isSkipLocked()) {
369+
builder.append(" SKIP LOCKED");
370+
}
371+
}
297372

298373
return builder;
299374
}
@@ -334,4 +409,24 @@ public SetOperationList getSetOperationList() {
334409
public <E extends Select> E as(Class<E> type) {
335410
return type.cast(this);
336411
}
412+
413+
public Select withForMode(ForMode forMode) {
414+
this.setForMode(forMode);
415+
return this;
416+
}
417+
418+
public Select withForUpdateTable(Table forUpdateTable) {
419+
this.setForUpdateTable(forUpdateTable);
420+
return this;
421+
}
422+
423+
public Select withSkipLocked(boolean skipLocked) {
424+
this.setSkipLocked(skipLocked);
425+
return this;
426+
}
427+
428+
public Select withWait(Wait wait) {
429+
this.setWait(wait);
430+
return this;
431+
}
337432
}

src/main/java/net/sf/jsqlparser/util/deparser/AbstractDeParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ protected AbstractDeParser(StringBuilder buffer) {
2828

2929
public static void deparseUpdateSets(List<UpdateSet> updateSets, StringBuilder buffer,
3030
ExpressionVisitor visitor) {
31-
ExpressionListDeParser expressionListDeParser =
32-
new ExpressionListDeParser(visitor, buffer);
31+
ExpressionListDeParser<?> expressionListDeParser =
32+
new ExpressionListDeParser<>(visitor, buffer);
3333
int j = 0;
3434
if (updateSets != null) {
3535
for (UpdateSet updateSet : updateSets) {

src/main/java/net/sf/jsqlparser/util/deparser/SelectDeParser.java

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -276,23 +276,6 @@ public void visit(PlainSelect plainSelect) {
276276
buffer.append(plainSelect.getWindowDefinitions().stream()
277277
.map(WindowDefinition::toString).collect(joining(", ")));
278278
}
279-
if (plainSelect.getForMode() != null) {
280-
buffer.append(" FOR ");
281-
buffer.append(plainSelect.getForMode().getValue());
282-
283-
if (plainSelect.getForUpdateTable() != null) {
284-
buffer.append(" OF ").append(plainSelect.getForUpdateTable());
285-
}
286-
if (plainSelect.getWait() != null) {
287-
// wait's toString will do the formatting for us
288-
buffer.append(plainSelect.getWait());
289-
}
290-
if (plainSelect.isNoWait()) {
291-
buffer.append(" NOWAIT");
292-
} else if (plainSelect.isSkipLocked()) {
293-
buffer.append(" SKIP LOCKED");
294-
}
295-
}
296279
if (plainSelect.getForClause() != null) {
297280
plainSelect.getForClause().appendTo(buffer);
298281
}
@@ -319,6 +302,23 @@ public void visit(PlainSelect plainSelect) {
319302
if (plainSelect.getIsolation() != null) {
320303
buffer.append(plainSelect.getIsolation().toString());
321304
}
305+
if (plainSelect.getForMode() != null) {
306+
buffer.append(" FOR ");
307+
buffer.append(plainSelect.getForMode().getValue());
308+
309+
if (plainSelect.getForUpdateTable() != null) {
310+
buffer.append(" OF ").append(plainSelect.getForUpdateTable());
311+
}
312+
if (plainSelect.getWait() != null) {
313+
// wait's toString will do the formatting for us
314+
buffer.append(plainSelect.getWait());
315+
}
316+
if (plainSelect.isNoWait()) {
317+
buffer.append(" NOWAIT");
318+
} else if (plainSelect.isSkipLocked()) {
319+
buffer.append(" SKIP LOCKED");
320+
}
321+
}
322322
if (plainSelect.getOptimizeFor() != null) {
323323
deparseOptimizeFor(plainSelect.getOptimizeFor());
324324
}

0 commit comments

Comments
 (0)