From b3d0b6d74a60114b84883d515e53c71cf168d3f3 Mon Sep 17 00:00:00 2001 From: Alex Towell Date: Wed, 25 Mar 2026 05:47:33 -0500 Subject: [PATCH 1/2] 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) --- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 6 +- .../select/SetOperationModifierTest.java | 132 ++++++++++++++++++ 2 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index ff2de3b85..cb8c9af46 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -5093,7 +5093,7 @@ Select SetOperationList(Select select) #SetOperationList: { selects.add(select); } - ( LOOKAHEAD(2) ( + ( LOOKAHEAD(2) { modifier = null; } ( ( [ modifier=SetOperationModifier() ] { UnionOp union = new UnionOp(modifier); linkAST(union,jjtThis); operations.add(union); } @@ -5104,11 +5104,11 @@ Select SetOperationList(Select select) #SetOperationList: { ) | ( - [ modifier=SetOperationModifier() ] { MinusOp minus = new MinusOp(); linkAST(minus,jjtThis); operations.add(minus); } + [ modifier=SetOperationModifier() ] { MinusOp minus = new MinusOp(modifier); linkAST(minus,jjtThis); operations.add(minus); } ) | ( - [ modifier=SetOperationModifier() ] { ExceptOp except = new ExceptOp(); linkAST(except,jjtThis); operations.add(except); } + [ modifier=SetOperationModifier() ] { ExceptOp except = new ExceptOp(modifier); linkAST(except,jjtThis); operations.add(except); } ) ) diff --git a/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java b/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java new file mode 100644 index 000000000..65fc2a187 --- /dev/null +++ b/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java @@ -0,0 +1,132 @@ +/*- + * #%L + * JSQLParser library + * %% + * Copyright (C) 2004 - 2019 JSQLParser + * %% + * Dual licensed under GNU LGPL 2.1 or Apache License 2.0 + * #L% + */ +package net.sf.jsqlparser.statement.select; + +import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed; +import static org.junit.jupiter.api.Assertions.*; + +import net.sf.jsqlparser.JSQLParserException; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; +import net.sf.jsqlparser.statement.Statement; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +/** + * Regression tests for EXCEPT/MINUS ALL/DISTINCT modifier handling. + *

