Skip to content

Commit 537c8c1

Browse files
fix: recognise set operation quantifiers written in any case (#2614)
The grammar records a set operation's modifier verbatim from the source, so `union all` arrives as "all" while `isAll()` asked whether the modifier contained "ALL". Every spelling but upper case therefore read as an unqualified UNION, silently dropping ALL semantics — and the same for DISTINCT, across UNION, EXCEPT, INTERSECT and MINUS alike. A regression from 5fe938b, which replaced the boolean `all` and `distinct` fields with a single verbatim modifier string. The round-trip tests all spell their SQL in upper case, so none of them noticed. Match the leading keyword case-insensitively instead. The match is anchored at the start of the modifier and must end on a word boundary, so a `CORRESPONDING BY NAME MATCHING(all)` column list is no longer mistaken for the ALL quantifier the way a bare `contains` would. `setAll` and `setDistinct` wrote their keyword whatever they were passed, so `setAll(false)` left an ALL modifier behind. They now honour the argument.
1 parent 74d1611 commit 537c8c1

2 files changed

Lines changed: 87 additions & 4 deletions

File tree

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,35 @@ public String getModifier() {
2020
}
2121

2222
public boolean isAll() {
23-
return modifier != null && modifier.contains("ALL");
23+
return leadsWith("ALL");
2424
}
2525

2626
public void setAll(boolean all) {
27-
this.modifier = "ALL";
27+
this.modifier = all ? "ALL" : "";
2828
}
2929

3030
public boolean isDistinct() {
31-
return modifier != null && modifier.contains("DISTINCT");
31+
return leadsWith("DISTINCT");
3232
}
3333

3434
public void setDistinct(boolean distinct) {
35-
this.modifier = "DISTINCT";
35+
this.modifier = distinct ? "DISTINCT" : "";
36+
}
37+
38+
/**
39+
* Whether the modifier opens with {@code keyword}, which the grammar records verbatim from the
40+
* source and so may be written in any case. Anchored at the start and required to end on a word
41+
* boundary, so a {@code CORRESPONDING BY NAME MATCHING(all)} column list cannot be mistaken for
42+
* the ALL quantifier.
43+
*/
44+
private boolean leadsWith(String keyword) {
45+
if (modifier == null) {
46+
return false;
47+
}
48+
String trimmed = modifier.trim();
49+
return trimmed.regionMatches(true, 0, keyword, 0, keyword.length())
50+
&& (trimmed.length() == keyword.length()
51+
|| !Character.isLetterOrDigit(trimmed.charAt(keyword.length())));
3652
}
3753

3854
private final SetOperationType type;

src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import net.sf.jsqlparser.JSQLParserException;
1717
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
1818
import net.sf.jsqlparser.statement.Statement;
19+
import org.junit.jupiter.api.Test;
1920
import org.junit.jupiter.api.parallel.Execution;
2021
import org.junit.jupiter.api.parallel.ExecutionMode;
2122
import org.junit.jupiter.params.ParameterizedTest;
@@ -50,6 +51,72 @@ void testSetOperationModifierRoundTrip(String sql) throws JSQLParserException {
5051
assertSqlCanBeParsedAndDeparsed(sql);
5152
}
5253

54+
/**
55+
* The grammar records the modifier verbatim from the source, so the quantifier survives in
56+
* whatever case it was written. Every case spells the same set operation.
57+
*
58+
* @see <a href="https://github.com/JSQLParser/JSqlParser/issues/2419">#2419</a>
59+
*/
60+
@ParameterizedTest
61+
@ValueSource(strings = {"ALL", "all", "All", "aLL"})
62+
void testAllModifierRecognisedInAnyCase(String quantifier) throws JSQLParserException {
63+
SetOperation operation = firstOperation(
64+
"SELECT a FROM t1 UNION " + quantifier + " SELECT a FROM t2");
65+
assertTrue(operation.isAll(), "UNION " + quantifier + " should be an ALL union");
66+
assertFalse(operation.isDistinct(), "UNION " + quantifier + " is not DISTINCT");
67+
}
68+
69+
@ParameterizedTest
70+
@ValueSource(strings = {"DISTINCT", "distinct", "Distinct"})
71+
void testDistinctModifierRecognisedInAnyCase(String quantifier) throws JSQLParserException {
72+
SetOperation operation = firstOperation(
73+
"SELECT a FROM t1 UNION " + quantifier + " SELECT a FROM t2");
74+
assertTrue(operation.isDistinct(), "UNION " + quantifier + " should be a DISTINCT union");
75+
assertFalse(operation.isAll(), "UNION " + quantifier + " is not ALL");
76+
}
77+
78+
@ParameterizedTest
79+
@ValueSource(strings = {"EXCEPT", "INTERSECT", "MINUS"})
80+
void testAllModifierRecognisedInAnyCaseForEverySetOperation(String setOperation)
81+
throws JSQLParserException {
82+
assertTrue(firstOperation("SELECT a FROM t1 " + setOperation + " all SELECT a FROM t2")
83+
.isAll(), setOperation + " all should be an ALL operation");
84+
}
85+
86+
/**
87+
* A plain set operation carries no quantifier, and neither predicate may claim one.
88+
*/
89+
@Test
90+
void testUnqualifiedSetOperationIsNeitherAllNorDistinct() throws JSQLParserException {
91+
SetOperation operation = firstOperation("SELECT a FROM t1 UNION SELECT a FROM t2");
92+
assertFalse(operation.isAll());
93+
assertFalse(operation.isDistinct());
94+
}
95+
96+
/**
97+
* The setters record what they are told rather than the keyword they are named after.
98+
*/
99+
@Test
100+
void testSettersHonourTheirArgument() {
101+
UnionOp union = new UnionOp();
102+
103+
union.setAll(true);
104+
assertTrue(union.isAll());
105+
union.setAll(false);
106+
assertFalse(union.isAll(), "setAll(false) must not leave an ALL modifier behind");
107+
108+
union.setDistinct(true);
109+
assertTrue(union.isDistinct());
110+
union.setDistinct(false);
111+
assertFalse(union.isDistinct(), "setDistinct(false) must not leave a DISTINCT modifier");
112+
}
113+
114+
private static SetOperation firstOperation(String sql) throws JSQLParserException {
115+
Statement statement = CCJSqlParserUtil.parse(sql);
116+
assertInstanceOf(SetOperationList.class, statement);
117+
return ((SetOperationList) statement).getOperations().get(0);
118+
}
119+
53120
@ParameterizedTest
54121
@MethodSource("provideModifierLeakCases")
55122
void testModifierDoesNotLeakBetweenOperators(String sql, String forbidden)

0 commit comments

Comments
 (0)