Skip to content

Commit 1ac7626

Browse files
fix: resolve WITH items by scope in TablesNamesFinder
getTables removed every alias and WITH name from its result wherever it was declared, so a real table with such a name went missing: SELECT * FROM (SELECT * FROM secrets) AS secrets found no tables. A name is now left out only where it resolves to a WITH item in scope, and the target of a DML statement never does. WITH lists on VALUES and TABLE selects are now walked too.
1 parent dd03d6f commit 1ac7626

3 files changed

Lines changed: 200 additions & 62 deletions

File tree

‎src/main/java/net/sf/jsqlparser/util/TablesNamesFinder.java‎

Lines changed: 85 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package net.sf.jsqlparser.util;
1111

1212
import java.util.ArrayList;
13+
import java.util.HashMap;
1314
import java.util.HashSet;
1415
import java.util.LinkedHashSet;
1516
import java.util.List;
@@ -287,6 +288,9 @@ public class TablesNamesFinder<Void>
287288
private Set<String> rowPatternVariables = java.util.Collections.emptySet();
288289

289290
private Set<String> otherItemNames;
291+
// the WITH items a table reference resolves to at the current position, counted because a
292+
// nested WITH may repeat a name of an enclosing one
293+
private Map<String, Integer> withItemsInScope;
290294

