Handle more advanced query join constructs. fixes issue #141

git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@1110 f203690c-595d-4dc9-a70b-905162fa7fd2
author: James Ahlborn <jtahlborn@yahoo.com> 2017-07-14 03:46:58 +0000
committer: James Ahlborn <jtahlborn@yahoo.com> 2017-07-14 03:46:58 +0000
commit: 813f732029d22819ab1cb34bd3daa53aeb7d00ee (patch)
tree: 929f533a9dc9d02189348301b3e0c94d01054051
parent: 249d73fd67a9cab110176e96eb298d9b309bb816 (diff)
download: jackcess-813f732029d22819ab1cb34bd3daa53aeb7d00ee.tar.gz
jackcess-813f732029d22819ab1cb34bd3daa53aeb7d00ee.zip
3 files changed, 283 insertions, 97 deletions
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d48db88..53330ef 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -4,6 +4,11 @@
     <author email="javajedi@users.sf.net">Tim McCune</author>
   </properties>
   <body>
+    <release version="2.1.9" date="TBD">
+      <action dev="jahlborn" type="fix" system="SourceForge2" issue="141">
+        Handle more advanced query join constructs.
+      </action>
+    </release>
     <release version="2.1.8" date="2017-06-25">
       <action dev="jahlborn" type="fix" system="SourceForge2" issue="142">
         Fix parsing of certain internal-use queries (such as those used as the
diff --git a/src/main/java/com/healthmarketscience/jackcess/impl/query/QueryImpl.java b/src/main/java/com/healthmarketscience/jackcess/impl/query/QueryImpl.java
index 151fd30..6b51236 100644
--- a/src/main/java/com/healthmarketscience/jackcess/impl/query/QueryImpl.java
+++ b/src/main/java/com/healthmarketscience/jackcess/impl/query/QueryImpl.java
@@ -17,12 +17,9 @@ limitations under the License.
 package com.healthmarketscience.jackcess.impl.query;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 
 import com.healthmarketscience.jackcess.RowId;
 import com.healthmarketscience.jackcess.impl.DatabaseImpl;
@@ -193,7 +190,8 @@ public abstract class QueryImpl implements Query
 
   protected List<String> getFromTables() 
   {
-    List<Join> joinExprs = new ArrayList<Join>();
+    // grab the list of query tables
+    List<TableSource> tableExprs = new ArrayList<TableSource>();
     for(Row table : getTableRows()) {
       StringBuilder builder = new StringBuilder();
 
@@ -206,101 +204,72 @@ public abstract class QueryImpl implements Query
       toAlias(builder, table.name2);
 
       String key = ((table.name2 != null) ? table.name2 : table.name1);
-      joinExprs.add(new Join(key, builder.toString()));
+      tableExprs.add(new SimpleTable(key, builder.toString()));
     }
 
-
+    // combine the tables with any query joins
     List<Row> joins = getJoinRows();
-    if(!joins.isEmpty()) {
-
-      // combine any multi-column joins
-      Collection<List<Row>> comboJoins = combineJoins(joins);
-      
-      for(List<Row> comboJoin : comboJoins) {
-
-        Row join = comboJoin.get(0);
-        String joinExpr = join.expression;
-
-        if(comboJoin.size() > 1) {
-
-          // combine all the join expressions with "AND"
-          AppendableList<String> comboExprs = new AppendableList<String>() {
-            private static final long serialVersionUID = 0L;
-            @Override
-            protected String getSeparator() {
-              return ") AND (";
-            }
-          };
-          for(Row tmpJoin : comboJoin) {
-            comboExprs.add(tmpJoin.expression);
+    for(Row joinRow : joins) {
+        
+      String fromTable = joinRow.name1;
+      String toTable = joinRow.name2;
+
+      TableSource fromTs = null;
+      TableSource toTs = null;
+
+      // combine existing join expressions containing the target tables
+      for(Iterator<TableSource> joinIter = tableExprs.iterator(); 
+          (joinIter.hasNext() && ((fromTs == null) || (toTs == null))); ) {
+        TableSource ts = joinIter.next();
+
+        if((fromTs == null) && ts.containsTable(fromTable)) {
+          fromTs = ts;
+
+          // special case adding expr to existing join
+          if((toTs == null) && ts.containsTable(toTable)) {
+            toTs = ts;
+            break;
           }
 
-          joinExpr = new StringBuilder().append("(")
-            .append(comboExprs).append(")").toString();
-        }
+          joinIter.remove();
 
-        String fromTable = join.name1;
-        String toTable = join.name2;
-      
-        Join fromExpr = getJoinExpr(fromTable, joinExprs);
-        Join toExpr = getJoinExpr(toTable, joinExprs);
-        String joinType = JOIN_TYPE_MAP.get(join.flag);
-        if(joinType == null) {
-          throw new IllegalStateException(withErrorContext(
-                "Unknown join type " + join.flag));
-        }
+        } else if((toTs == null) && ts.containsTable(toTable)) {
 
-        String expr = new StringBuilder().append(fromExpr)
-          .append(joinType).append(toExpr).append(" ON ")
-          .append(joinExpr).toString();
+          toTs = ts;
+          joinIter.remove();
+        }
+      }
 
-        fromExpr.join(toExpr, expr);
-        joinExprs.add(fromExpr);
+      if(fromTs == null) {
+        fromTs = new SimpleTable(fromTable);
+      }
+      if(toTs == null) {
+        toTs = new SimpleTable(toTable);
       }
-    }
 
-    List<String> result = new AppendableList<String>();
-    for(Join joinExpr : joinExprs) {
-      result.add(joinExpr.expression);
-    }
+      if(fromTs == toTs) {
 
-    return result;
-  }
+        if(fromTs.sameJoin(joinRow.flag, joinRow.expression)) {
+          // easy-peasy, we just added the join expression to existing join,
+          // nothing more to do
+          continue;
+        }
 
-  private static Join getJoinExpr(String table, List<Join> joinExprs)
-  {
-    for(Iterator<Join> iter = joinExprs.iterator(); iter.hasNext(); ) {
-      Join joinExpr = iter.next();
-      if(joinExpr.tables.contains(table)) {
-        iter.remove();
-        return joinExpr;
+        throw new IllegalStateException(withErrorContext(
+                "Inconsistent join types for " + fromTable + " and " + toTable));
       }
+
+      // new join expression
+      tableExprs.add(new Join(fromTs, toTs, joinRow.flag, joinRow.expression));
     }
-    // just use the table name as is
-    return new Join(table, toOptionalQuotedExpr(new StringBuilder(), 
-                                                table, true).toString());
-  }
 
-  private Collection<List<Row>> combineJoins(List<Row> joins)
-  {
-    // combine joins with the same to/from tables
-    Map<List<String>,List<Row>> comboJoinMap = 
-      new LinkedHashMap<List<String>,List<Row>>();
-    for(Row join : joins) {
-      List<String> key = Arrays.asList(join.name1, join.name2);
-      List<Row> comboJoins = comboJoinMap.get(key);
-      if(comboJoins == null) {
-        comboJoins = new ArrayList<Row>();
-        comboJoinMap.put(key, comboJoins);
-      } else {
-        if(comboJoins.get(0).flag != (short)join.flag) {
-          throw new IllegalStateException(withErrorContext(
-              "Mismatched join flags for combo joins"));
-        }
-      }
-      comboJoins.add(join);
+    // convert join objects to SQL strings
+    List<String> result = new AppendableList<String>();
+    for(TableSource ts : tableExprs) {
+      result.add(ts.toString());
     }
-    return comboJoinMap.values();
+
+    return result;
   }
 
   protected String getFromRemoteDbPath() 
@@ -729,27 +698,127 @@ public abstract class QueryImpl implements Query
     }
   }
 
-  private static final class Join
+  /**
+   * Base type of something which provides table data in a query
+   */
+  private static abstract class TableSource
   {
-    public final List<String> tables = new ArrayList<String>();
-    public boolean isJoin;
-    public String expression;
+    @Override
+    public String toString() {
+      StringBuilder sb = new StringBuilder();
+      toString(sb, true);
+      return sb.toString();
+    }
+
+    protected abstract void toString(StringBuilder sb, boolean isTopLevel);
+
+    public abstract boolean containsTable(String table);
 
-    private Join(String table, String expr) {
-      tables.add(table);
-      expression = expr;
+    public abstract boolean sameJoin(short type, String on);
+  }
+
+  /**
+   * Table data provided by a single table expression.
+   */
+  private static final class SimpleTable extends TableSource
+  {
+    private final String _tableName;
+    private final String _tableExpr;
+
+    private SimpleTable(String tableName) {
+      this(tableName, toOptionalQuotedExpr(
+               new StringBuilder(), tableName, true).toString());
     }
 
-    public void join(Join other, String newExpr) {
-      tables.addAll(other.tables);
-      isJoin = true;
-      expression = newExpr;
+    private SimpleTable(String tableName, String tableExpr) {
+      _tableName = tableName;
+      _tableExpr = tableExpr;
     }
 
     @Override
-    public String toString() {
-      return (isJoin ? ("(" + expression + ")") : expression);
+    protected void toString(StringBuilder sb, boolean isTopLevel) {
+      sb.append(_tableExpr);
+    }
+
+    @Override
+    public boolean containsTable(String table) {
+      return _tableName.equalsIgnoreCase(table);
+    }
+
+    @Override
+    public boolean sameJoin(short type, String on) {
+      return false;
     }
   }
 
+  /**
+   * Table data provided by a join expression.
+   */
+  private final class Join extends TableSource
+  {
+    private final TableSource _from;
+    private final TableSource _to;
+    private final short _jType;
+    // combine all the join expressions with "AND"
+    private final List<String> _on = new AppendableList<String>() {
+      private static final long serialVersionUID = 0L;
+      @Override
+      protected String getSeparator() {
+        return ") AND (";
+      }
+    };
+
+    private Join(TableSource from, TableSource to, short type, String on) {
+      _from = from;
+      _to = to;
+      _jType = type;
+      _on.add(on);
+    }
+
+    @Override
+    protected void toString(StringBuilder sb, boolean isTopLevel) {
+      String joinType = JOIN_TYPE_MAP.get(_jType);
+      if(joinType == null) {
+        throw new IllegalStateException(withErrorContext(
+                                            "Unknown join type " + _jType));
+      }
+
+      if(!isTopLevel) {
+        sb.append("(");
+      }
+
+      _from.toString(sb, false);
+      sb.append(joinType);
+      _to.toString(sb, false);
+      sb.append(" ON ");
+
+      boolean multiOnExpr = (_on.size() > 1);
+      if(multiOnExpr) {
+        sb.append("(");
+      }
+      sb.append(_on);
+      if(multiOnExpr) {
+        sb.append(")");
+      }
+
+      if(!isTopLevel) {
+        sb.append(")");
+      }
+    }
+
+    @Override
+    public boolean containsTable(String table) {
+      return _from.containsTable(table) || _to.containsTable(table);
+    }
+
+    @Override
+    public boolean sameJoin(short type, String on) {
+      if(_jType == type) {
+        // note, AND conditions are added in _reverse_ order
+        _on.add(0, on);
+        return true;
+      }
+      return false;
+    }
+  }
 }
diff --git a/src/test/java/com/healthmarketscience/jackcess/query/QueryTest.java b/src/test/java/com/healthmarketscience/jackcess/query/QueryTest.java
index 0414b35..f7e48f4 100644
--- a/src/test/java/com/healthmarketscience/jackcess/query/QueryTest.java
+++ b/src/test/java/com/healthmarketscience/jackcess/query/QueryTest.java
@@ -196,7 +196,7 @@ public class QueryTest extends TestCase
       expectedQueries.put(
           "SelectQuery", multiline(
               "SELECT DISTINCT Table1.*, Table2.col1, Table2.col2, Table3.col3",
-              "FROM (Table1 LEFT JOIN Table3 ON Table1.col1 = Table3.col1) INNER JOIN Table2 ON (Table3.col1 = Table2.col1) AND (Table3.col1 = Table2.col2)",
+              "FROM (Table1 LEFT JOIN Table3 ON Table1.col1 = Table3.col1) INNER JOIN Table2 ON (Table3.col1 = Table2.col2) AND (Table3.col1 = Table2.col1)",
               "WHERE (((Table2.col2)=\"foo\" Or (Table2.col2) In (\"buzz\",\"bazz\")))",
               "ORDER BY Table2.col1;"));
       expectedQueries.put(
@@ -464,6 +464,118 @@ public class QueryTest extends TestCase
                  query.toSQLString());    
   }
 
+  public void testComplexJoins() throws Exception
+  {
+    SelectQuery query = (SelectQuery)newQuery(
+        Query.Type.SELECT, new Row[0]);
+    setFlag(query, 1);
+
+    for(int i = 1; i <= 10; ++i) {
+      addRows(query, newRow(TABLE_ATTRIBUTE, null, "Table" + i, null));
+    }
+
+    addJoinRows(query, 1, 2, 1,
+                2, 3, 1,
+                3, 4, 1);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table5, Table6, Table7, Table8, Table9, Table10, ((Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN Table3 ON Table2.f3 = Table3.f3) INNER JOIN Table4 ON Table3.f6 = Table4.f6;"),
+                 query.toSQLString());
+
+    addJoinRows(query, 1, 2, 1,
+                2, 1, 1);
+    
+    assertEquals(multiline("SELECT *",
+                           "FROM Table3, Table4, Table5, Table6, Table7, Table8, Table9, Table10, Table1 INNER JOIN Table2 ON (Table2.f3 = Table1.f3) AND (Table1.f0 = Table2.f0);"),
+                 query.toSQLString());
+
+    addJoinRows(query, 1, 2, 1,
+                2, 1, 2);
+
+    try {
+      query.toSQLString();
+      fail("IllegalStateException should have been thrown");
+    } catch(IllegalStateException e) {
+      // success
+    }
+    
+    addJoinRows(query, 1, 2, 1,
+                3, 4, 1,
+                5, 6, 1,
+                7, 8, 1,
+                2, 3, 1);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table9, Table10, Table5 INNER JOIN Table6 ON Table5.f6 = Table6.f6, Table7 INNER JOIN Table8 ON Table7.f9 = Table8.f9, (Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN (Table3 INNER JOIN Table4 ON Table3.f3 = Table4.f3) ON Table2.f12 = Table3.f12;"),
+                 query.toSQLString());
+
+    addJoinRows(query, 1, 2, 1,
+                3, 4, 1,
+                5, 6, 1,
+                7, 8, 1,
+                2, 3, 1,
+                5, 8, 1);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table9, Table10, (Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN (Table3 INNER JOIN Table4 ON Table3.f3 = Table4.f3) ON Table2.f12 = Table3.f12, (Table5 INNER JOIN Table6 ON Table5.f6 = Table6.f6) INNER JOIN (Table7 INNER JOIN Table8 ON Table7.f9 = Table8.f9) ON Table5.f15 = Table8.f15;"),
+                 query.toSQLString());
+
+    addJoinRows(query, 1, 2, 1,
+                1, 3, 1,
+                6, 3, 1,
+                1, 9, 2,
+                10, 9, 3,
+                7, 8, 3,
+                1, 8, 2,
+                1, 4, 2,
+                5, 4, 3);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table5 RIGHT JOIN (((Table10 RIGHT JOIN ((Table6 INNER JOIN ((Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN Table3 ON Table1.f3 = Table3.f3) ON Table6.f6 = Table3.f6) LEFT JOIN Table9 ON Table1.f9 = Table9.f9) ON Table10.f12 = Table9.f12) LEFT JOIN (Table7 RIGHT JOIN Table8 ON Table7.f15 = Table8.f15) ON Table1.f18 = Table8.f18) LEFT JOIN Table4 ON Table1.f21 = Table4.f21) ON Table5.f24 = Table4.f24;"),
+                 query.toSQLString());
+
+    addJoinRows(query, 1, 2, 1,
+                1, 3, 1,
+                1, 9, 2,
+                1, 8, 2,
+                1, 4, 2,
+                6, 3, 1,
+                5, 4, 3,
+                7, 8, 3,
+                10, 9, 3);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table10 RIGHT JOIN (Table7 RIGHT JOIN (Table5 RIGHT JOIN (Table6 INNER JOIN (((((Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN Table3 ON Table1.f3 = Table3.f3) LEFT JOIN Table9 ON Table1.f6 = Table9.f6) LEFT JOIN Table8 ON Table1.f9 = Table8.f9) LEFT JOIN Table4 ON Table1.f12 = Table4.f12) ON Table6.f15 = Table3.f15) ON Table5.f18 = Table4.f18) ON Table7.f21 = Table8.f21) ON Table10.f24 = Table9.f24;"),
+                 query.toSQLString());
+
+
+    removeRows(query, TABLE_ATTRIBUTE);
+
+    addJoinRows(query, 1, 2, 1,
+                2, 3, 1,
+                2, 4, 1,
+                1, 4, 1,
+                5, 3, 1);
+
+    assertEquals(multiline("SELECT *",
+                           "FROM Table5 INNER JOIN (((Table1 INNER JOIN Table2 ON Table1.f0 = Table2.f0) INNER JOIN Table3 ON Table2.f3 = Table3.f3) INNER JOIN Table4 ON (Table1.f9 = Table4.f9) AND (Table2.f6 = Table4.f6)) ON Table5.f12 = Table3.f12;"),
+                 query.toSQLString());
+  }
+
+  private static void addJoinRows(SelectQuery query, int... joins)
+  {
+    removeRows(query, JOIN_ATTRIBUTE);
+    for(int i = 0; i < joins.length; i += 3) {
+      int from = joins[i + 0];
+      int to = joins[i + 1];
+      int type = joins[i + 2];
+
+      String tf = "Table" + from;
+      String tt = "Table" + to;
+      String joinExpr = tf + ".f" + i + " = " + tt + ".f" + i;
+      addRows(query, newRow(JOIN_ATTRIBUTE, joinExpr, type, tf, tt));
+    }
+  }
 
   private static Query newQuery(Query.Type type, Row... rows)
   {
author	James Ahlborn <jtahlborn@yahoo.com>	2017-07-14 03:46:58 +0000
committer	James Ahlborn <jtahlborn@yahoo.com>	2017-07-14 03:46:58 +0000
commit	813f732029d22819ab1cb34bd3daa53aeb7d00ee (patch)
tree	929f533a9dc9d02189348301b3e0c94d01054051
parent	249d73fd67a9cab110176e96eb298d9b309bb816 (diff)
download	jackcess-813f732029d22819ab1cb34bd3daa53aeb7d00ee.tar.gz jackcess-813f732029d22819ab1cb34bd3daa53aeb7d00ee.zip