Skip to content

fix: traverse skipped expression children in TablesNamesFinder #2826

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

Open
AndreasLanz01 wants to merge 1 commit into
base: master
Choose a base branch
from
+182 −27
Open

fix: traverse skipped expression children in TablesNamesFinder #2826

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
84 changes: 59 additions & 25 deletions src/main/java/net/sf/jsqlparser/util/TablesNamesFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@
import net.sf.jsqlparser.statement.select.Join;
import net.sf.jsqlparser.statement.select.LateralSubSelect;
import net.sf.jsqlparser.statement.select.LateralView;
import net.sf.jsqlparser.statement.select.Limit;
import net.sf.jsqlparser.statement.select.MatchRecognize;
import net.sf.jsqlparser.statement.select.OrderByElement;
import net.sf.jsqlparser.statement.select.ParenthesedFromItem;
Expand Down Expand Up @@ -701,9 +702,28 @@ public <S> Void visit(Function function, S context) {
if (exprList != null) {
visit(exprList, context);
}
// an attribute column names a field of the result, only an attribute function is visited
if (function.getAttribute() != function.getAttributeColumn()) {
visitExpression((Expression) function.getAttribute(), context);
}
visitOrderBy(function.getOrderByElements(), context);
visitAggregateModifiers(function.getKeep(), function.getHavingClause(),
function.getKeywordArguments(), function.getLimit(), context);
return null;
}

private <S> void visitAggregateModifiers(KeepExpression keep, Function.HavingClause having,
List<Function.KeywordArgument> keywordArguments, Limit limit, S context) {
visitExpression(keep, context);
visitExpression(having, context);
if (keywordArguments != null) {
for (Function.KeywordArgument keywordArgument : keywordArguments) {
visitExpression(keywordArgument.getExpression(), context);
}
}
visitLimit(limit, context);
}

