Skip to content

Commit

Permalink
fix: FOR UPDATE clause should come after the select body
Browse files Browse the repository at this point in the history 
- fixes #1995
- disable faulty Oracle test `for_update08`

Signed-off-by: Andreas Reichel <andreas@manticore-projects.com>
  • Loading branch information
@manticore-projects
manticore-projects committed Apr 20, 2024
1 parent f417c8f commit cf7fe15
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 133 deletions.
109 changes: 8 additions & 101 deletions src/main/java/net/sf/jsqlparser/statement/select/PlainSelect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
*/
package net.sf.jsqlparser.statement.select;

import static java.util.stream.Collectors.joining;
import net.sf.jsqlparser.expression.Alias;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.expression.OracleHierarchicalExpression;
import net.sf.jsqlparser.expression.OracleHint;
import net.sf.jsqlparser.expression.WindowDefinition;
import net.sf.jsqlparser.schema.Table;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -18,12 +23,8 @@
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import net.sf.jsqlparser.expression.Alias;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.expression.OracleHierarchicalExpression;
import net.sf.jsqlparser.expression.OracleHint;
import net.sf.jsqlparser.expression.WindowDefinition;
import net.sf.jsqlparser.schema.Table;

import static java.util.stream.Collectors.joining;

@SuppressWarnings({"PMD.CyclomaticComplexity"})
public class PlainSelect extends Select {
Expand All @@ -45,15 +46,10 @@ public class PlainSelect extends Select {
private Top top;
private OracleHierarchicalExpression oracleHierarchical = null;
private OracleHint oracleHint = null;
private ForMode forMode = null;
private Table forUpdateTable = null;
private boolean skipLocked;
private Wait wait;
private boolean mySqlSqlCalcFoundRows = false;
private MySqlSqlCacheFlags mySqlCacheFlag = null;
private String forXmlPath;
private KSQLWindow ksqlWindow = null;
private boolean noWait = false;
private boolean emitChanges = false;

private List<WindowDefinition> windowDefinitions;
Expand Down Expand Up @@ -360,22 +356,6 @@ public void setOracleHierarchical(OracleHierarchicalExpression oracleHierarchica
this.oracleHierarchical = oracleHierarchical;
}

public ForMode getForMode() {
return forMode;
}

public void setForMode(ForMode forMode) {
this.forMode = forMode;
}

public Table getForUpdateTable() {
return forUpdateTable;
}

public void setForUpdateTable(Table forUpdateTable) {
this.forUpdateTable = forUpdateTable;
}

public OracleHint getOracleHint() {
return oracleHint;
}
Expand All @@ -384,24 +364,6 @@ public void setOracleHint(OracleHint oracleHint) {
this.oracleHint = oracleHint;
}

/**
* Sets the {@link Wait} for this SELECT
*
* @param wait the {@link Wait} for this SELECT
*/
public void setWait(final Wait wait) {
this.wait = wait;
}

/**
* Returns the value of the {@link Wait} set for this SELECT
*
* @return the value of the {@link Wait} set for this SELECT
*/
public Wait getWait() {
return wait;
}

public String getForXmlPath() {
return forXmlPath;
}
Expand Down Expand Up @@ -434,14 +396,6 @@ public void setWindowDefinitions(List<WindowDefinition> windowDefinitions) {
this.windowDefinitions = windowDefinitions;
}

public boolean isSkipLocked() {
return skipLocked;
}

public void setSkipLocked(boolean skipLocked) {
this.skipLocked = skipLocked;
}

@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.ExcessiveMethodLength",
"PMD.NPathComplexity"})
public StringBuilder appendSelectBodyTo(StringBuilder builder) {
Expand Down Expand Up @@ -538,25 +492,6 @@ public StringBuilder appendSelectBodyTo(StringBuilder builder) {
if (emitChanges) {
builder.append(" EMIT CHANGES");
}
if (getForMode() != null) {
builder.append(" FOR ");
builder.append(getForMode().getValue());

if (forUpdateTable != null) {
builder.append(" OF ").append(forUpdateTable);
}

if (wait != null) {
// Wait's toString will do the formatting for us
builder.append(wait);
}

if (isNoWait()) {
builder.append(" NOWAIT");
} else if (isSkipLocked()) {
builder.append(" SKIP LOCKED");
}
}
} else {
// without from
if (where != null) {
Expand Down Expand Up @@ -616,14 +551,6 @@ public MySqlSqlCacheFlags getMySqlSqlCacheFlag() {
return this.mySqlCacheFlag;
}

public void setNoWait(boolean noWait) {
this.noWait = noWait;
}

public boolean isNoWait() {
return this.noWait;
}

public PlainSelect withDistinct(Distinct distinct) {
this.setDistinct(distinct);
return this;
Expand Down Expand Up @@ -679,16 +606,6 @@ public PlainSelect withOracleSiblings(boolean oracleSiblings) {
return this;
}

public PlainSelect withForMode(ForMode forMode) {
this.setForMode(forMode);
return this;
}

public PlainSelect withForUpdateTable(Table forUpdateTable) {
this.setForUpdateTable(forUpdateTable);
return this;
}

public PlainSelect withForXmlPath(String forXmlPath) {
this.setForXmlPath(forXmlPath);
return this;
Expand All @@ -704,21 +621,11 @@ public PlainSelect withNoWait(boolean noWait) {
return this;
}

public PlainSelect withSkipLocked(boolean skipLocked) {
this.setSkipLocked(skipLocked);
return this;
}

public PlainSelect withHaving(Expression having) {
this.setHaving(having);
return this;
}

public PlainSelect withWait(Wait wait) {
this.setWait(wait);
return this;
}

public PlainSelect addSelectItems(Collection<? extends SelectItem<?>> selectItems) {
List<SelectItem<?>> collection =
Optional.ofNullable(getSelectItems()).orElseGet(ArrayList::new);
Expand Down
99 changes: 97 additions & 2 deletions src/main/java/net/sf/jsqlparser/statement/select/Select.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.expression.ExpressionVisitor;
import net.sf.jsqlparser.parser.ASTNodeAccessImpl;
import net.sf.jsqlparser.schema.Table;
import net.sf.jsqlparser.statement.Statement;
import net.sf.jsqlparser.statement.StatementVisitor;

Expand All @@ -23,6 +24,7 @@
import java.util.Optional;

public abstract class Select extends ASTNodeAccessImpl implements Statement, Expression {
protected Table forUpdateTable = null;
List<WithItem> withItemsList;
Limit limitBy;
Limit limit;
Expand All @@ -34,6 +36,10 @@ public abstract class Select extends ASTNodeAccessImpl implements Statement, Exp
ForClause forClause = null;

List<OrderByElement> orderByElements;
ForMode forMode = null;
private boolean skipLocked;
private Wait wait;
private boolean noWait = false;

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

if (sql.length() > 0) {
if (expression.length() > 0) {
if (!sql.isEmpty()) {
if (!expression.isEmpty()) {
sql = " " + expression + " " + sql;
} else {
sql = " " + sql;
Expand Down Expand Up @@ -152,6 +158,14 @@ public void setOracleSiblings(boolean oracleSiblings) {
this.oracleSiblings = oracleSiblings;
}

public void setNoWait(boolean noWait) {
this.noWait = noWait;
}

public boolean isNoWait() {
return this.noWait;
}

public Select withOracleSiblings(boolean oracleSiblings) {
this.setOracleSiblings(oracleSiblings);
return this;
Expand Down Expand Up @@ -255,6 +269,48 @@ public Select withIsolation(WithIsolation isolation) {
return this;
}

public ForMode getForMode() {
return this.forMode;
}

public void setForMode(ForMode forMode) {
this.forMode = forMode;
}

public Table getForUpdateTable() {
return this.forUpdateTable;
}

public void setForUpdateTable(Table forUpdateTable) {
this.forUpdateTable = forUpdateTable;
}

/**
* Sets the {@link Wait} for this SELECT
*
* @param wait the {@link Wait} for this SELECT
*/
public void setWait(final Wait wait) {
this.wait = wait;
}

/**
* Returns the value of the {@link Wait} set for this SELECT
*
* @return the value of the {@link Wait} set for this SELECT
*/
public Wait getWait() {
return wait;
}

public boolean isSkipLocked() {
return skipLocked;
}

public void setSkipLocked(boolean skipLocked) {
this.skipLocked = skipLocked;
}

public abstract StringBuilder appendSelectBodyTo(StringBuilder builder);

@SuppressWarnings({"PMD.CyclomaticComplexity"})
Expand Down Expand Up @@ -294,6 +350,25 @@ public StringBuilder appendTo(StringBuilder builder) {
if (isolation != null) {
builder.append(isolation);
}
if (forMode != null) {
builder.append(" FOR ");
builder.append(forMode.getValue());

if (getForUpdateTable() != null) {
builder.append(" OF ").append(forUpdateTable);
}

if (wait != null) {
// Wait's toString will do the formatting for us
builder.append(wait);
}

if (isNoWait()) {
builder.append(" NOWAIT");
} else if (isSkipLocked()) {
builder.append(" SKIP LOCKED");
}
}

return builder;
}
Expand Down Expand Up @@ -334,4 +409,24 @@ public SetOperationList getSetOperationList() {
public <E extends Select> E as(Class<E> type) {
return type.cast(this);
}

public Select withForMode(ForMode forMode) {
this.setForMode(forMode);
return this;
}

public Select withForUpdateTable(Table forUpdateTable) {
this.setForUpdateTable(forUpdateTable);
return this;
}

public Select withSkipLocked(boolean skipLocked) {
this.setSkipLocked(skipLocked);
return this;
}

public Select withWait(Wait wait) {
this.setWait(wait);
return this;
}
}
4 changes: 2 additions & 2 deletions src/main/java/net/sf/jsqlparser/util/deparser/AbstractDeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ protected AbstractDeParser(StringBuilder buffer) {

public static void deparseUpdateSets(List<UpdateSet> updateSets, StringBuilder buffer,
ExpressionVisitor visitor) {
ExpressionListDeParser expressionListDeParser =
new ExpressionListDeParser(visitor, buffer);
ExpressionListDeParser<?> expressionListDeParser =
new ExpressionListDeParser<>(visitor, buffer);
int j = 0;
if (updateSets != null) {
for (UpdateSet updateSet : updateSets) {
Expand Down
34 changes: 17 additions & 17 deletions src/main/java/net/sf/jsqlparser/util/deparser/SelectDeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,23 +276,6 @@ public void visit(PlainSelect plainSelect) {
buffer.append(plainSelect.getWindowDefinitions().stream()
.map(WindowDefinition::toString).collect(joining(", ")));
}
if (plainSelect.getForMode() != null) {
buffer.append(" FOR ");
buffer.append(plainSelect.getForMode().getValue());

if (plainSelect.getForUpdateTable() != null) {
buffer.append(" OF ").append(plainSelect.getForUpdateTable());
}
if (plainSelect.getWait() != null) {
// wait's toString will do the formatting for us
buffer.append(plainSelect.getWait());
}
if (plainSelect.isNoWait()) {
buffer.append(" NOWAIT");
} else if (plainSelect.isSkipLocked()) {
buffer.append(" SKIP LOCKED");
}
}
if (plainSelect.getForClause() != null) {
plainSelect.getForClause().appendTo(buffer);
}
Expand All @@ -319,6 +302,23 @@ public void visit(PlainSelect plainSelect) {
if (plainSelect.getIsolation() != null) {
buffer.append(plainSelect.getIsolation().toString());
}
if (plainSelect.getForMode() != null) {
buffer.append(" FOR ");
buffer.append(plainSelect.getForMode().getValue());

if (plainSelect.getForUpdateTable() != null) {
buffer.append(" OF ").append(plainSelect.getForUpdateTable());
}
if (plainSelect.getWait() != null) {
// wait's toString will do the formatting for us
buffer.append(plainSelect.getWait());
}
if (plainSelect.isNoWait()) {
buffer.append(" NOWAIT");
} else if (plainSelect.isSkipLocked()) {
buffer.append(" SKIP LOCKED");
}
}
if (plainSelect.getOptimizeFor() != null) {
deparseOptimizeFor(plainSelect.getOptimizeFor());
}
Expand Down
Loading

0 comments on commit cf7fe15

Please sign in to comment.