Skip to content

fix: traverse missing expression and query children in visitor adapters #2761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
manticore-projects merged 1 commit into from
Oct 9, 2026
+148 −32
Merged

fix: traverse missing expression and query children in visitor adapters #2761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,17 @@
if (function.getParameters() != null) {
subExpressions.addAll(function.getParameters());
}
if (function.getNamedParameters() != null) {
subExpressions.addAll(function.getNamedParameters());
}
if (function.getChainedParameters() != null) {
subExpressions.addAll(function.getChainedParameters());
}
// an attribute name is not a column reference, only an attribute expression is visited
if (function.getAttribute() instanceof Expression
&& function.getAttribute() != function.getAttributeColumn()) {
subExpressions.add((Expression) function.getAttribute());
}
if (function.getKeep() != null) {
subExpressions.add(function.getKeep());
}
Expand Down Expand Up @@ -284,7 +292,8 @@

@Override
public <S> T visit(LikeExpression likeExpression, S context) {
return visitBinaryExpression(likeExpression, context);
return visitExpressions(likeExpression, context, likeExpression.getLeftExpression(),
likeExpression.getRightExpression(), likeExpression.getEscape());
}

@Override
Expand Down Expand Up @@ -319,7 +328,11 @@

@Override
public <S> T visit(Column column, S context) {
return applyExpression(column, context);
T result = applyExpression(column, context);
if (column.getArrayConstructor() != null) {
column.getArrayConstructor().accept(this, context);
}
return result;
}

@Override
Expand Down Expand Up @@ -358,12 +371,14 @@

@Override
public <S> T visit(MemberOfExpression memberOfExpression, S context) {
return memberOfExpression.getRightExpression().accept(this, context);
return visitExpressions(memberOfExpression, context,
memberOfExpression.getLeftExpression(), memberOfExpression.getRightExpression());
}

@Override
public <S> T visit(AnyComparisonExpression anyComparisonExpression, S context) {
return applyExpression(anyComparisonExpression, context);
return visitExpressions(anyComparisonExpression, context,
anyComparisonExpression.getSelect());
}

@Override
Expand Down Expand Up @@ -535,7 +550,10 @@

@Override
public <S> T visit(JsonExpression jsonExpr, S context) {
return jsonExpr.getExpression().accept(this, context);
ArrayList<Expression> subExpressions = new ArrayList<>();
subExpressions.add(jsonExpr.getExpression());
subExpressions.addAll(jsonExpr.getIdents());

Check warning on line 555 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (windows-latest) 

[deprecation] getIdents() in JsonExpression has been deprecated

Check warning on line 555 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (ubuntu-latest) 

[deprecation] getIdents() in JsonExpression has been deprecated

Check warning on line 555 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (macos-latest) 

[deprecation] getIdents() in JsonExpression has been deprecated
return visitExpressions(jsonExpr, context, subExpressions);
}

@Override
Expand Down Expand Up @@ -728,13 +746,28 @@

@Override
public <S> T visit(TimezoneExpression timezoneExpression, S context) {
return timezoneExpression.getLeftExpression().accept(this, context);
ArrayList<Expression> subExpressions = new ArrayList<>();
subExpressions.add(timezoneExpression.getLeftExpression());
subExpressions.addAll(timezoneExpression.getTimezoneExpressions());
return visitExpressions(timezoneExpression, context, subExpressions);
}

@Override
public <S> T visit(JsonAggregateFunction jsonAggregateFunction, S context) {
return visitExpressions(jsonAggregateFunction, context,
jsonAggregateFunction.getExpression(), jsonAggregateFunction.getFilterExpression());
ArrayList<Expression> subExpressions = new ArrayList<>();
subExpressions.add(jsonAggregateFunction.getExpression());
if (jsonAggregateFunction.getKey() instanceof Expression) {
subExpressions.add((Expression) jsonAggregateFunction.getKey());
}
if (jsonAggregateFunction.getValue() instanceof Expression) {
subExpressions.add((Expression) jsonAggregateFunction.getValue());
}
addOrderByExpressions(subExpressions,
jsonAggregateFunction.getExpressionOrderByElements());
subExpressions.add(jsonAggregateFunction.getFilterExpression());
subExpressions.addAll(jsonAggregateFunction.getPartitionExpressionList());
addOrderByExpressions(subExpressions, jsonAggregateFunction.getOrderByElements());
return visitExpressions(jsonAggregateFunction, context, subExpressions);
}

@Override
Expand Down Expand Up @@ -781,7 +814,7 @@

