avoid quadratic member lookups in table and alias visitors (#2763)

TablesNamesFinder tracked derived-table, CTE and lateral names in an ArrayList and matched every plain table reference against it, and AddAliasesVisitor checked each generated alias against a LinkedList, so both walked parsed SQL in O(n^2). Both collections are used only for membership, so back them with a LinkedHashSet.

Signed-off-by: Javid Khan <dxbjavid@gmail.com>

Author: dxbjavid

src/main/java/net/sf/jsqlparser/util/AddAliasesVisitor.java

@@ -9,8 +9,8 @@
  */
 package net.sf.jsqlparser.util;
 
-import java.util.LinkedList;
-import java.util.List;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import java.util.Locale;
 import net.sf.jsqlparser.expression.Alias;
@@ -37,7 +37,7 @@
 public class AddAliasesVisitor<T> implements SelectVisitor<T>, SelectItemVisitor<T> {
 
     private static final String NOT_SUPPORTED_YET = "Not supported yet.";
-    private final List<String> aliases = new LinkedList<String>();
+    private final Set<String> aliases = new LinkedHashSet<String>();
     private boolean firstRun = true;
     private int counter = 0;
     private String prefix = "A";

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

@@ -11,6 +11,7 @@
 
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -265,7 +266,7 @@ public class TablesNamesFinder<Void>
     private boolean allowColumnProcessing = false;
     private Set<String> rowPatternVariables = java.util.Collections.emptySet();
 
-    private List<String> otherItemNames;
+    private Set<String> otherItemNames;
 
     public static Set<String> findTables(String sqlStr) throws JSQLParserException {
         TablesNamesFinder<?> tablesNamesFinder = new TablesNamesFinder<>();
@@ -1309,7 +1310,7 @@ public <S> Void visit(DateUnitExpression dateUnitExpression, S context) {
      * @param allowColumnProcessing
      */
     protected void init(boolean allowColumnProcessing) {
-        otherItemNames = new ArrayList<String>();
+        otherItemNames = new LinkedHashSet<String>();
         tables = new HashSet<>();
         this.allowColumnProcessing = allowColumnProcessing;
         this.rowPatternVariables = java.util.Collections.emptySet();

src/test/java/net/sf/jsqlparser/util/AddAliasesVisitorTest.java

@@ -15,8 +15,10 @@
 import org.junit.jupiter.api.Test;
 
 import java.io.StringReader;
+import java.time.Duration;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
 
 public class AddAliasesVisitorTest {
 
@@ -68,4 +70,24 @@ public void testVisit_SetOperationList() throws JSQLParserException {
         assertEquals("SELECT 3 + 4 AS A1 FROM test UNION SELECT 7 + 8 AS A1 FROM test2",
                 setOpList.toString());
     }
+
+    @Test
+    public void testManySelectItemsDoNotScaleQuadratically() throws JSQLParserException {
+        // Generated aliases were checked against the already-used ones with a linear
+        // LinkedList.contains, so adding aliases to a select with m items cost O(m^2).
+        final int m = 50000;
+        final StringBuilder sb = new StringBuilder("SELECT ");
+        for (int i = 0; i < m; i++) {
+            if (i > 0) {
+                sb.append(", ");
+            }
+            sb.append("c").append(i);
+        }
+        sb.append(" FROM t");
+
+        // Parsing is left outside the timed region; only the alias walk is measured.
+        final Select select = (Select) parserManager.parse(new StringReader(sb.toString()));
+        assertTimeoutPreemptively(Duration.ofSeconds(2),
+                () -> select.accept(new AddAliasesVisitor<>(), null));
+    }
 }

src/test/java/net/sf/jsqlparser/util/TablesNamesFinderTest.java

@@ -14,8 +14,10 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
@@ -1062,4 +1064,32 @@ void testUpdateOrderByLimitSubquery() throws JSQLParserException {
                 "MY_TABLE2");
     }
 
+    @Test
+    void testManyDerivedNamesDoNotScaleQuadratically() throws JSQLParserException {
+        // Each aliased WITH item is remembered in otherItemNames and every plain table
+        // reference used to be matched against it with a linear ArrayList.contains, so a
+        // statement with n CTE aliases and n tables cost O(n^2) to walk.
+        final int n = 30000;
+        final StringBuilder sb = new StringBuilder("WITH ");
+        for (int i = 0; i < n; i++) {
+            if (i > 0) {
+                sb.append(", ");
+            }
+            sb.append("a").append(i).append(" AS (SELECT 1)");
+        }
+        sb.append(" SELECT * FROM ");
+        for (int i = 0; i < n; i++) {
+            if (i > 0) {
+                sb.append(", ");
+            }
+            sb.append("t").append(i);
+        }
+
+        // Parsing is linear and left outside the timed region; only the table walk is measured.
+        final Statement statement = CCJSqlParserUtil.parse(sb.toString());
+        final Set<String> tables = assertTimeoutPreemptively(Duration.ofSeconds(2),
+                () -> new TablesNamesFinder<>().getTables(statement));
+        assertEquals(n, tables.size());
+    }
+
 }