diff options
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) { |