@Override
public <S> T visit(ConnectByRootOperator connectByRootOperator, S context) {
return connectByRootOperator.getColumn().accept(this, context);

Check warning on line 817 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (windows-latest) 

[deprecation] getColumn() in ConnectByRootOperator has been deprecated

Check warning on line 817 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (ubuntu-latest) 

[deprecation] getColumn() in ConnectByRootOperator has been deprecated

Check warning on line 817 in src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java

View workflow job for this annotation

GitHub Actions / Maven Verify (macos-latest) 

[deprecation] getColumn() in ConnectByRootOperator has been deprecated
}

@Override
Expand Down Expand Up @@ -834,7 +867,8 @@

@Override
public <S> T visit(TrimFunction trimFunction, S context) {
return trimFunction.getExpression().accept(this, context);
return visitExpressions(trimFunction, context, trimFunction.getExpression(),
trimFunction.getFromExpression());
}

@Override
Expand Down
38 changes: 15 additions & 23 deletions src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,19 @@ public <S> T visit(ParenthesedSelect select, S context) {
unpivot.accept(pivotVisitor, context);
}

expressionVisitor.visitLimit(select.getLimit(), context);
visitLimitOffsetFetch(select, context);
return null;
}

private <S> void visitLimitOffsetFetch(Select select, S context) {
expressionVisitor.visitLimit(select.getLimitBy(), context);
expressionVisitor.visitLimit(select.getLimit(), context);
if (select.getOffset() != null) {
expressionVisitor.visitExpression(select.getOffset().getOffset(), null);
expressionVisitor.visitExpression(select.getOffset().getOffset(), context);
}
if (select.getFetch() != null) {
expressionVisitor.visitExpression(select.getFetch().getExpression(), null);
expressionVisitor.visitExpression(select.getFetch().getExpression(), context);
}

return null;
}

