diff --git a/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java b/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java index 08715557a..2d979f5b8 100644 --- a/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java +++ b/src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java @@ -1018,11 +1018,18 @@ static final class FeatureFromItemVisitor extends FromItemVisitorAdapter { this.analysis = analysis; } - /** The inherited implementation is empty, so table functions escape the purity check. */ + /** + * The inherited implementation is empty, so table functions escape the purity check. + * {@link TableFunction#getFunctions()} also covers {@code ROWS FROM (..)}, where + * {@link TableFunction#getFunction()} is null. + */ @Override public Void visit(TableFunction tableFunction, S context) { - if (tableFunction.getFunction() != null) { - tableFunction.getFunction().accept(analysis.expressions, context); + List functions = tableFunction.getFunctions(); + if (functions != null) { + for (Function function : functions) { + function.accept(analysis.expressions, context); + } } return super.visit(tableFunction, context); } 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 639162410..96d5e3324 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 @@ -464,6 +464,7 @@ public Void visit(TableStatement tableStatement, S context) { @Override public Void visit(TableFunction tableFunction, S context) { validateFeature(Feature.tableFunction); + validateOptionalExpressions(tableFunction.getFunctions()); validateOptional(tableFunction.getPivot(), p -> p.accept(this, context)); validateOptional(tableFunction.getUnPivot(), up -> up.accept(this, context)); diff --git a/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java b/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java index 95490e753..6961934ea 100644 --- a/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java @@ -327,6 +327,88 @@ void allowListCollapsesTheUncertainty() throws JSQLParserException { } } + /** + * {@code ROWS FROM (..)} keeps its functions in {@code getRowsFromFunctions()} and leaves + * {@code getFunction()} null, so a check against the single function alone lets every one of + * them through as harmless. + */ + @Nested + @DisplayName("table functions in the FROM clause") + class TableFunctions { + + private StatementFeatures analyseNothingPure(String sql) throws JSQLParserException { + Predicate nothingIsPure = name -> false; + return StatementFeatureVisitor.analyse(CCJSqlParserUtil.parse(sql), nothingIsPure); + } + + @Test + void singleTableFunctionIsChecked() throws JSQLParserException { + String sql = "SELECT * FROM pg_ls_dir('.')"; + + StatementFeatures impure = analyseNothingPure(sql); + assertThat(impure.returnsResultSet()).isTrue(); + assertThat(impure.modifiesData()).isFalse(); + assertThat(impure.getUncertain()).contains(StmtFeature.MODIFIES_DATA, + StmtFeature.MODIFIES_SCHEMA); + assertThat(impure.getUnresolvedReferences()).containsExactly("pg_ls_dir"); + + StatementFeatures pure = analyse(sql, "pg_ls_dir"); + assertThat(pure.mayModifyData()).isFalse(); + assertThat(pure.getUnresolvedReferences()).isEmpty(); + } + + @Test + void rowsFromWithOneFunctionIsChecked() throws JSQLParserException { + String sql = "SELECT * FROM ROWS FROM (pg_ls_dir('.'))"; + + StatementFeatures impure = analyseNothingPure(sql); + assertThat(impure.returnsResultSet()).isTrue(); + assertThat(impure.modifiesData()).isFalse(); + assertThat(impure.getUncertain()).contains(StmtFeature.MODIFIES_DATA, + StmtFeature.MODIFIES_SCHEMA); + assertThat(impure.getUnresolvedReferences()).containsExactly("pg_ls_dir"); + + StatementFeatures pure = analyse(sql, "pg_ls_dir"); + assertThat(pure.mayModifyData()).isFalse(); + assertThat(pure.getUnresolvedReferences()).isEmpty(); + } + + @Test + void rowsFromChecksEveryFunction() throws JSQLParserException { + String sql = "SELECT * FROM ROWS FROM (generate_series(1, 2), pg_ls_dir('.'))"; + + StatementFeatures impure = analyseNothingPure(sql); + assertThat(impure.mayModifyData()).isTrue(); + assertThat(impure.getUnresolvedReferences()) + .containsExactlyInAnyOrder("generate_series", "pg_ls_dir"); + + StatementFeatures onlyFirstPure = analyse(sql, "generate_series"); + assertThat(onlyFirstPure.mayModifyData()) + .as("a pure first function must not hide the second") + .isTrue(); + assertThat(onlyFirstPure.getUnresolvedReferences()).containsExactly("pg_ls_dir"); + + StatementFeatures pure = analyse(sql, "generate_series", "pg_ls_dir"); + assertThat(pure.mayModifyData()).isFalse(); + assertThat(pure.getUnresolvedReferences()).isEmpty(); + } + + @Test + void rowsFromWithOrdinalityIsChecked() throws JSQLParserException { + String sql = "SELECT * FROM ROWS FROM (generate_series(1, 2), pg_ls_dir('.')) " + + "WITH ORDINALITY AS r(n, name, ord)"; + + StatementFeatures impure = analyseNothingPure(sql); + assertThat(impure.mayModifyData()).isTrue(); + assertThat(impure.getUnresolvedReferences()) + .containsExactlyInAnyOrder("generate_series", "pg_ls_dir"); + + StatementFeatures pure = analyse(sql, "generate_series", "pg_ls_dir"); + assertThat(pure.mayModifyData()).isFalse(); + assertThat(pure.getUnresolvedReferences()).isEmpty(); + } + } + @Nested @DisplayName("DDL and opaque statements") class PendingStatementOverrides { diff --git a/src/test/java/net/sf/jsqlparser/util/validation/validator/SelectValidatorTest.java b/src/test/java/net/sf/jsqlparser/util/validation/validator/SelectValidatorTest.java index 55fc6cf0a..6191402dc 100644 --- a/src/test/java/net/sf/jsqlparser/util/validation/validator/SelectValidatorTest.java +++ b/src/test/java/net/sf/jsqlparser/util/validation/validator/SelectValidatorTest.java @@ -17,9 +17,19 @@ import net.sf.jsqlparser.util.validation.feature.FeaturesAllowed; import net.sf.jsqlparser.util.validation.feature.MariaDbVersion; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class SelectValidatorTest extends ValidationTestAsserts { + /** + * {@link FeaturesAllowed#SELECT} does not allow {@link Feature#jdbcParameter}, so a {@code ?} + * argument must be reported wherever the function is written. + */ + private static final FeaturesAllowed SELECT_WITH_TABLE_FUNCTIONS = + new FeaturesAllowed("SELECT + tableFunction", Feature.tableFunction) + .add(FeaturesAllowed.SELECT); + @Test public void testValidationSelectNotAllowed() throws JSQLParserException { String sql = "SELECT 1"; @@ -206,6 +216,20 @@ public void testValidateTableFunction() { } } + @Test + public void testValidateFunctionArgumentInWhere() { + validateNotAllowed("SELECT * FROM t WHERE a = f(?)", 1, 1, SELECT_WITH_TABLE_FUNCTIONS, + Feature.jdbcParameter); + } + + @ParameterizedTest + @ValueSource(strings = {"SELECT * FROM f(?)", + "SELECT * FROM ROWS FROM (f(1), g(?))", + "SELECT * FROM ROWS FROM (f(1), g(?)) WITH ORDINALITY"}) + public void testValidateTableFunctionArguments(String sql) { + validateNotAllowed(sql, 1, 1, SELECT_WITH_TABLE_FUNCTIONS, Feature.jdbcParameter); + } + @Test public void testValidateLateral() throws JSQLParserException { validateNoErrors(