+ * Verifies that the ALL and DISTINCT modifiers are correctly preserved during + * parse-toString round-trips for all set operation types: UNION, INTERSECT, + * EXCEPT, and MINUS. + * + * @see #2080 + */ +@Execution(ExecutionMode.CONCURRENT) +public class SetOperationModifierTest { + + // ── EXCEPT modifier tests ───────────────────────────────────── + + @Test + public void testExceptAllRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2"); + } + + @Test + public void testExceptDistinctRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT DISTINCT SELECT a FROM t2"); + } + + @Test + public void testPlainExceptRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT SELECT a FROM t2"); + } + + // ── MINUS modifier tests ───────────────────────────────────── + + @Test + public void testMinusAllRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS ALL SELECT a FROM t2"); + } + + @Test + public void testMinusDistinctRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS DISTINCT SELECT a FROM t2"); + } + + @Test + public void testPlainMinusRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS SELECT a FROM t2"); + } + + // ── Cross-check: UNION and INTERSECT still work ────────────── + + @Test + public void testUnionAllRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 UNION ALL SELECT a FROM t2"); + } + + @Test + public void testIntersectAllRoundTrip() throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 INTERSECT ALL SELECT a FROM t2"); + } + + // ── Modifier leak prevention: modifiers must not bleed across operators ── + + @Test + public void testModifierDoesNotLeakFromUnionAllToExcept() throws JSQLParserException { + String sql = "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT SELECT c FROM t3"; + Statement stmt = CCJSqlParserUtil.parse(sql); + String result = stmt.toString(); + // EXCEPT must NOT inherit ALL from the preceding UNION ALL + assertFalse(result.contains("EXCEPT ALL"), + "Modifier should not leak from UNION ALL to a subsequent plain EXCEPT: " + result); + } + + @Test + public void testModifierDoesNotLeakFromIntersectAllToUnion() throws JSQLParserException { + String sql = "SELECT a FROM t1 INTERSECT ALL SELECT b FROM t2 UNION SELECT c FROM t3"; + Statement stmt = CCJSqlParserUtil.parse(sql); + String result = stmt.toString(); + assertFalse(result.contains("UNION ALL"), + "Modifier should not leak from INTERSECT ALL to a subsequent plain UNION: " + result); + } + + @Test + public void testMixedModifiersPreserved() throws JSQLParserException { + // UNION ALL followed by EXCEPT DISTINCT + assertSqlCanBeParsedAndDeparsed( + "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT DISTINCT SELECT c FROM t3"); + } + + // ── SetOperation object state verification ────────────────── + + @Test + public void testExceptAllSetOperationObject() throws JSQLParserException { + String sql = "SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2"; + Statement stmt = CCJSqlParserUtil.parse(sql); + SetOperationList setOpList = (SetOperationList) stmt; + SetOperation op = setOpList.getOperation(0); + + assertInstanceOf(ExceptOp.class, op); + assertTrue(op.isAll(), "ExceptOp should report isAll() == true"); + assertFalse(op.isDistinct(), "ExceptOp should report isDistinct() == false"); + } + + @Test + public void testMinusAllSetOperationObject() throws JSQLParserException { + String sql = "SELECT a FROM t1 MINUS ALL SELECT a FROM t2"; + Statement stmt = CCJSqlParserUtil.parse(sql); + SetOperationList setOpList = (SetOperationList) stmt; + SetOperation op = setOpList.getOperation(0); + + assertInstanceOf(MinusOp.class, op); + assertTrue(op.isAll(), "MinusOp should report isAll() == true"); + } +} From 38741ac5c2d13460b6a8e7449b190a5afb9c2bfa Mon Sep 17 00:00:00 2001 From: Alex Towell Date: Wed, 25 Mar 2026 08:00:11 -0500 Subject: [PATCH 2/2] refactor: convert to parameterized tests, run spotlessApply Co-Authored-By: Claude Opus 4.6 (1M context) --- .../select/SetOperationModifierTest.java | 147 +++++++----------- 1 file changed, 54 insertions(+), 93 deletions(-) diff --git a/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java b/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java index 65fc2a187..a30f3e2fa 100644 --- a/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/select/SetOperationModifierTest.java @@ -12,121 +12,82 @@ import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed; import static org.junit.jupiter.api.Assertions.*; +import java.util.stream.Stream; import net.sf.jsqlparser.JSQLParserException; import net.sf.jsqlparser.parser.CCJSqlParserUtil; import net.sf.jsqlparser.statement.Statement; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; /** * Regression tests for EXCEPT/MINUS ALL/DISTINCT modifier handling. *