@Override
public <S> Void visit(GreaterThan greaterThan, S context) {
visitBinaryExpression(greaterThan);
Expand Down Expand Up @@ -739,7 +759,8 @@ public <S> Void visit(ExcludesExpression excludesExpression, S context) {

@Override
public <S> Void visit(FullTextSearch fullTextSearch, S context) {

visitExpressions(fullTextSearch.getMatchColumns(), context);
visitExpression(fullTextSearch.getAgainstValue(), context);
return null;
}

Expand Down Expand Up @@ -769,7 +790,7 @@ public <S> Void visit(IsJsonExpression expression, S context) {

@Override
public <S> Void visit(IsUnknownExpression isUnknownExpression, S context) {

isUnknownExpression.getLeftExpression().accept(this, context);
return null;
}

Expand All @@ -782,6 +803,7 @@ public <S> Void visit(JdbcParameter jdbcParameter, S context) {
@Override
public <S> Void visit(LikeExpression likeExpression, S context) {
visitBinaryExpression(likeExpression);
visitExpression(likeExpression.getEscape(), context);
return null;
}

Expand Down Expand Up @@ -1029,9 +1051,8 @@ public <S> Void visit(AnalyticExpression analytic, S context) {
if (analytic.getOffset() != null) {
analytic.getOffset().accept(this, context);
}
if (analytic.getKeep() != null) {
analytic.getKeep().accept(this, context);
}
visitAggregateModifiers(analytic.getKeep(), analytic.getHavingClause(),
analytic.getKeywordArguments(), analytic.getLimit(), context);
if (analytic.getFilterExpression() != null) {
analytic.getFilterExpression().accept(this, context);
}
Expand All @@ -1048,29 +1069,24 @@ public <S> Void visit(AnalyticExpression analytic, S context) {
element.getExpression().accept(this, context);
}
}
visitWindowElement(analytic.getWindowElement(), context);
return null;
}

if (analytic.getWindowElement() != null) {
if (analytic.getWindowElement().getRange() != null) {
if (analytic.getWindowElement().getRange().getStart() != null
&& analytic.getWindowElement().getRange().getStart()
.getExpression() != null) {
analytic.getWindowElement().getRange().getStart().getExpression().accept(this,
context);
private <S> void visitWindowElement(WindowElement windowElement, S context) {
if (windowElement != null) {
if (windowElement.getRange() != null) {
if (windowElement.getRange().getStart() != null) {
visitExpression(windowElement.getRange().getStart().getExpression(), context);
}
if (analytic.getWindowElement().getRange().getEnd() != null
&& analytic.getWindowElement().getRange().getEnd()
.getExpression() != null) {
analytic.getWindowElement().getRange().getEnd().getExpression().accept(this,
context);
if (windowElement.getRange().getEnd() != null) {
visitExpression(windowElement.getRange().getEnd().getExpression(), context);
}
}
if (analytic.getWindowElement().getOffset() != null
&& analytic.getWindowElement().getOffset().getExpression() != null) {
analytic.getWindowElement().getOffset().getExpression().accept(this, context);
if (windowElement.getOffset() != null) {
visitExpression(windowElement.getOffset().getExpression(), context);
}

}
return null;
}

@Override
Expand Down Expand Up @@ -1365,6 +1381,9 @@ public <S> Void visit(JsonExpression jsonExpr, S context) {
if (jsonExpr.getExpression() != null) {
jsonExpr.getExpression().accept(this, context);
}
for (Expression ident : jsonExpr.getIdents()) {
visitExpression(ident, context);
}
return null;
}

Expand Down Expand Up @@ -1436,13 +1455,14 @@ public <S> Void visit(NumericBind numericBind, S context) {

@Override
public <S> Void visit(KeepExpression keepExpression, S context) {

visitOrderBy(keepExpression.getOrderByElements(), context);
return null;
}

@Override
public <S> Void visit(MySQLGroupConcat groupConcat, S context) {

visitExpressions(groupConcat.getExpressionList(), context);
visitOrderBy(groupConcat.getOrderByElements(), context);
return null;
}

Expand Down Expand Up @@ -2001,7 +2021,8 @@ public <S> Void visit(OracleHint hint, S context) {
public <S> Void visit(TableFunction tableFunction, S context) {
if (tableFunction.getFunctions() != null) {
for (var function : tableFunction.getFunctions()) {
visit(function, null);
// dispatch dynamically, XMLTABLE and JSON_TABLE are Function subclasses
function.accept(this, context);
}
}
return null;
Expand Down Expand Up @@ -2450,6 +2471,9 @@ public void visit(CreateSynonym createSynonym) {
@Override
public <S> Void visit(TimezoneExpression timezoneExpression, S context) {
timezoneExpression.getLeftExpression().accept(this, context);
for (Expression timezone : timezoneExpression.getTimezoneExpressions()) {
visitExpression(timezone, context);
}
return null;
}

Expand Down Expand Up @@ -2491,11 +2515,21 @@ public <S> Void visit(JsonAggregateFunction expression, S context) {
if (expr != null) {
expr.accept(this, context);
}
if (expression.getKey() instanceof Expression) {
((Expression) expression.getKey()).accept(this, context);
}
if (expression.getValue() instanceof Expression) {
((Expression) expression.getValue()).accept(this, context);
}
visitOrderBy(expression.getExpressionOrderByElements(), context);

expr = expression.getFilterExpression();
if (expr != null) {
expr.accept(this, context);
}
visitExpressions(expression.getPartitionExpressionList(), context);
visitOrderBy(expression.getOrderByElements(), context);
visitWindowElement(expression.getWindowElement(), context);
return null;
}

Expand Down
125 changes: 123 additions & 2 deletions src/test/java/net/sf/jsqlparser/util/TablesNamesFinderTraversalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import net.sf.jsqlparser.JSQLParserException;
import net.sf.jsqlparser.expression.ArrayExpression;
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.parser.AbstractJSqlParser.Dialect;
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
import net.sf.jsqlparser.schema.Table;
import net.sf.jsqlparser.statement.Statement;
Expand All @@ -30,6 +31,8 @@
import org.junit.jupiter.params.provider.ValueSource;

class TablesNamesFinderTraversalTest {
private static final String SUBQUERY = "(SELECT max(x) FROM t2)";

@ParameterizedTest
@MethodSource("nestedTables")
void findsTablesInNestedChildren(String sql, List<String> expected) throws JSQLParserException {
Expand Down Expand Up @@ -98,7 +101,110 @@ private static Stream<Arguments> nestedTables() {
tables("UPSERT INTO t1 VALUES (1)", "t1"),
tables("SELECT POSITION('x' IN 'xx') FROM t1", "t1"),
tables("IF a>1 DELETE FROM t3", "t3"),
tables("SELECT sum(x) OVER (ORDER BY (SELECT a FROM t2)) FROM t1", "t1", "t2"));
tables("SELECT sum(x) OVER (ORDER BY (SELECT a FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT max(a) KEEP (DENSE_RANK FIRST ORDER BY (SELECT x FROM t2)) FROM t1",
"t1", "t2"),
tables("SELECT max(a) KEEP (DENSE_RANK FIRST ORDER BY (SELECT x FROM t2)) OVER (PARTITION BY b) FROM t1",
"t1", "t2"),
tables("SELECT array_agg(a LIMIT (SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT any_value(a HAVING MAX (SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT any_value(a HAVING MAX (SELECT x FROM t2)) OVER () FROM t1", "t1",
"t2"),
tables("SELECT group_concat(a SEPARATOR (SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT group_concat(a ORDER BY (SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT f(a).g((SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT a FROM t1 WHERE (SELECT x FROM t2) IS UNKNOWN", "t1", "t2"),
tables("SELECT a FROM t1 WHERE MATCH (a) AGAINST ((SELECT x FROM t2))", "t1",
"t2"),
tables("SELECT JSON_OBJECTAGG(KEY a VALUE (SELECT x FROM t2)) FROM t1", "t1",
"t2"),
tables("SELECT JSON_ARRAYAGG(a ORDER BY (SELECT x FROM t2)) FROM t1", "t1", "t2"),
tables("SELECT JSON_ARRAYAGG(a) OVER (PARTITION BY (SELECT x FROM t2) ORDER BY (SELECT y FROM t3)) FROM t1",
"t1", "t2", "t3"),
tables("SELECT JSON_ARRAYAGG(a) OVER (ORDER BY b ROWS (SELECT x FROM t2) PRECEDING) FROM t1",
"t1", "t2"),
tables("SELECT * FROM XMLTABLE('/r' PASSING (SELECT x FROM t2) COLUMNS a int PATH 'a') x",
"t2"),
tables("SELECT * FROM JSON_TABLE((SELECT x FROM t2), '$' COLUMNS (a int PATH '$.a')) j",
"t2"),
tables("SELECT * FROM generate_series(1, (SELECT max(x) FROM t2)) g", "t2"));
}

@ParameterizedTest
@MethodSource("postgresTables")
void findsTablesInPostgresExpressionChildren(String sql, List<String> expected)
throws JSQLParserException {
Statement statement = CCJSqlParserUtil.parse(sql,
parser -> parser.withDialect(Dialect.POSTGRESQL));
assertThat(new TablesNamesFinder<>().getTables(statement))
.containsExactlyInAnyOrderElementsOf(expected);
assertThat(new TablesNamesFinder<>().getTablesOrOtherSources(statement))
.containsAll(expected);
}

private static Stream<Arguments> postgresTables() {
return Stream.of(
// aggregate ORDER BY inside the call
tables("SELECT string_agg(a, ',' ORDER BY " + SUBQUERY + ") FROM t1", "t1",
"t2"),
tables("SELECT array_agg(a ORDER BY " + SUBQUERY + ") FROM t1", "t1",
"t2"),
// LIKE ... ESCAPE
tables("SELECT a FROM t1 WHERE a LIKE 'a' ESCAPE " + SUBQUERY, "t1",
"t2"),
// AT TIME ZONE, zone operand
tables("SELECT now() AT TIME ZONE " + SUBQUERY, "t2"),
// JSON operators, right operand
tables("SELECT j -> " + SUBQUERY + " FROM t1", "t1", "t2"),
tables("SELECT j ->> " + SUBQUERY + " FROM t1", "t1", "t2"),
tables("SELECT j #> ARRAY[" + SUBQUERY + "] FROM t1", "t1", "t2"),
tables("SELECT j #>> ARRAY[" + SUBQUERY + "] FROM t1", "t1", "t2"),
// array slice bounds and subscript
tables("SELECT a[1:(SELECT count(*)::int FROM t2)] FROM t1", "t1",
"t2"),
tables("SELECT a[(SELECT count(*)::int FROM t2):2] FROM t1", "t1",
"t2"),
tables("SELECT a[(SELECT count(*)::int FROM t2)] FROM t1", "t1",
"t2"),
// positions that were already traversed
tables("SELECT f(" + SUBQUERY + ") FROM t1", "t1", "t2"),
tables("SELECT count(*) FILTER (WHERE a = " + SUBQUERY + ") FROM t1", "t1",
"t2"),
tables("SELECT percentile_cont(0.5) WITHIN GROUP (ORDER BY " + SUBQUERY
+ ") FROM t1", "t1", "t2"),
tables("SELECT sum(a) OVER (PARTITION BY " + SUBQUERY + ") FROM t1", "t1",
"t2"),
tables("SELECT sum(a) OVER (ORDER BY " + SUBQUERY + ") FROM t1", "t1",
"t2"),
tables("SELECT sum(a) OVER (ORDER BY b ROWS BETWEEN " + SUBQUERY
+ " PRECEDING AND CURRENT ROW) FROM t1", "t1", "t2"),
tables("SELECT a FROM t1 LIMIT " + SUBQUERY, "t1", "t2"),
tables("SELECT a FROM t1 OFFSET " + SUBQUERY, "t1", "t2"),
tables("SELECT a FROM t1 FETCH FIRST " + SUBQUERY + " ROWS ONLY", "t1",
"t2"),
tables("SELECT a FROM t1 WHERE a = ANY (SELECT x FROM t2)", "t1",
"t2"),
tables("SELECT a FROM t1 WHERE a > ALL (SELECT x FROM t2)", "t1",
"t2"),
tables("SELECT a FROM t1 WHERE a = SOME (SELECT x FROM t2)", "t1",
"t2"),
tables("SELECT ARRAY(SELECT x FROM t2)", "t2"),
tables("SELECT * FROM (VALUES (" + SUBQUERY + ")) v(x)", "t2"),
tables("SELECT substring(a FROM " + SUBQUERY + ") FROM t1", "t1", "t2"),
tables("SELECT trim(BOTH " + SUBQUERY + " FROM a) FROM t1", "t1", "t2"),
tables("SELECT position(" + SUBQUERY + " IN a) FROM t1", "t1", "t2"),
tables("SELECT overlay(a PLACING " + SUBQUERY + " FROM 1) FROM t1", "t1",
"t2"),
tables("SELECT CASE WHEN a = 1 THEN " + SUBQUERY + " END FROM t1", "t1",
"t2"),
tables("SELECT CAST(" + SUBQUERY + " AS text) FROM t1", "t1", "t2"),
tables("SELECT " + SUBQUERY + "::text FROM t1", "t1", "t2"),
tables("SELECT count(*) FROM t1 GROUP BY " + SUBQUERY, "t1", "t2"),
tables("SELECT count(*) FROM t1 GROUP BY GROUPING SETS ((" + SUBQUERY + "))", "t1",
"t2"),
tables("SELECT DISTINCT ON (" + SUBQUERY + ") a FROM t1", "t1", "t2"),
tables("SELECT * FROM t1, LATERAL (SELECT x FROM t2) p", "t1",
"t2"));
}

private static Arguments tables(String sql, String... names) {
Expand All @@ -122,7 +228,12 @@ private static Stream<Arguments> expressionTables() {
tables("f()[OFFSET((SELECT x FROM t2)):OFFSET((SELECT y FROM t3))]", "t2", "t3"),
tables("POSITION('x' IN (SELECT a FROM t2))", "t2"),
tables("sum(x) OVER (PARTITION BY (SELECT a FROM t2))", "t2"),
tables("a[1]"), tables("f()[1]"), tables("f()[:2]"), tables("f()[:]"));
tables("string_agg(a, ',' ORDER BY (SELECT x FROM t2))", "t2"),
tables("a LIKE 'a' ESCAPE (SELECT x FROM t2)", "t2"),
tables("now() AT TIME ZONE (SELECT x FROM t2)", "t2"),
tables("(SELECT x FROM t2) IS UNKNOWN", "t2"),
tables("a[1]"), tables("f()[1]"), tables("f()[:2]"), tables("f()[:]"),
tables("f(a).b.c"));
}

@ParameterizedTest
Expand Down Expand Up @@ -164,6 +275,16 @@ void visitsNewChildrenWithoutRepeatingExistingChildren() throws JSQLParserExcept
assertThat(finder.visits).isEqualTo(Map.of("t1", 1, "t2", 1, "t3", 1, "t4", 1, "t5", 1));
}

@Test
void visitsAggregateChildrenOnce() throws JSQLParserException {
CountingFinder finder = new CountingFinder();
Statement statement = CCJSqlParserUtil.parse(
"SELECT max(a) KEEP (DENSE_RANK FIRST ORDER BY (SELECT x FROM t2)) OVER (PARTITION BY (SELECT y FROM t3)), string_agg(a, ',' ORDER BY (SELECT z FROM t4)) FROM t1");
assertThat(finder.getTables(statement)).containsExactlyInAnyOrder("t1", "t2", "t3",
"t4");
assertThat(finder.visits).isEqualTo(Map.of("t1", 1, "t2", 1, "t3", 1, "t4", 1));
}

@Test
void resetsStateBetweenStatementAndExpressionVisits() throws JSQLParserException {
TablesNamesFinder<?> finder = new TablesNamesFinder<>();
Expand Down
Loading