avoid quadratic positional access over linked-list column lists (#2860)

Author: tanjiroK-coder

src/main/java/net/sf/jsqlparser/statement/select/Select.java

@@ -131,9 +131,13 @@ public static StringBuilder appendStringListTo(StringBuilder builder, List<?> li
                 builder.append("(");
             }
 
-            int size = list.size();
-            for (int i = 0; i < size; i++) {
-                builder.append(list.get(i)).append(i < size - 1 ? comma : "");
+            boolean first = true;
+            for (Object element : list) {
+                if (!first) {
+                    builder.append(comma);
+                }
+                builder.append(element);
+                first = false;
             }
 
             if (useBrackets) {

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

@@ -9,6 +9,7 @@
  */
 package net.sf.jsqlparser.util;
 
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -70,15 +71,16 @@ public <S> T visit(PlainSelect plainSelect, S context) {
         }
 
         if (itemsExpr.size() > 1) {
+            Iterator<SelectItem<? extends Expression>> it = itemsExpr.iterator();
             BinaryExpression binExpr = createBinaryExpression();
-            binExpr.setLeftExpression(itemsExpr.get(0).getExpression());
-            for (int i = 1; i < itemsExpr.size() - 1; i++) {
-                binExpr.setRightExpression(itemsExpr.get(i).getExpression());
+            binExpr.setLeftExpression(it.next().getExpression());
+            binExpr.setRightExpression(it.next().getExpression());
+            while (it.hasNext()) {
                 BinaryExpression binExpr2 = createBinaryExpression();
                 binExpr2.setLeftExpression(binExpr);
+                binExpr2.setRightExpression(it.next().getExpression());
                 binExpr = binExpr2;
             }
-            binExpr.setRightExpression(itemsExpr.get(itemsExpr.size() - 1).getExpression());
 
             SelectItem<Expression> sei = new SelectItem<>();
             sei.setExpression(binExpr);

src/test/java/net/sf/jsqlparser/statement/select/JoinUsingDeParseTest.java

@@ -0,0 +1,48 @@
+/*-
+ * #%L
+ * JSQLParser library
+ * %%
+ * Copyright (C) 2004 - 2019 JSQLParser
+ * %%
+ * Dual licensed under GNU LGPL 2.1 or Apache License 2.0
+ * #L%
+ */
+package net.sf.jsqlparser.statement.select;
+
+import net.sf.jsqlparser.parser.CCJSqlParserUtil;
+import net.sf.jsqlparser.statement.Statement;
+import org.junit.jupiter.api.Test;
+
+import java.time.Duration;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+public class JoinUsingDeParseTest {
+
+    private static String usingSelect(int columnCount) {
+        StringBuilder sb = new StringBuilder("SELECT * FROM a JOIN b USING (");
+        for (int i = 0; i < columnCount; i++) {
+            if (i > 0) {
+                sb.append(", ");
+            }
+            sb.append("c").append(i);
+        }
+        return sb.append(")").toString();
+    }
+
+    @Test
+    void deParseOfLargeUsingColumnListIsLinear() throws Exception {
+        // usingColumns is a LinkedList, so deparsing the clause through
+        // Select.appendStringListTo used to read it by index (get(i)), which is O(n^2).
+        String sql = usingSelect(200000);
+        Statement statement = CCJSqlParserUtil.parse(sql);
+        assertEquals(sql, assertTimeoutPreemptively(Duration.ofSeconds(5), statement::toString));
+    }
+
+    @Test
+    void deParseOfUsingColumnListIsUnchanged() throws Exception {
+        String sql = "SELECT * FROM a JOIN b USING (c0, c1, c2)";
+        assertEquals(sql, CCJSqlParserUtil.parse(sql).toString());
+    }
+}

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

@@ -18,8 +18,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 ConnectExpressionsVisitorTest {
 
@@ -54,4 +56,28 @@ protected BinaryExpression createBinaryExpression() {
 
         assertEquals("SELECT a + b + c AS testexpr FROM test", select.toString());
     }
+
+    @Test
+    public void testVisit_PlainSelect_manyItemsIsLinear() throws JSQLParserException {
+        int itemCount = 200000;
+        StringBuilder sql = new StringBuilder("select ");
+        for (int i = 0; i < itemCount; i++) {
+            if (i > 0) {
+                sql.append(',');
+            }
+            sql.append('c').append(i);
+        }
+        sql.append(" from test");
+        Select select = (Select) parserManager.parse(new StringReader(sql.toString()));
+        ConnectExpressionsVisitor<Void> instance = new ConnectExpressionsVisitor<>() {
+            @Override
+            protected BinaryExpression createBinaryExpression() {
+                return new Concat();
+            }
+        };
+        // itemsExpr is a LinkedList; connecting the items used to index it by position (get(i)),
+        // which is O(n^2) in the number of select items.
+        assertTimeoutPreemptively(Duration.ofSeconds(5), () -> select.accept(instance, null));
+        assertEquals(1, select.getPlainSelect().getSelectItems().size());
+    }
 }