From d18c59bf845c57d186c0eac796ce7f543f061c76 Mon Sep 17 00:00:00 2001 From: manticore-projects Date: Mon, 6 Sep 2021 18:34:05 +0700 Subject: [PATCH] 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 --- .../sf/jsqlparser/statement/select/Join.java | 86 +++++++++++-------- .../net/sf/jsqlparser/util/SelectUtils.java | 2 +- .../util/deparser/SelectDeParser.java | 13 +-- .../validation/validator/SelectValidator.java | 6 +- .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 28 +++--- .../alter/RenameTableStatementTest.java | 2 +- .../statement/select/SelectTest.java | 33 ++++++- .../statement/select/SpecialOracleTest.java | 3 + .../select/oracle-tests/connect_by01.sql | 4 +- .../select/oracle-tests/connect_by06.sql | 3 +- .../statement/select/oracle-tests/join17.sql | 3 +- 11 files changed, 115 insertions(+), 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 index df755b3cc..4c6d74ffc 100644 --- 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 usingColumns; + private final LinkedList onExpressions = new LinkedList<>(); + private final LinkedList 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 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 expressions) { + onExpressions.clear(); + onExpressions.addAll(expressions); + return this; } /** @@ -254,7 +269,8 @@ public Join withUsingColumns(List list) { } public void setUsingColumns(List 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 usingColumns) { collection.addAll(usingColumns); return this.withUsingColumns(collection); } - - public E getRightItem(Class type) { - return type.cast(getRightItem()); - } - - public E getOnExpression(Class 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 index 5e1efc6d1..f370b8dda 100644 --- 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 index e54a1f2b4..0d4ef1588 100644 --- 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 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 index 807b9df08..a46c61697 100644 --- 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 index 918ac7d2a..45d502ce8 100644 --- 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= | tk= | tk= | tk= | tk= | tk= | tk= - | tk= | tk= | tk= ) + | tk= | tk= | tk= ) { if (tk!=null) result=tk.image; @@ -1653,7 +1653,7 @@ String RelObjectNameWithoutStart() : { Token tk = null; String result = null; } { (result = RelObjectNameWithoutValue() | tk= | tk= | tk= - | tk=) + | tk= ) { if (tk!=null) result=tk.image; @@ -1672,7 +1672,7 @@ String RelObjectNameExt(): ( result=RelObjectName() | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= | tk= - | tk= | tk= | tk= ) + | tk= | tk= | tk= ) { if (tk!=null) result=tk.image; return result; @@ -2483,13 +2483,19 @@ Join JoinerExpression() #JoinerExpression: [ - LOOKAHEAD(2) ([ "(" joinWindow = JoinWindow() ")" {join.setJoinWindow(joinWindow);}] - ( onExpression=Expression() { join.setOnExpression(onExpression); }) - | - ( "(" tableColumn=Column() { columns = new ArrayList(); columns.add(tableColumn); } - ("," tableColumn=Column() { columns.add(tableColumn); } )* ")" - { join.setUsingColumns(columns); } )) - ] + LOOKAHEAD(2) ( + [ "(" joinWindow = JoinWindow() ")" {join.setJoinWindow(joinWindow);} ] + ( onExpression=Expression() { join.addOnExpression(onExpression); } + ( LOOKAHEAD(2) onExpression=Expression() { join.addOnExpression(onExpression); } )* + ) + | + ( + "(" tableColumn=Column() { columns = new ArrayList(); 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 index 6287bc203..8e35a1a62 100644 --- 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 index e5c5bc581..89af6acc2 100644 --- 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 index c62f92ff6..31d27d3b5 100644 --- 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 index 2623519fb..1bded99ba 100644 --- 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" recorded first on Aug 3, 2021, 7:20:08 AM ---@SUCCESSFULLY_PARSED_AND_DEPARSED first on 25.08.2021 23:11:02 \ No newline at end of file +--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM \ No newline at end of file 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 index 5f97a9dac..2ea7a0c8e 100644 --- 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\"" recorded first on Aug 3, 2021, 7:20:08 AM ---@SUCCESSFULLY_PARSED_AND_DEPARSED first on 25.08.2021 23:11:02 \ No newline at end of file +--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM \ No newline at end of file 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 index 0cc456113..986256e00 100644 --- 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 \ No newline at end of file +--@SUCCESSFULLY_PARSED_AND_DEPARSED first on Aug 14, 2021 9:00:57 PM \ No newline at end of file