- * Verifies that the ALL and DISTINCT modifiers are correctly preserved during - * parse-toString round-trips for all set operation types: UNION, INTERSECT, - * EXCEPT, and MINUS. + * Verifies that the ALL and DISTINCT modifiers are correctly preserved during parse-toString + * round-trips for all set operation types: UNION, INTERSECT, EXCEPT, and MINUS. * - * @see #2080 + * @see #2419 */ @Execution(ExecutionMode.CONCURRENT) public class SetOperationModifierTest { - // ── EXCEPT modifier tests ───────────────────────────────────── - - @Test - public void testExceptAllRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2"); - } - - @Test - public void testExceptDistinctRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT DISTINCT SELECT a FROM t2"); - } - - @Test - public void testPlainExceptRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 EXCEPT SELECT a FROM t2"); - } - - // ── MINUS modifier tests ───────────────────────────────────── - - @Test - public void testMinusAllRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS ALL SELECT a FROM t2"); - } - - @Test - public void testMinusDistinctRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS DISTINCT SELECT a FROM t2"); - } - - @Test - public void testPlainMinusRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 MINUS SELECT a FROM t2"); - } - - // ── Cross-check: UNION and INTERSECT still work ────────────── - - @Test - public void testUnionAllRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 UNION ALL SELECT a FROM t2"); - } - - @Test - public void testIntersectAllRoundTrip() throws JSQLParserException { - assertSqlCanBeParsedAndDeparsed("SELECT a FROM t1 INTERSECT ALL SELECT a FROM t2"); + @ParameterizedTest + @ValueSource(strings = { + "SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2", + "SELECT a FROM t1 EXCEPT DISTINCT SELECT a FROM t2", + "SELECT a FROM t1 EXCEPT SELECT a FROM t2", + "SELECT a FROM t1 MINUS ALL SELECT a FROM t2", + "SELECT a FROM t1 MINUS DISTINCT SELECT a FROM t2", + "SELECT a FROM t1 MINUS SELECT a FROM t2", + "SELECT a FROM t1 UNION ALL SELECT a FROM t2", + "SELECT a FROM t1 INTERSECT ALL SELECT a FROM t2", + "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT DISTINCT SELECT c FROM t3" + }) + void testSetOperationModifierRoundTrip(String sql) throws JSQLParserException { + assertSqlCanBeParsedAndDeparsed(sql); } - // ── Modifier leak prevention: modifiers must not bleed across operators ── - - @Test - public void testModifierDoesNotLeakFromUnionAllToExcept() throws JSQLParserException { - String sql = "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT SELECT c FROM t3"; + @ParameterizedTest + @MethodSource("provideModifierLeakCases") + void testModifierDoesNotLeakBetweenOperators(String sql, String forbidden) + throws JSQLParserException { Statement stmt = CCJSqlParserUtil.parse(sql); - String result = stmt.toString(); - // EXCEPT must NOT inherit ALL from the preceding UNION ALL - assertFalse(result.contains("EXCEPT ALL"), - "Modifier should not leak from UNION ALL to a subsequent plain EXCEPT: " + result); + String deparsed = stmt.toString(); + assertFalse(deparsed.contains(forbidden), + "Modifier leaked: found '" + forbidden + "' in: " + deparsed); } - @Test - public void testModifierDoesNotLeakFromIntersectAllToUnion() throws JSQLParserException { - String sql = "SELECT a FROM t1 INTERSECT ALL SELECT b FROM t2 UNION SELECT c FROM t3"; - Statement stmt = CCJSqlParserUtil.parse(sql); - String result = stmt.toString(); - assertFalse(result.contains("UNION ALL"), - "Modifier should not leak from INTERSECT ALL to a subsequent plain UNION: " + result); + private static Stream provideModifierLeakCases() { + return Stream.of( + Arguments.of( + "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT SELECT c FROM t3", + "EXCEPT ALL"), + Arguments.of( + "SELECT a FROM t1 INTERSECT ALL SELECT b FROM t2 UNION SELECT c FROM t3", + "UNION ALL")); } - @Test - public void testMixedModifiersPreserved() throws JSQLParserException { - // UNION ALL followed by EXCEPT DISTINCT - assertSqlCanBeParsedAndDeparsed( - "SELECT a FROM t1 UNION ALL SELECT b FROM t2 EXCEPT DISTINCT SELECT c FROM t3"); + @ParameterizedTest + @MethodSource("provideSetOperationObjectCases") + void testSetOperationObjectState(String sql, Class expectedType, + boolean expectedAll, boolean expectedDistinct) throws JSQLParserException { + SetOperationList setOpList = (SetOperationList) CCJSqlParserUtil.parse(sql); + SetOperation op = setOpList.getOperations().get(0); + assertInstanceOf(expectedType, op); + assertEquals(expectedAll, op.isAll(), + "isAll() mismatch for: " + sql); + assertEquals(expectedDistinct, op.isDistinct(), + "isDistinct() mismatch for: " + sql); } - // ── SetOperation object state verification ────────────────── - - @Test - public void testExceptAllSetOperationObject() throws JSQLParserException { - String sql = "SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2"; - Statement stmt = CCJSqlParserUtil.parse(sql); - SetOperationList setOpList = (SetOperationList) stmt; - SetOperation op = setOpList.getOperation(0); - - assertInstanceOf(ExceptOp.class, op); - assertTrue(op.isAll(), "ExceptOp should report isAll() == true"); - assertFalse(op.isDistinct(), "ExceptOp should report isDistinct() == false"); - } - - @Test - public void testMinusAllSetOperationObject() throws JSQLParserException { - String sql = "SELECT a FROM t1 MINUS ALL SELECT a FROM t2"; - Statement stmt = CCJSqlParserUtil.parse(sql); - SetOperationList setOpList = (SetOperationList) stmt; - SetOperation op = setOpList.getOperation(0); - - assertInstanceOf(MinusOp.class, op); - assertTrue(op.isAll(), "MinusOp should report isAll() == true"); + private static Stream provideSetOperationObjectCases() { + return Stream.of( + Arguments.of("SELECT a FROM t1 EXCEPT ALL SELECT a FROM t2", + ExceptOp.class, true, false), + Arguments.of("SELECT a FROM t1 MINUS ALL SELECT a FROM t2", + MinusOp.class, true, false)); } }