Skip to content

Commit 7fc300f

Browse files
queeliusclaude
andauthored
fix(parser): pass modifier to ExceptOp/MinusOp constructors (#2419) (#2420)
* fix(parser): pass modifier to ExceptOp/MinusOp constructors and reset between iterations EXCEPT ALL/DISTINCT and MINUS ALL/DISTINCT modifiers were silently dropped during parsing because the grammar captured the modifier via SetOperationModifier() but constructed ExceptOp and MinusOp with their no-arg constructors (defaulting to empty string), unlike UnionOp and IntersectOp which correctly received the modifier. Additionally, the modifier variable was not reset between iterations of the set-operation loop, causing modifiers to leak from one operator to the next (e.g., UNION ALL ... EXCEPT would incorrectly make the EXCEPT inherit ALL). Fixes #2419 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: convert to parameterized tests, run spotlessApply Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c0e1d05 commit 7fc300f

File tree

2 files changed

+96
-3
lines changed

2 files changed

+96
-3
lines changed

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5093,7 +5093,7 @@ Select SetOperationList(Select select) #SetOperationList: {
50935093
selects.add(select);
50945094
}
50955095

5096-
( LOOKAHEAD(2) (
5096+
( LOOKAHEAD(2) { modifier = null; } (
50975097
(
50985098
<K_UNION> [ modifier=SetOperationModifier() ] { UnionOp union = new UnionOp(modifier); linkAST(union,jjtThis); operations.add(union); }
50995099

@@ -5104,11 +5104,11 @@ Select SetOperationList(Select select) #SetOperationList: {
51045104
)
51055105
|
51065106
(
5107-
<K_MINUS> [ modifier=SetOperationModifier() ] { MinusOp minus = new MinusOp(); linkAST(minus,jjtThis); operations.add(minus); }
5107+
<K_MINUS> [ modifier=SetOperationModifier() ] { MinusOp minus = new MinusOp(modifier); linkAST(minus,jjtThis); operations.add(minus); }
51085108
)
51095109
|
51105110
(
5111-
<K_EXCEPT> [ modifier=SetOperationModifier() ] { ExceptOp except = new ExceptOp(); linkAST(except,jjtThis); operations.add(except); }
5111+
<K_EXCEPT> [ modifier=SetOperationModifier() ] { ExceptOp except = new ExceptOp(modifier); linkAST(except,jjtThis); operations.add(except); }
51125112
)
51135113

51145114
)
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*-
2+
* #%L
3+
* JSQLParser library
4+
* %%
5+
* Copyright (C) 2004 - 2019 JSQLParser
6+
* %%
7+
* Dual licensed under GNU LGPL 2.1 or Apache License 2.0
8+
* #L%
9+
*/
10+
package net.sf.jsqlparser.statement.select;
11+
12+
import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed;
13+
import static org.junit.jupiter.api.Assertions.*;
14+
15+
import java.util.stream.Stream;
16+
import net.sf.jsqlparser.JSQLParserException;
17+
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
18+
import net.sf.jsqlparser.statement.Statement;
19+
import org.junit.jupiter.api.parallel.Execution;
20+
import org.junit.jupiter.api.parallel.ExecutionMode;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.Arguments;
23+
import org.junit.jupiter.params.provider.MethodSource;
24+
import org.junit.jupiter.params.provider.ValueSource;
25+
26+
/**
27+
* Regression tests for EXCEPT/MINUS ALL/DISTINCT modifier handling.
28+
* <p>
29+
* Verifies that the ALL and DISTINCT modifiers are correctly preserved during parse-toString
30+
* round-trips for all set operation types: UNION, INTERSECT, EXCEPT, and MINUS.
31+
*
32+
* @see <a href="https://github.com/JSQLParser/JSqlParser/issues/2419">#2419</a>
33+
*/
34+
@Execution(ExecutionMode.CONCURRENT)
35+
public class SetOperationModifierTest {
36+
37+
@ParameterizedTest
38+
@ValueSource(strings = {
39+
"SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2",
40+
"SELECT a FROM t1 EXCEPT DISTINCT SELECT a FROM t2",
41+
"SELECT a FROM t1 EXCEPT SELECT a FROM t2",
42+
"SELECT a FROM t1 MINUS ALL SELECT a FROM t2",
43+
"SELECT a FROM t1 MINUS DISTINCT SELECT a FROM t2",
44+
"SELECT a FROM t1 MINUS SELECT a FROM t2",
45+
"SELECT a FROM t1 UNION ALL SELECT a FROM t2",
46+
"SELECT a FROM t1 INTERSECT ALL SELECT a FROM t2",
47+
"SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT DISTINCT SELECT c FROM t3"
48+
})
49+
void testSetOperationModifierRoundTrip(String sql) throws JSQLParserException {
50+
assertSqlCanBeParsedAndDeparsed(sql);
51+
}
52+
53+
@ParameterizedTest
54+
@MethodSource("provideModifierLeakCases")
55+
void testModifierDoesNotLeakBetweenOperators(String sql, String forbidden)
56+
throws JSQLParserException {
57+
Statement stmt = CCJSqlParserUtil.parse(sql);
58+
String deparsed = stmt.toString();
59+
assertFalse(deparsed.contains(forbidden),
60+
"Modifier leaked: found '" + forbidden + "' in: " + deparsed);
61+
}
62+
63+
private static Stream<Arguments> provideModifierLeakCases() {
64+
return Stream.of(
65+
Arguments.of(
66+
"SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT SELECT c FROM t3",
67+
"EXCEPT ALL"),
68+
Arguments.of(
69+
"SELECT a FROM t1 INTERSECT ALL SELECT b FROM t2 UNION SELECT c FROM t3",
70+
"UNION ALL"));
71+
}
72+
73+
@ParameterizedTest
74+
@MethodSource("provideSetOperationObjectCases")
75+
void testSetOperationObjectState(String sql, Class<?> expectedType,
76+
boolean expectedAll, boolean expectedDistinct) throws JSQLParserException {
77+
SetOperationList setOpList = (SetOperationList) CCJSqlParserUtil.parse(sql);
78+
SetOperation op = setOpList.getOperations().get(0);
79+
assertInstanceOf(expectedType, op);
80+
assertEquals(expectedAll, op.isAll(),
81+
"isAll() mismatch for: " + sql);
82+
assertEquals(expectedDistinct, op.isDistinct(),
83+
"isDistinct() mismatch for: " + sql);
84+
}
85+
86+
private static Stream<Arguments> provideSetOperationObjectCases() {
87+
return Stream.of(
88+
Arguments.of("SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2",
89+
ExceptOp.class, true, false),
90+
Arguments.of("SELECT a FROM t1 MINUS ALL SELECT a FROM t2",
91+
MinusOp.class, true, false));
92+
}
93+
}

0 commit comments

Comments
 (0)