@Override
Expand Down Expand Up @@ -190,11 +193,7 @@ public <S> T visit(PlainSelect plainSelect, S context) {

expressionVisitor.visitExpression(plainSelect.getPreWhere(), context);
expressionVisitor.visitExpression(plainSelect.getWhere(), context);

// if (plainSelect.getOracleHierarchical() != null) {
// //@todo: implement
// }
//
expressionVisitor.visitExpression(plainSelect.getOracleHierarchical(), context);

expressionVisitor.visitPreferringClause(plainSelect.getPreferringClause(), context);
expressionVisitor.visit(plainSelect.getGroupBy(), context);
Expand All @@ -219,25 +218,14 @@ public <S> T visit(PlainSelect plainSelect, S context) {
}

expressionVisitor.visitOrderBy(plainSelect.getOrderByElements(), context);
visitLimitOffsetFetch(plainSelect, context);

// if (plainSelect.getLimitBy() != null) {
// //@todo: implement
// }
// if (plainSelect.getLimit() != null) {
// //@todo: implement
// }
if (plainSelect.getOffset() != null) {
expressionVisitor.visitExpression(plainSelect.getOffset().getOffset(), context);
}
if (plainSelect.getMySqlProcedureAnalyse() != null) {
expressionVisitor.visitExpression(
plainSelect.getMySqlProcedureAnalyse().getMaxElements(), context);
expressionVisitor.visitExpression(
plainSelect.getMySqlProcedureAnalyse().getMaxMemory(), context);
}
if (plainSelect.getFetch() != null) {
expressionVisitor.visitExpression(plainSelect.getFetch().getExpression(), context);
}
// if (plainSelect.getForMode() != null) {
// //@todo: implement
// }
Expand Down Expand Up @@ -289,9 +277,12 @@ public <S> T visit(FromQuery fromQuery, S context) {

@Override
public <S> T visit(SetOperationList setOpList, S context) {
visitWithItems(setOpList.getWithItemsList(), context);
for (Select select : setOpList.getSelects()) {
select.accept(this, context);
}
expressionVisitor.visitOrderBy(setOpList.getOrderByElements(), context);
visitLimitOffsetFetch(setOpList, context);
return null;
}

Expand Down Expand Up @@ -341,7 +332,8 @@ public <S> T visit(WithItem<?> withItem, S context) {
}

@Override
public <S> T visit(Values aThis, S context) {
public <S> T visit(Values values, S context) {
expressionVisitor.visitExpressions(values.getExpressions(), context);
return null;
}

Expand Down
90 changes: 90 additions & 0 deletions src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.util.Collections;
import java.util.EnumSet;
Expand Down Expand Up @@ -498,4 +500,92 @@ void explainIsUncertain() throws JSQLParserException {
assertThat(f.returnsResultSet()).isTrue();
assertThat(f.mayModifyData()).isTrue();
}

/**
* Every expression position has to be traversed, otherwise a function call placed there escapes
* the purity check and a subquery placed there escapes the lock and write checks.
*/
@Nested
@DisplayName("every expression position is traversed")
class ExpressionPositions {

private StatementFeatures analyseAllPureExcept(String sql, String impure)
throws JSQLParserException {
return StatementFeatureVisitor.analyse(CCJSqlParserUtil.parse(sql),
name -> !name.equals(impure));
}

@ParameterizedTest(name = "{0}")
@ValueSource(strings = {
"SELECT substring(pg_read_file('x') FROM 2 FOR 3)",
"SELECT trim(LEADING 'x' FROM pg_read_file('x'))",
"SELECT position(pg_read_file('x') IN a) FROM t",
"SELECT overlay(a PLACING pg_read_file('x') FROM 2) FROM t",
"SELECT a FROM t WHERE a = ANY (SELECT pg_read_file('x'))",
"SELECT a FROM t WHERE a > ALL (SELECT pg_read_file('x'))",
"SELECT a FROM t WHERE a = SOME (SELECT pg_read_file('x'))",
"VALUES (pg_read_file('x'))",
"SELECT a FROM t WHERE a IN (VALUES (pg_read_file('x')))",
"SELECT a FROM t LIMIT length(pg_read_file('x'))",
"SELECT a FROM t WHERE a LIKE 'a' ESCAPE pg_read_file('x')",
"SELECT d AT TIME ZONE pg_read_file('x') FROM t",
"SELECT j -> pg_read_file('x') FROM t",
"SELECT json_objectagg(a : pg_read_file('x')) FROM t",
"SELECT a FROM t WHERE pg_read_file('x') MEMBER OF (j)",
"SELECT a FROM t START WITH a = pg_read_file('x') CONNECT BY PRIOR a = b",
"SELECT a FROM t LIMIT 1 BY pg_read_file('x')",
"WITH c AS (SELECT pg_read_file('x')) SELECT a FROM c UNION SELECT b FROM u",
"SELECT a FROM t UNION SELECT b FROM u ORDER BY pg_read_file('x')",
"SELECT a FROM t UNION SELECT b FROM u LIMIT length(pg_read_file('x'))",
"SELECT j ->> pg_read_file('x') FROM t",
"SELECT j #> ARRAY[pg_read_file('x')] FROM t",
"SELECT j #>> ARRAY[pg_read_file('x')] FROM t",
"SELECT a[length(pg_read_file('x'))] FROM t",
"SELECT t.a[length(pg_read_file('x'))] FROM t",
"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"})
void functionIsChecked(String sql) throws JSQLParserException {
StatementFeatures impure = analyseAllPureExcept(sql, "pg_read_file");
assertThat(impure.getUnresolvedReferences()).containsExactly("pg_read_file");
assertThat(impure.getUncertain()).contains(StmtFeature.MODIFIES_DATA,
StmtFeature.MODIFIES_SCHEMA);

StatementFeatures pure = analyseAllPureExcept(sql, "");
assertThat(pure.mayModifyData()).isFalse();
assertThat(pure.getUnresolvedReferences()).isEmpty();
}

@ParameterizedTest(name = "{0}")
@ValueSource(strings = {
"SELECT a FROM t WHERE a = ANY (SELECT b FROM u FOR UPDATE)",
"SELECT a FROM t WHERE a > ALL (SELECT b FROM u FOR UPDATE)",
"VALUES ((SELECT b FROM u FOR UPDATE))",
"SELECT a FROM t WHERE a IN (VALUES ((SELECT b FROM u FOR UPDATE)))",
"SELECT a FROM t LIMIT (SELECT b FROM u FOR UPDATE)"})
void rowLockIsSeen(String sql) throws JSQLParserException {
StatementFeatures f = analyse(sql);
assertThat(f.is(StmtFeature.MODIFIES_TRANSACTION)).isTrue();
assertThat(f.is(StmtFeature.READS_DATA)).isTrue();
}

@ParameterizedTest(name = "{0}")
@ValueSource(strings = {
"SELECT a FROM t WHERE a = ANY (WITH c AS (DELETE FROM u RETURNING b) SELECT b FROM c)",
"VALUES ((WITH c AS (DELETE FROM u RETURNING b) SELECT b FROM c))"})
void nestedWriteIsSeen(String sql) throws JSQLParserException {
StatementFeatures f = analyse(sql);
assertThat(f.modifiesData()).isTrue();
assertThat(f.returnsResultSet()).isTrue();
}

@Test
void topLevelValuesReturnsRowsWithoutReading() throws JSQLParserException {
StatementFeatures f = analyse("VALUES (1, 2)");
assertThat(f.returnsResultSet()).isTrue();
assertThat(f.is(StmtFeature.READS_DATA)).isFalse();
assertThat(f.mayModifyData()).isFalse();
}
}
}
Loading