291295
public static Set<String> findTables(String sqlStr) throws JSQLParserException {
292296
TablesNamesFinder<?> tablesNamesFinder = new TablesNamesFinder<>();
@@ -317,10 +321,6 @@ public Set<String> getTables(Statement statement) {
317321
init(false);
318322
statement.accept(this, null);
319323

320-
// @todo: assess this carefully, maybe we want to remove more specifically
321-
// only Aliases on WithItems, Parenthesed Selects and Lateral Selects
322-
otherItemNames.forEach(tables::remove);
323-
324324
return tables;
325325
}
326326

@@ -336,13 +336,9 @@ public Set<String> getTablesOrOtherSources(Statement statement) {
336336

337337
@Override
338338
public <S> Void visit(Select select, S context) {
339-
List<WithItem<?>> withItemsList = select.getWithItemsList();
340-
if (withItemsList != null && !withItemsList.isEmpty()) {
341-
for (WithItem<?> withItem : withItemsList) {
342-
withItem.accept((SelectVisitor<?>) this, context);
343-
}
344-
}
339+
visitWithItems(select.getWithItemsList(), context);
345340
select.accept((SelectVisitor<?>) this, context);
341+
leaveWithItems(select.getWithItemsList());
346342
return null;
347343
}
348344

@@ -401,6 +397,56 @@ public Set<String> getTables(Expression expr) {
401397
return tables;
402398
}
403399

400+
/**
401+
* Visits the WITH items of a query and brings their names into scope, which the caller ends
402+
* with {@link #leaveWithItems(List)} after the query. An item only sees the items before it, so
403+
* a reference to its own or a later name is a table, unless RECURSIVE makes all of them visible
404+
* to each other. The parser sets RECURSIVE on the first item, but it covers the list.
405+
*/
406+
@Override
407+
public <S> Void visitWithItems(List<WithItem<?>> withItemsList, S context) {
408+
if (withItemsList != null) {
409+
boolean recursive = withItemsList.stream().anyMatch(WithItem::isRecursive);
410+
if (recursive) {
411+
withItemsList.forEach(this::enterWithItem);
412+
}
413+
for (WithItem<?> withItem : withItemsList) {
414+
withItem.accept((SelectVisitor<?>) this, context);
415+
if (!recursive) {
416+
enterWithItem(withItem);
417+
}
418+
}
419+
}
420+
return null;
421+
}
422+
423+
private void enterWithItem(WithItem<?> withItem) {
424+
if (withItem.getAlias() != null) {
425+
withItemsInScope.merge(withItem.getAlias().getName(), 1, Integer::sum);
426+
}
427+
}
428+
429+
private void leaveWithItems(List<WithItem<?>> withItemsList) {
430+
if (withItemsList != null) {
431+
for (WithItem<?> withItem : withItemsList) {
432+
if (withItem.getAlias() != null) {
433+
withItemsInScope.computeIfPresent(withItem.getAlias().getName(),
434+
(name, count) -> count > 1 ? count - 1 : null);
435+
}
436+
}
437+
}
438+
}
439+
440+
/**
441+
* The target of a DML statement is a table even if a WITH item in scope has the same name.
442+
*/
443+
private <S> void visitTargetTable(Table table, S context) {
444+
Map<String, Integer> scope = withItemsInScope;
445+
withItemsInScope = new HashMap<>();
446+
visit(table, context);
447+
withItemsInScope = scope;
448+
}
449+
404450
@Override
405451
public <S> Void visit(WithItem<?> withItem, S context) {
406452
if (withItem.getAlias() != null) {
@@ -428,12 +474,7 @@ public <S> Void visit(ParenthesedSelect select, S context) {
428474
if (select.getAlias() != null) {
429475
otherItemNames.add(select.getAlias().getName());
430476
}
431-
List<WithItem<?>> withItemsList = select.getWithItemsList();
432-
if (withItemsList != null && !withItemsList.isEmpty()) {
433-
for (WithItem<?> withItem : withItemsList) {
434-
withItem.accept((SelectVisitor<?>) this, context);
435-
}
436-
}
477+
visitWithItems(select.getWithItemsList(), context);
437478
select.getSelect().accept((SelectVisitor<?>) this, context);
438479
visitOrderBy(select.getOrderByElements(), context);
439480
if (select.getPivot() != null) {
@@ -449,6 +490,7 @@ public <S> Void visit(ParenthesedSelect select, S context) {
449490
if (select.getFetch() != null) {
450491
select.getFetch().getExpression().accept(this, context);
451492
}
493+
leaveWithItems(select.getWithItemsList());
452494
return null;
453495
}
454496

@@ -517,6 +559,7 @@ private <S> Void visitPlainSelectBody(PlainSelect plainSelect, S context) {
517559
visitSelectPagination(plainSelect, context);
518560
visitUpdateSets(plainSelect.getSettings(), context);
519561
visitFromItem(plainSelect.getIntoTempTable(), context);
562+
leaveWithItems(plainSelect.getWithItemsList());
520563
return null;
521564
}
522565

@@ -531,6 +574,7 @@ public <S> Void visit(PivotQuery pivotQuery, S context) {
531574
visitLimit(pivotQuery.getLimit(), context);
532575

533576
visitSelectPagination(pivotQuery, context);
577+
leaveWithItems(pivotQuery.getWithItemsList());
534578
return null;
535579
}
536580

@@ -540,6 +584,7 @@ public <S> Void visit(UnPivotQuery unPivotQuery, S context) {
540584
visitFromItem(unPivotQuery.getFromItem(), context);
541585
visitExpressions(unPivotQuery.getOnExpressions(), context);
542586
visitExpressions(unPivotQuery.getValueColumns(), context);
587+
leaveWithItems(unPivotQuery.getWithItemsList());
543588
return null;
544589
}
545590

@@ -617,7 +662,7 @@ public <S> Void visit(Table table, S context) {
617662
String tableWholeName = extractTableName(table);
618663
if (table.isTableVariable()) {
619664
otherItemNames.add(tableWholeName);
620-
} else if (!otherItemNames.contains(tableWholeName)) {
665+
} else if (!withItemsInScope.containsKey(table.getFullyQualifiedName())) {
621666
tables.add(tableWholeName);
622667
}
623668
if (table.getPivot() != null) {
@@ -665,6 +710,7 @@ public <S> Void visit(OverlapsCondition overlapsCondition, S context) {
665710
public <S> Void visit(Column tableColumn, S context) {
666711
if (allowColumnProcessing && tableColumn.getTable() != null
667712
&& tableColumn.getTable().getName() != null
713+
&& !otherItemNames.contains(extractTableName(tableColumn.getTable()))
668714
&& !rowPatternVariables.contains(
669715
MatchRecognize.normalizeVariableName(tableColumn.getTable().getName()))) {
670716
visit(tableColumn.getTable(), context);
@@ -1108,12 +1154,7 @@ private <S> void visitWindowElement(WindowElement windowElement, S context) {
11081154

11091155
@Override
11101156
public <S> Void visit(SetOperationList list, S context) {
1111-
List<WithItem<?>> withItemsList = list.getWithItemsList();
1112-
if (withItemsList != null && !withItemsList.isEmpty()) {
1113-
for (WithItem<?> withItem : withItemsList) {
1114-
withItem.accept((SelectVisitor<?>) this, context);
1115-
}
1116-
}
1157+
visitWithItems(list.getWithItemsList(), context);
11171158
for (Select selectBody : list.getSelects()) {
11181159
selectBody.accept((SelectVisitor<?>) this, context);
11191160
}
@@ -1125,6 +1166,7 @@ public <S> Void visit(SetOperationList list, S context) {
11251166
if (list.getFetch() != null) {
11261167
list.getFetch().getExpression().accept(this, context);
11271168
}
1169+
leaveWithItems(list.getWithItemsList());
11281170
return null;
11291171
}
11301172

@@ -1157,7 +1199,9 @@ public void visit(LateralSubSelect lateralSubSelect) {
11571199

11581200
@Override
11591201
public <S> Void visit(TableStatement tableStatement, S context) {
1202+
visitWithItems(tableStatement.getWithItemsList(), context);
11601203
tableStatement.getTable().accept(this, null);
1204+
leaveWithItems(tableStatement.getWithItemsList());
11611205
return null;
11621206
}
11631207

@@ -1168,12 +1212,7 @@ public void visit(TableStatement tableStatement) {
11681212

11691213
@Override
11701214
public <S> Void visit(FromQuery fromQuery, S context) {
1171-
List<WithItem<?>> withItemsList = fromQuery.getWithItemsList();
1172-
if (withItemsList != null && !withItemsList.isEmpty()) {
1173-
for (WithItem<?> withItem : withItemsList) {
1174-
withItem.accept((SelectVisitor<?>) this, context);
1175-
}
1176-
}
1215+
visitWithItems(fromQuery.getWithItemsList(), context);
11771216
if (fromQuery.getFromItem() != null) {
11781217
fromQuery.getFromItem().accept(this, context);
11791218
}
@@ -1186,6 +1225,7 @@ public <S> Void visit(FromQuery fromQuery, S context) {
11861225
for (PipeOperator pipeOperator : fromQuery.getPipeOperators()) {
11871226
pipeOperator.accept(this, null);
11881227
}
1228+
leaveWithItems(fromQuery.getWithItemsList());
11891229
return null;
11901230
}
11911231

@@ -1356,6 +1396,7 @@ public <S> Void visit(DateUnitExpression dateUnitExpression, S context) {
13561396
*/
13571397
protected void init(boolean allowColumnProcessing) {
13581398
otherItemNames = new LinkedHashSet<String>();
1399+
withItemsInScope = new HashMap<>();
13591400
tables = new HashSet<>();
13601401
this.allowColumnProcessing = allowColumnProcessing;
13611402
this.rowPatternVariables = java.util.Collections.emptySet();
@@ -1485,12 +1526,8 @@ public <S> Void visit(MySQLGroupConcat groupConcat, S context) {
14851526

14861527
@Override
14871528
public <S> Void visit(Delete delete, S context) {
1488-
if (delete.getWithItemsList() != null) {
1489-
for (WithItem<?> withItem : delete.getWithItemsList()) {
1490-
withItem.accept((SelectVisitor<?>) this, context);
1491-
}
1492-
}
1493-
visit(delete.getTable(), context);
1529+
visitWithItems(delete.getWithItemsList(), context);
1530+
visitTargetTable(delete.getTable(), context);
14941531

14951532
if (delete.getUsingFromItemList() != null) {
14961533
for (FromItem usingFromItem : delete.getUsingFromItemList()) {
@@ -1510,6 +1547,7 @@ public <S> Void visit(Delete delete, S context) {
15101547
visitLimit(delete.getLimit(), context);
15111548
visitOutputClause(delete.getOutputClause(), context);
15121549
visitReturningClause(delete.getReturningClause(), context);
1550+
leaveWithItems(delete.getWithItemsList());
15131551
return null;
15141552
}
15151553

@@ -1530,14 +1568,10 @@ public <S> Void visit(SessionStatement sessionStatement, S context) {
15301568

15311569
@Override
15321570
public <S> Void visit(Update update, S context) {
1533-
if (update.getWithItemsList() != null) {
1534-
for (WithItem<?> withItem : update.getWithItemsList()) {
1535-
withItem.accept((SelectVisitor<?>) this, context);
1536-
}
1537-
}
1571+
visitWithItems(update.getWithItemsList(), context);
15381572

15391573
if (!update.isTargetTableAlias()) {
1540-
visit(update.getTable(), context);
1574+
visitTargetTable(update.getTable(), context);
15411575
}
15421576

15431577
visitJoins(update.getStartJoins(), context);
@@ -1565,6 +1599,7 @@ public <S> Void visit(Update update, S context) {
15651599
visitLimit(update.getLimit(), context);
15661600
visitOutputClause(update.getOutputClause(), context);
15671601
visitReturningClause(update.getReturningClause(), context);
1602+
leaveWithItems(update.getWithItemsList());
15681603
return null;
15691604
}
15701605

@@ -1589,20 +1624,16 @@ public <S> Void visit(Insert insert, S context) {
15891624
continue;
15901625
}
15911626
for (OracleMultiInsertClause clause : branch.getClauses()) {
1592-
visit(clause.getTable(), context);
1627+
visitTargetTable(clause.getTable(), context);
15931628
if (clause.getSelect() != null) {
15941629
visit(clause.getSelect(), context);
15951630
}
15961631
}
15971632
}
15981633
} else if (insert.getTable() != null) {
1599-
visit(insert.getTable(), context);
1600-
}
1601-
if (insert.getWithItemsList() != null) {
1602-
for (WithItem<?> withItem : insert.getWithItemsList()) {
1603-
withItem.accept((SelectVisitor<?>) this, context);
1604-
}
1634+
visitTargetTable(insert.getTable(), context);
16051635
}
1636+
visitWithItems(insert.getWithItemsList(), context);
16061637
if (insert.getSetUpdateSets() != null) {
16071638
visitUpdateSets(insert.getSetUpdateSets(), context);
16081639
}
@@ -1620,6 +1651,7 @@ public <S> Void visit(Insert insert, S context) {
16201651
if (insert.getSelect() != null) {
16211652
visit(insert.getSelect(), context);
16221653
}
1654+
leaveWithItems(insert.getWithItemsList());
16231655
return null;
16241656
}
16251657

@@ -2032,12 +2064,8 @@ public <S> Void visit(HexValue hexValue, S context) {
20322064

20332065
@Override
20342066
public <S> Void visit(Merge merge, S context) {
2035-
visit(merge.getTable(), context);
2036-
if (merge.getWithItemsList() != null) {
2037-
for (WithItem<?> withItem : merge.getWithItemsList()) {
2038-
withItem.accept((SelectVisitor<?>) this, context);
2039-
}
2040-
}
2067+
visitTargetTable(merge.getTable(), context);
2068+
visitWithItems(merge.getWithItemsList(), context);
20412069

20422070
if (merge.getFromItem() != null) {
20432071
merge.getFromItem().accept(this, context);
@@ -2054,6 +2082,7 @@ public <S> Void visit(Merge merge, S context) {
20542082
}
20552083
visitOutputClause(merge.getOutputClause(), context);
20562084
visitReturningClause(merge.getReturningClause(), context);
2085+
leaveWithItems(merge.getWithItemsList());
20572086
return null;
20582087
}
20592088

@@ -2266,6 +2295,7 @@ public void visit(Comment comment) {
22662295

22672296
@Override
22682297
public <S> Void visit(Values values, S context) {
2298+
visitWithItems(values.getWithItemsList(), context);
22692299
values.getExpressions().accept(this, context);
22702300
visitOrderBy(values.getOrderByElements(), context);
22712301
visitLimit(values.getLimit(), context);
@@ -2275,6 +2305,7 @@ public <S> Void visit(Values values, S context) {
22752305
if (values.getFetch() != null) {
22762306
values.getFetch().getExpression().accept(this, context);
22772307
}
2308+
leaveWithItems(values.getWithItemsList());
22782309
return null;
22792310
}
22802311

‎src/site/sphinx/usage.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,8 @@ Find Table Names
13351335
tableNames = TablesNamesFinder.findTablesInExpression(exprStr);
13361336
assertThat( tableNames ).containsExactlyInAnyOrder("A", "B", "C");
13371337
1338+
A name is left out only where it resolves to a ``WITH`` item in scope. An alias hides no table, and without ``RECURSIVE`` a ``WITH`` item does not see itself or the items after it. The target of a DML statement is always reported. So the result may hold names that are no tables on some databases, such as a ``WITH`` item that recurses without ``RECURSIVE``: check it against the metadata of the database if that matters.
1339+
13381340

13391341
Use the Visitor Patterns
13401342
==============================

0 commit comments

Comments
 (0)