Implement Joins with multiple trailing ON Expressions (#1303)

* Implement Joins with multiple trailing ON Expressions Fixes #1302 Fixes SpecialOracleTest JOIN17, now 190/273 tests pass * Fixes #1229 * Merge MASTER Refactor the appendTo() method in favour of the traditional toString() * Remove unused imports
JSQLParser · Sep 6, 2021 · d18c59b · d18c59b
1 parent 2e87613
commit d18c59b
Show file tree

Hide file tree

Showing 11 changed files with 115 additions and 68 deletions.
diff --git a/src/main/java/net/sf/jsqlparser/statement/select/Join.java b/src/main/java/net/sf/jsqlparser/statement/select/Join.java
@@ -9,11 +9,8 @@
  */
 package net.sf.jsqlparser.statement.select;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Optional;
+import java.util.*;
+
 import net.sf.jsqlparser.expression.Expression;
 import net.sf.jsqlparser.parser.ASTNodeAccessImpl;
 import net.sf.jsqlparser.schema.Column;
@@ -32,8 +29,8 @@ public class Join extends ASTNodeAccessImpl {
     private boolean straight = false;
     private boolean apply = false;
     private FromItem rightItem;
-    private Expression onExpression;
-    private List<Column> usingColumns;
+    private final LinkedList<Expression> onExpressions = new LinkedList<>();
+    private final LinkedList<Column> usingColumns = new LinkedList<>();
     private KSQLJoinWindow joinWindow;
 
     public boolean isSimple() {
@@ -228,17 +225,35 @@ public void setRightItem(FromItem item) {
     /**
      * Returns the "ON" expression (if any)
      */
+    @Deprecated
     public Expression getOnExpression() {
-        return onExpression;
+        return onExpressions.get(0);
+    }
+
+    public Collection<Expression> getOnExpressions() {
+        return onExpressions;
     }
 
+    @Deprecated
     public Join withOnExpression(Expression expression) {
         this.setOnExpression(expression);
         return this;
     }
 
+    @Deprecated
     public void setOnExpression(Expression expression) {
-        onExpression = expression;
+        onExpressions.add(0, expression);
+    }
+
+    public Join addOnExpression(Expression expression) {
+        onExpressions.add(expression);
+        return this;
+    }
+
+    public Join setOnExpressions(Collection<Expression> expressions) {
+        onExpressions.clear();
+        onExpressions.addAll(expressions);
+        return this;
     }
 
     /**
@@ -254,7 +269,8 @@ public Join withUsingColumns(List<Column> list) {
     }
 
     public void setUsingColumns(List<Column> list) {
-        usingColumns = list;
+        usingColumns.clear();
+        usingColumns.addAll(list);
     }
 
     public boolean isWindowJoin() {
@@ -281,46 +297,52 @@ public void setJoinWindow(KSQLJoinWindow joinWindow) {
     @Override
     @SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
     public String toString() {
+        StringBuilder builder = new StringBuilder();
+
         if (isSimple() && isOuter()) {
-            return "OUTER " + rightItem;
+            builder.append("OUTER ").append(rightItem);
         } else if (isSimple()) {
-            return "" + rightItem;
+            builder.append(rightItem);
         } else {
-            String type = "";
-
             if (isRight()) {
-                type += "RIGHT ";
+                builder.append("RIGHT ");
             } else if (isNatural()) {
-                type += "NATURAL ";
+                builder.append("NATURAL ");
             } else if (isFull()) {
-                type += "FULL ";
+                builder.append("FULL ");
             } else if (isLeft()) {
-                type += "LEFT ";
+                builder.append("LEFT ");
             } else if (isCross()) {
-                type += "CROSS ";
+                builder.append("CROSS ");
             }
 
             if (isOuter()) {
-                type += "OUTER ";
+                builder.append("OUTER ");
             } else if (isInner()) {
-                type += "INNER ";
+                builder.append("INNER ");
             } else if (isSemi()) {
-                type += "SEMI ";
+                builder.append("SEMI ");
             }
 
             if (isStraight()) {
-                type = "STRAIGHT_JOIN ";
+                builder.append("STRAIGHT_JOIN ");
             } else if (isApply()) {
-                type += "APPLY ";
+                builder.append("APPLY ");
             } else {
-                type += "JOIN ";
+                builder.append("JOIN ");
             }
 
-            return type + rightItem + ((joinWindow != null) ? " WITHIN " + joinWindow : "")
-                    + ((onExpression != null) ? " ON " + onExpression + "" : "")
-                    + PlainSelect.getFormatedList(usingColumns, "USING", true, true);
+            builder.append(rightItem).append((joinWindow != null) ? " WITHIN " + joinWindow : "");
+        }
+
+        for (Expression onExpression: onExpressions) {
+            builder.append(" ON ").append(onExpression);
+        }
+        if (usingColumns.size()>0) {
+            builder.append(PlainSelect.getFormatedList(usingColumns, "USING", true, true));
         }
 
+        return builder.toString();
     }
 
     public Join addUsingColumns(Column... usingColumns) {
@@ -334,12 +356,4 @@ public Join addUsingColumns(Collection<? extends Column> usingColumns) {
         collection.addAll(usingColumns);
         return this.withUsingColumns(collection);
     }
-
-    public <E extends FromItem> E getRightItem(Class<E> type) {
-        return type.cast(getRightItem());
-    }
-
-    public <E extends Expression> E getOnExpression(Class<E> type) {
-        return type.cast(getOnExpression());
-    }
 }
diff --git a/src/main/java/net/sf/jsqlparser/util/SelectUtils.java b/src/main/java/net/sf/jsqlparser/util/SelectUtils.java
@@ -83,7 +83,7 @@ public static void addExpression(Select select, final Expression expr) {
      */
     public static Join addJoin(Select select, final Table table, final Expression onExpression) {
         if (select.getSelectBody() instanceof PlainSelect) {
-            Join join = new Join().withRightItem(table).withOnExpression(onExpression);
+            Join join = new Join().withRightItem(table).addOnExpression(onExpression);
             select.getSelectBody(PlainSelect.class).addJoins(join);
             return join;
         } else {

diff --git a/src/main/java/net/sf/jsqlparser/util/deparser/SelectDeParser.java b/src/main/java/net/sf/jsqlparser/util/deparser/SelectDeParser.java
@@ -12,12 +12,7 @@
 import java.util.Iterator;
 import java.util.List;
 
-import net.sf.jsqlparser.expression.Alias;
-import net.sf.jsqlparser.expression.ExpressionVisitor;
-import net.sf.jsqlparser.expression.ExpressionVisitorAdapter;
-import net.sf.jsqlparser.expression.MySQLIndexHint;
-import net.sf.jsqlparser.expression.OracleHint;
-import net.sf.jsqlparser.expression.SQLServerHints;
+import net.sf.jsqlparser.expression.*;
 import net.sf.jsqlparser.expression.operators.relational.ExpressionList;
 import net.sf.jsqlparser.expression.operators.relational.ItemsList;
 import net.sf.jsqlparser.expression.operators.relational.ItemsListVisitor;
@@ -429,11 +424,11 @@ public void deparseJoin(Join join) {
             buffer.append(" WITHIN ");
             buffer.append(join.getJoinWindow().toString());
         }
-        if (join.getOnExpression() != null) {
+        for (Expression onExpression : join.getOnExpressions()) {
             buffer.append(" ON ");
-            join.getOnExpression().accept(expressionVisitor);
+            onExpression.accept(expressionVisitor);
         }
-        if (join.getUsingColumns() != null) {
+        if (join.getUsingColumns().size()>0) {
             buffer.append(" USING (");
             for (Iterator<Column> iterator = join.getUsingColumns().iterator(); iterator.hasNext();) {
                 Column column = iterator.next();

diff --git a/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java b/src/main/java/net/sf/jsqlparser/util/validation/validator/SelectValidator.java
@@ -10,6 +10,8 @@
 package net.sf.jsqlparser.util.validation.validator;
 
 import java.util.List;
+
+import net.sf.jsqlparser.expression.Expression;
 import net.sf.jsqlparser.expression.MySQLIndexHint;
 import net.sf.jsqlparser.expression.SQLServerHints;
 import net.sf.jsqlparser.parser.feature.Feature;
@@ -241,7 +243,9 @@ public void validateOptionalJoin(Join join) {
         }
 
         validateOptionalFromItem(join.getRightItem());
-        validateOptionalExpression(join.getOnExpression());
+        for (Expression onExpression: join.getOnExpressions()) {
+            validateOptionalExpression(onExpression);
+        }
         validateOptionalExpressions(join.getUsingColumns());
     }
 

diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt
@@ -1641,7 +1641,7 @@ String RelObjectName() :
 {
     (result = RelObjectNameWithoutValue()
 		| tk=<K_GROUP> | tk=<K_INTERVAL> | tk=<K_ON> | tk=<K_ORDER> | tk=<K_START> | tk=<K_TOP> | tk=<K_VALUE> 
-        | tk=<K_VALUES> | tk=<K_CREATE> | tk=<K_TABLES>  )
+        | tk=<K_VALUES> | tk=<K_CREATE> | tk=<K_TABLES> )
 
     {
 		if (tk!=null) result=tk.image;
@@ -1653,7 +1653,7 @@ String RelObjectNameWithoutStart() :
 {    Token tk = null; String result = null; }
 {
     (result = RelObjectNameWithoutValue() | tk=<K_TOP> | tk=<K_VALUE> | tk=<K_VALUES>
-        | tk=<K_INTERVAL>)
+        | tk=<K_INTERVAL> )
 
     {
 		if (tk!=null) result=tk.image;
@@ -1672,7 +1672,7 @@ String RelObjectNameExt():
     ( result=RelObjectName() | tk=<K_ALL> | tk=<K_ANY>  | tk=<K_SOME> | tk=<K_LEFT> | tk=<K_RIGHT> | tk=<K_SET>
         | tk=<K_DOUBLE> | tk=<K_IF> | tk=<K_IIF> | tk=<K_OPTIMIZE> | tk=<K_LIMIT>
         | tk=<K_OFFSET> | tk=<K_PROCEDURE> | tk=<K_PUBLIC> 
-        | tk=<K_CASEWHEN> | tk=<K_IN> | tk=<K_GROUPING> )
+        | tk=<K_CASEWHEN> | tk=<K_IN> | tk=<K_GROUPING>  )
     {
         if (tk!=null) result=tk.image;
         return result;
@@ -2483,13 +2483,19 @@ Join JoinerExpression() #JoinerExpression:
 
 
     [
-    LOOKAHEAD(2) ([ <K_WITHIN> "(" joinWindow = JoinWindow() ")" {join.setJoinWindow(joinWindow);}]
-                    ( <K_ON> onExpression=Expression()  { join.setOnExpression(onExpression); })
-        |
-        ( <K_USING> "(" tableColumn=Column() { columns = new ArrayList<Column>(); columns.add(tableColumn); }
-                ("," tableColumn=Column() { columns.add(tableColumn); } )* ")"
-          { join.setUsingColumns(columns); }   ))
-      ]
+        LOOKAHEAD(2) (
+                        [ <K_WITHIN> "(" joinWindow = JoinWindow() ")" {join.setJoinWindow(joinWindow);} ]
+                        ( <K_ON> onExpression=Expression()  { join.addOnExpression(onExpression); }
+                          ( LOOKAHEAD(2)  <K_ON> onExpression=Expression()  { join.addOnExpression(onExpression); } )*
+                        )
+                        |
+                        (
+                            <K_USING> "(" tableColumn=Column() { columns = new ArrayList<Column>(); columns.add(tableColumn); }
+                            ( "," tableColumn=Column() { columns.add(tableColumn); } ) *
+                            ")" { join.setUsingColumns(columns); }
+                        )
+                    )
+    ]
   {
       linkAST(join,jjtThis);
       join.setRightItem(right);
@@ -5786,7 +5792,7 @@ AlterSystemStatement AlterSystemStatement():
             "QUIESCE" "RESTRICTED" { operation = AlterSystemOperation.QUIESCE; }
         )
         |
-        ( 
+        (
             "UNQUIESCE" { operation = AlterSystemOperation.UNQUIESCE; }
         )
         |

diff --git a/src/test/java/net/sf/jsqlparser/statement/alter/RenameTableStatementTest.java b/src/test/java/net/sf/jsqlparser/statement/alter/RenameTableStatementTest.java
@@ -89,7 +89,7 @@ public void testTableNamesFinder() throws JSQLParserException {
   public void testValidator() throws JSQLParserException {
     String sqlStr = "RENAME oldTableName TO newTableName";
 
-    ValidationTestAsserts.validateNoErrors(sqlStr, 1, DatabaseType.ORACLE);
+    ValidationTestAsserts.validateNoErrors(sqlStr, 1, DatabaseType.POSTGRESQL);
   }
 
   @Test

diff --git a/src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java b/src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java
@@ -4716,7 +4716,36 @@ public void testCollisionWithSpecialStringFunctionsIssue1284() throws JSQLParser
             "recv_time >= toDateTime('2021-07-20 00:00:00')\n" +
             "and recv_time < toDateTime('2021-07-21 00:00:00')", true);
     }
-
+
+    @Test
+    public void testJoinWithTrailingOnExpressionIssue1302() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed(
+                "SELECT * FROM TABLE1 tb1\n" +
+                        "INNER JOIN TABLE2 tb2\n" +
+                        "INNER JOIN TABLE3 tb3\n" +
+                        "INNER JOIN TABLE4 tb4\n" +
+                        "ON (tb3.aaa = tb4.aaa)\n" +
+                        "ON (tb2.aaa = tb3.aaa)\n" +
+                        "ON (tb1.aaa = tb2.aaa)", true);
+
+        assertSqlCanBeParsedAndDeparsed(
+                "SELECT *\n" +
+                        "FROM\n" +
+                        "TABLE1 tbl1\n" +
+                        "    INNER JOIN TABLE2 tbl2\n" +
+                        "        INNER JOIN TABLE3 tbl3\n" +
+                        "        ON (tbl2.column1 = tbl3.column1)\n" +
+                        "    ON (tbl1.column2 = tbl2.column2)\n" +
+                        "WHERE\n" +
+                        "tbl1.column1 = 123", true);
+    }
+
+    @Test
+    public void testSimpleJoinOnExpressionIssue1229() throws JSQLParserException {
+        assertSqlCanBeParsedAndDeparsed(
+                "select t1.column1,t1.column2,t2.field1,t2.field2 from T_DT_ytb_01 t1 , T_DT_ytb_02 t2 on t1.column1 = t2.field1", true);
+ }
+
     @Test
     public void testNestedCaseComplexExpressionIssue1306() throws JSQLParserException {
         // with extra brackets
@@ -4736,7 +4765,7 @@ public void testNestedCaseComplexExpressionIssue1306() throws JSQLParserExceptio
             "END AS \"column1\"\n" +
             "FROM test_schema.table_name\n" +
             "", true);
-        
+
         // without brackets
         assertSqlCanBeParsedAndDeparsed(
             "SELECT CASE\n" +

diff --git a/src/test/java/net/sf/jsqlparser/statement/select/SpecialOracleTest.java b/src/test/java/net/sf/jsqlparser/statement/select/SpecialOracleTest.java
@@ -115,10 +115,12 @@ public class SpecialOracleTest {
             "condition14.sql",
             "condition19.sql",
             "condition20.sql",
+            "connect_by01.sql",
             "connect_by02.sql",
             "connect_by03.sql",
             "connect_by04.sql",
             "connect_by05.sql",
+            "connect_by06.sql",
             "connect_by07.sql",
             "datetime01.sql",
             "datetime02.sql",
@@ -175,6 +177,7 @@ public class SpecialOracleTest {
             "join14.sql",
             "join15.sql",
             "join16.sql",
+            "join17.sql",
             "join18.sql",
             "join19.sql",
             "join20.sql",

diff --git a/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/connect_by01.sql b/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/connect_by01.sql
@@ -27,6 +27,4 @@ from o
 connect by nocycle obj=prior link
 start with obj='a'
 
-
---@FAILURE: Encountered unexpected token: "root" <S_IDENTIFIER> recorded first on Aug 3, 2021, 7:20:08 AM
---@SUCCESSFULLY_PARSED_AND_DEPARSED first on 25.08.2021 23:11:02
+--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM
diff --git a/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/connect_by06.sql b/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/connect_by06.sql
@@ -15,5 +15,4 @@ select last_name "Employee", connect_by_root last_name "Manager",
    order by "Employee", "Manager", "Pathlen", "Path"
 
 
---@FAILURE: Encountered unexpected token: "\"Manager\"" <S_QUOTED_IDENTIFIER> recorded first on Aug 3, 2021, 7:20:08 AM
---@SUCCESSFULLY_PARSED_AND_DEPARSED first on 25.08.2021 23:11:02
+--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM
diff --git a/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/join17.sql b/src/test/resources/net/sf/jsqlparser/statement/select/oracle-tests/join17.sql
@@ -14,5 +14,4 @@ inner join ca c
 on c.id = s.id
 on a.va = s.va
 
-
---@FAILURE: Encountered unexpected token: "on" "ON" recorded first on Aug 3, 2021, 7:20:08 AM
+--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM