diff --git a/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java b/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java index fed7b6f0c..62ab36cae 100644 --- a/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java +++ b/src/main/java/net/sf/jsqlparser/expression/ExpressionVisitorAdapter.java @@ -113,9 +113,17 @@ public T visit(Function function, S context) { 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()); } @@ -284,7 +292,8 @@ public T visit(IsUnknownExpression isUnknownExpression, S context) { @Override public T visit(LikeExpression likeExpression, S context) { - return visitBinaryExpression(likeExpression, context); + return visitExpressions(likeExpression, context, likeExpression.getLeftExpression(), + likeExpression.getRightExpression(), likeExpression.getEscape()); } @Override @@ -319,7 +328,11 @@ public T visit(ContainedBy containedBy, S context) { @Override public 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 @@ -358,12 +371,14 @@ public T visit(ExistsExpression existsExpression, S context) { @Override public T visit(MemberOfExpression memberOfExpression, S context) { - return memberOfExpression.getRightExpression().accept(this, context); + return visitExpressions(memberOfExpression, context, + memberOfExpression.getLeftExpression(), memberOfExpression.getRightExpression()); } @Override public T visit(AnyComparisonExpression anyComparisonExpression, S context) { - return applyExpression(anyComparisonExpression, context); + return visitExpressions(anyComparisonExpression, context, + anyComparisonExpression.getSelect()); } @Override @@ -535,7 +550,10 @@ protected T visitBinaryExpression(BinaryExpression binaryExpression, S conte @Override public T visit(JsonExpression jsonExpr, S context) { - return jsonExpr.getExpression().accept(this, context); + ArrayList subExpressions = new ArrayList<>(); + subExpressions.add(jsonExpr.getExpression()); + subExpressions.addAll(jsonExpr.getIdents()); + return visitExpressions(jsonExpr, context, subExpressions); } @Override @@ -728,13 +746,28 @@ public T visit(XMLSerializeExpr xmlSerializeExpr, S context) { @Override public T visit(TimezoneExpression timezoneExpression, S context) { - return timezoneExpression.getLeftExpression().accept(this, context); + ArrayList subExpressions = new ArrayList<>(); + subExpressions.add(timezoneExpression.getLeftExpression()); + subExpressions.addAll(timezoneExpression.getTimezoneExpressions()); + return visitExpressions(timezoneExpression, context, subExpressions); } @Override public T visit(JsonAggregateFunction jsonAggregateFunction, S context) { - return visitExpressions(jsonAggregateFunction, context, - jsonAggregateFunction.getExpression(), jsonAggregateFunction.getFilterExpression()); + ArrayList 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 @@ -834,7 +867,8 @@ public T visit(TranscodingFunction transcodingFunction, S context) { @Override public T visit(TrimFunction trimFunction, S context) { - return trimFunction.getExpression().accept(this, context); + return visitExpressions(trimFunction, context, trimFunction.getExpression(), + trimFunction.getFromExpression()); } @Override 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 45a2db3dd..50d209635 100644 --- a/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java +++ b/src/main/java/net/sf/jsqlparser/statement/select/SelectVisitorAdapter.java @@ -135,16 +135,19 @@ public T visit(ParenthesedSelect select, S context) { unpivot.accept(pivotVisitor, context); } - expressionVisitor.visitLimit(select.getLimit(), context); + visitLimitOffsetFetch(select, context); + return null; + } + private 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 @@ -190,11 +193,7 @@ public 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); @@ -219,25 +218,14 @@ public 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 // } @@ -289,9 +277,12 @@ public T visit(FromQuery fromQuery, S context) { @Override public 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; } @@ -341,7 +332,8 @@ public T visit(WithItem withItem, S context) { } @Override - public T visit(Values aThis, S context) { + public T visit(Values values, S context) { + expressionVisitor.visitExpressions(values.getExpressions(), context); 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 95490e753..a83e9d928 100644 --- a/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/StatementFeatureVisitorTest.java @@ -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; @@ -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(); + } + } }