Skip to content

Commit 13a9dda

Browse files
fix: inspect every table function in feature analysis and validation (#2760)
* fix(feature): analyse ROWS FROM functions for purity StatementFeatureVisitor only checked TableFunction#getFunction(), which is null for ROWS FROM (...), so every function listed there escaped the purity check. Iterate TableFunction#getFunctions() instead. * fix(validation): validate table function arguments SelectValidator only checked that table functions are allowed and never validated the functions themselves, so features, names and nested queries in their arguments went unchecked. This covers both f(..) and every function of ROWS FROM (..).
1 parent f7bf370 commit 13a9dda

4 files changed

Lines changed: 117 additions & 3 deletions

File tree

‎src/main/java/net/sf/jsqlparser/statement/StatementFeatureVisitor.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,11 +1022,18 @@ static final class FeatureFromItemVisitor extends FromItemVisitorAdapter<Void> {
10221022
this.analysis = analysis;
10231023
}
10241024

1025-
/** The inherited implementation is empty, so table functions escape the purity check. */
1025+
/**
1026+
* The inherited implementation is empty, so table functions escape the purity check.
1027+
* {@link TableFunction#getFunctions()} also covers {@code ROWS FROM (..)}, where
1028+
* {@link TableFunction#getFunction()} is null.
1029+
*/
10261030
@Override
10271031
public <S> Void visit(TableFunction tableFunction, S context) {
1028-
if (tableFunction.getFunction() != null) {
1029-
tableFunction.getFunction().accept(analysis.expressions, context);
1032+
List<Function> functions = tableFunction.getFunctions();
1033+
if (functions != null) {
1034+
for (Function function : functions) {
1035+
function.accept(analysis.expressions, context);
1036+
}
10301037
}
10311038
return super.visit(tableFunction, context);
10321039
}

‎src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ public <S> Void visit(TableStatement tableStatement, S context) {
464464
@Override
465465
public <S> Void visit(TableFunction tableFunction, S context) {
466466
validateFeature(Feature.tableFunction);
467+
validateOptionalExpressions(tableFunction.getFunctions());
467468

468469
validateOptional(tableFunction.getPivot(), p -> p.accept(this, context));
469470
validateOptional(tableFunction.getUnPivot(), up -> up.accept(this, context));

‎src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,88 @@ void allowListCollapsesTheUncertainty() throws JSQLParserException {
329329
}
330330
}
331331

332+
/**
333+
* {@code ROWS FROM (..)} keeps its functions in {@code getRowsFromFunctions()} and leaves
334+
* {@code getFunction()} null, so a check against the single function alone lets every one of
335+
* them through as harmless.
336+
*/
337+
@Nested
338+
@DisplayName("table functions in the FROM clause")
339+
class TableFunctions {
340+
341+
private StatementFeatures analyseNothingPure(String sql) throws JSQLParserException {
342+
Predicate<String> nothingIsPure = name -> false;
343+
return StatementFeatureVisitor.analyse(CCJSqlParserUtil.parse(sql), nothingIsPure);
344+
}
345+
346+
@Test
347+
void singleTableFunctionIsChecked() throws JSQLParserException {
348+
String sql = "SELECT * FROM pg_ls_dir('.')";
349+
350+
StatementFeatures impure = analyseNothingPure(sql);
351+
assertThat(impure.returnsResultSet()).isTrue();
352+
assertThat(impure.modifiesData()).isFalse();
353+
assertThat(impure.getUncertain()).contains(StmtFeature.MODIFIES_DATA,
354+
StmtFeature.MODIFIES_SCHEMA);
355+
assertThat(impure.getUnresolvedReferences()).containsExactly("pg_ls_dir");
356+
357+
StatementFeatures pure = analyse(sql, "pg_ls_dir");
358+
assertThat(pure.mayModifyData()).isFalse();
359+
assertThat(pure.getUnresolvedReferences()).isEmpty();
360+
}
361+
362+
@Test
363+
void rowsFromWithOneFunctionIsChecked() throws JSQLParserException {
364+
String sql = "SELECT * FROM ROWS FROM (pg_ls_dir('.'))";
365+
366+
StatementFeatures impure = analyseNothingPure(sql);
367+
assertThat(impure.returnsResultSet()).isTrue();
368+
assertThat(impure.modifiesData()).isFalse();
369+
assertThat(impure.getUncertain()).contains(StmtFeature.MODIFIES_DATA,
370+
StmtFeature.MODIFIES_SCHEMA);
371+
assertThat(impure.getUnresolvedReferences()).containsExactly("pg_ls_dir");
372+
373+
StatementFeatures pure = analyse(sql, "pg_ls_dir");
374+
assertThat(pure.mayModifyData()).isFalse();
375+
assertThat(pure.getUnresolvedReferences()).isEmpty();
376+
}
377+
378+
@Test
379+
void rowsFromChecksEveryFunction() throws JSQLParserException {
380+
String sql = "SELECT * FROM ROWS FROM (generate_series(1, 2), pg_ls_dir('.'))";
381+
382+
StatementFeatures impure = analyseNothingPure(sql);
383+
assertThat(impure.mayModifyData()).isTrue();
384+
assertThat(impure.getUnresolvedReferences())
385+
.containsExactlyInAnyOrder("generate_series", "pg_ls_dir");
386+
387+
StatementFeatures onlyFirstPure = analyse(sql, "generate_series");
388+
assertThat(onlyFirstPure.mayModifyData())
389+
.as("a pure first function must not hide the second")
390+
.isTrue();
391+
assertThat(onlyFirstPure.getUnresolvedReferences()).containsExactly("pg_ls_dir");
392+
393+
StatementFeatures pure = analyse(sql, "generate_series", "pg_ls_dir");
394+
assertThat(pure.mayModifyData()).isFalse();
395+
assertThat(pure.getUnresolvedReferences()).isEmpty();
396+
}
397+
398+
@Test
399+
void rowsFromWithOrdinalityIsChecked() throws JSQLParserException {
400+
String sql = "SELECT * FROM ROWS FROM (generate_series(1, 2), pg_ls_dir('.')) "
401+
+ "WITH ORDINALITY AS r(n, name, ord)";
402+
403+
StatementFeatures impure = analyseNothingPure(sql);
404+
assertThat(impure.mayModifyData()).isTrue();
405+
assertThat(impure.getUnresolvedReferences())
406+
.containsExactlyInAnyOrder("generate_series", "pg_ls_dir");
407+
408+
StatementFeatures pure = analyse(sql, "generate_series", "pg_ls_dir");
409+
assertThat(pure.mayModifyData()).isFalse();
410+
assertThat(pure.getUnresolvedReferences()).isEmpty();
411+
}
412+
}
413+
332414
@Nested
333415
@DisplayName("DDL and opaque statements")
334416
class PendingStatementOverrides {

‎src/test/java/net/sf/jsqlparser/util/validation/validator/SelectValidatorTest.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,19 @@
1717
import net.sf.jsqlparser.util.validation.feature.FeaturesAllowed;
1818
import net.sf.jsqlparser.util.validation.feature.MariaDbVersion;
1919
import org.junit.jupiter.api.Test;
20+
import org.junit.jupiter.params.ParameterizedTest;
21+
import org.junit.jupiter.params.provider.ValueSource;
2022

2123
public class SelectValidatorTest extends ValidationTestAsserts {
2224

25+
/**
26+
* {@link FeaturesAllowed#SELECT} does not allow {@link Feature#jdbcParameter}, so a {@code ?}
27+
* argument must be reported wherever the function is written.
28+
*/
29+
private static final FeaturesAllowed SELECT_WITH_TABLE_FUNCTIONS =
30+
new FeaturesAllowed("SELECT + tableFunction", Feature.tableFunction)
31+
.add(FeaturesAllowed.SELECT);
32+
2333
@Test
2434
public void testValidationSelectNotAllowed() throws JSQLParserException {
2535
String sql = "SELECT 1";
@@ -206,6 +216,20 @@ public void testValidateTableFunction() {
206216
}
207217
}
208218

219+
@Test
220+
public void testValidateFunctionArgumentInWhere() {
221+
validateNotAllowed("SELECT * FROM t WHERE a = f(?)", 1, 1, SELECT_WITH_TABLE_FUNCTIONS,
222+
Feature.jdbcParameter);
223+
}
224+
225+
@ParameterizedTest
226+
@ValueSource(strings = {"SELECT * FROM f(?)",
227+
"SELECT * FROM ROWS FROM (f(1), g(?))",
228+
"SELECT * FROM ROWS FROM (f(1), g(?)) WITH ORDINALITY"})
229+
public void testValidateTableFunctionArguments(String sql) {
230+
validateNotAllowed(sql, 1, 1, SELECT_WITH_TABLE_FUNCTIONS, Feature.jdbcParameter);
231+
}
232+
209233
@Test
210234
public void testValidateLateral() throws JSQLParserException {
211235
validateNoErrors(

0 commit comments

Comments
 (0)