From e5cbbad4dad77471e4e297130348e94b8ecadd29 Mon Sep 17 00:00:00 2001 From: tanjiro Kamado Date: Sat, 14 Nov 2026 14:32:31 +0530 Subject: [PATCH] fix: walk WITH items of TableStatement in adapter and validator SelectVisitorAdapter.visit(TableStatement) visited nothing and SelectValidator only reported tableStatement, so a data-modifying CTE in front of TABLE was invisible to StatementFeatures and to Validation: WITH d AS (DELETE FROM t RETURNING a) TABLE d gave mayModifyData() == false and passed FeaturesAllowed.SELECT. The adapter now walks the WITH items, ORDER BY and LIMIT/OFFSET/FETCH of TABLE like its VALUES visit, and SelectValidator validates the WITH list of TABLE and VALUES. --- .../statement/select/SelectVisitorAdapter.java | 3 +++ .../validation/validator/SelectValidator.java | 8 ++++++++ .../statement/StatementFeatureVisitorTest.java | 18 +++++++++++++++++- .../validator/TableStatementValidatorTest.java | 13 +++++++++++++ .../ValuesStatementValidatorTest.java | 9 +++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java b/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java index 8b6e657875..db2ad75949 100644 --- a/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java +++ b/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java @@ -347,6 +347,9 @@ public T visit(LateralSubSelect lateralSubSelect, S context) { @Override public T visit(TableStatement tableStatement, S context) { + visitWithItems(tableStatement.getWithItemsList(), context); + expressionVisitor.visitOrderBy(tableStatement.getOrderByElements(), context); + visitLimitOffsetFetch(tableStatement, context); return null; } } diff --git a/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java b/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java index 9681622879..2223d28ca9 100644 --- a/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java +++ b/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java @@ -503,6 +503,10 @@ public Void visit(LateralSubSelect lateralSubSelect, S context) { @Override public Void visit(TableStatement tableStatement, S context) { + if (isNotEmpty(tableStatement.getWithItemsList())) { + tableStatement.getWithItemsList() + .forEach(withItem -> withItem.accept((SelectVisitor) this, context)); + } getValidator(TableStatementValidator.class).validate(tableStatement); return null; } @@ -525,6 +529,10 @@ public Void visit(ParenthesedFromItem parenthesis, S context) { @Override public Void visit(Values values, S context) { + if (isNotEmpty(values.getWithItemsList())) { + values.getWithItemsList() + .forEach(withItem -> withItem.accept((SelectVisitor) this, context)); + } getValidator(ValuesStatementValidator.class).validate(values); return null; } diff --git a/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java b/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java index d62b829e2e..29fcf46397 100644 --- a/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java @@ -315,6 +315,17 @@ void dataModifyingCteAtTopLevel() throws JSQLParserException { assertThat(f.modifiesData()).isTrue(); } + @ParameterizedTest(name = "{0}") + @ValueSource(strings = {"WITH c AS (DELETE FROM t RETURNING *) TABLE c", + "WITH c AS (UPDATE t SET a = 1 RETURNING *) TABLE u", + "WITH c AS (INSERT INTO t VALUES (1) RETURNING *) TABLE c ORDER BY a LIMIT 1"}) + @DisplayName("data-modifying CTE in front of TABLE: TABLE returns rows, the CTE still mutates") + void dataModifyingCteBeforeTable(String sql) throws JSQLParserException { + StatementFeatures f = analyse(sql); + assertThat(f.returnsResultSet()).isTrue(); + assertThat(f.modifiesData()).isTrue(); + } + @Test @DisplayName("RETURNING inside a nested CTE of an INSERT returns nothing to the client") void nestedReturningIsNotAResultSet() throws JSQLParserException { @@ -666,7 +677,12 @@ private StatementFeatures analyseAllPureExcept(String sql, String impure) "SELECT a[1:length(pg_read_file('x'))] FROM t", "SELECT a FROM t WHERE a[length(pg_read_file('x'))] = 1", "SELECT 1 UNION SELECT 2 LIMIT length(pg_read_file('x'))", - "SELECT 1 UNION SELECT 2 FETCH FIRST length(pg_read_file('x')) ROWS ONLY"}) + "SELECT 1 UNION SELECT 2 FETCH FIRST length(pg_read_file('x')) ROWS ONLY", + "WITH c AS (SELECT pg_read_file('x')) TABLE c", + "TABLE t ORDER BY pg_read_file('x')", + "TABLE t LIMIT length(pg_read_file('x'))", + "TABLE t OFFSET length(pg_read_file('x'))", + "TABLE t FETCH FIRST length(pg_read_file('x')) ROWS ONLY"}) void functionIsChecked(String sql) throws JSQLParserException { StatementFeatures impure = analyseAllPureExcept(sql, "pg_read_file"); assertThat(impure.getUnresolvedReferences()).containsExactly("pg_read_file"); diff --git a/src/test/java/net/sf/jsqlparser/util/validation/validator/TableStatementValidatorTest.java b/src/test/java/net/sf/jsqlparser/util/validation/validator/TableStatementValidatorTest.java index 24d50a1060..1f4cc9134c 100644 --- a/src/test/java/net/sf/jsqlparser/util/validation/validator/TableStatementValidatorTest.java +++ b/src/test/java/net/sf/jsqlparser/util/validation/validator/TableStatementValidatorTest.java @@ -31,4 +31,17 @@ public void testValidationSelectNotAllowed() throws JSQLParserException { validateNotAllowed(sql, 1, 1, FeaturesAllowed.DDL, Feature.select, Feature.tableStatement); } + @Test + public void testValidationWithItemNotAllowed() throws JSQLParserException { + String sql = "WITH d AS (DELETE FROM t RETURNING a) TABLE d"; + validateNotAllowed(sql, 1, 1, FeaturesAllowed.SELECT, Feature.withItem, Feature.delete, + Feature.deleteReturningExpressionList); + } + + @Test + public void testValidationWithItemAllowed() throws JSQLParserException { + String sql = "WITH d AS (DELETE FROM t RETURNING a) TABLE d"; + validateNoErrors(sql, 1, PostgresqlVersion.V14); + } + } diff --git a/src/test/java/net/sf/jsqlparser/util/validation/validator/ValuesStatementValidatorTest.java b/src/test/java/net/sf/jsqlparser/util/validation/validator/ValuesStatementValidatorTest.java index 66f7209acf..bc511d628b 100644 --- a/src/test/java/net/sf/jsqlparser/util/validation/validator/ValuesStatementValidatorTest.java +++ b/src/test/java/net/sf/jsqlparser/util/validation/validator/ValuesStatementValidatorTest.java @@ -9,8 +9,10 @@ */ package net.sf.jsqlparser.util.validation.validator; +import net.sf.jsqlparser.parser.feature.Feature; import net.sf.jsqlparser.util.validation.ValidationTestAsserts; import net.sf.jsqlparser.util.validation.feature.DatabaseType; +import net.sf.jsqlparser.util.validation.feature.FeaturesAllowed; import org.junit.jupiter.api.Test; public class ValuesStatementValidatorTest extends ValidationTestAsserts { @@ -22,4 +24,11 @@ public void testValidateValuesStatement() { 1, DatabaseType.SQLSERVER); } + @Test + public void testValidateWithItemsOfValues() { + validateNotAllowed("WITH d AS (UPDATE t SET a = 1 RETURNING a) VALUES (1)", 1, 1, + FeaturesAllowed.SELECT.copy().add(Feature.values), Feature.withItem, + Feature.update, Feature.updateReturning); + } + }