OPENJPA-547 INNER JOIN FETCH query incorrectly generates LEFT join SQL

git-svn-id: https://svn.apache.org/repos/asf/openjpa/branches/1.0.x@646455 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/DelegatingJDBCFetchConfiguration.java

@@ -260,4 +260,38 @@ public class DelegatingJDBCFetchConfiguration
             throw translate(re);
         }
     }
+
+    public Set getFetchInnerJoins() {
+        try {
+            return getJDBCDelegate().getFetchInnerJoins();
+        } catch (RuntimeException re) {
+            throw translate(re);
+        }
+    }
+
+    public boolean hasFetchInnerJoin(String field) {
+        try {
+            return getJDBCDelegate().hasFetchInnerJoin(field);
+        } catch (RuntimeException re) {
+            throw translate(re);
+        }
+    }
+
+    public JDBCFetchConfiguration addFetchInnerJoin(String field) {
+        try {
+            getJDBCDelegate().addFetchInnerJoin(field);
+            return this;
+        } catch (RuntimeException re) {
+            throw translate(re);
+        }
+    }
+
+    public JDBCFetchConfiguration addFetchInnerJoins(Collection fields) {
+        try {
+            getJDBCDelegate().addFetchInnerJoins(fields);
+            return this;
+        } catch (RuntimeException re) {
+            throw translate(re);
+        }
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfiguration.java

@@ -207,4 +207,38 @@ public interface JDBCFetchConfiguration
      * Convenience method to cast traversal to store-specific type.
      */
     public JDBCFetchConfiguration traverseJDBC(FieldMetaData fm);
+
+    /**
+     * Returns the names of the inner fetch joins that this component will use
+     * when loading objects. Defaults to the empty set.  This set is not
+     * thread safe.
+     *
+     * @since 1.1
+     */
+    public Set getFetchInnerJoins();
+
+    /**
+     * Return true if the given fully-qualified inner fetch join has been added.
+     *
+     * @since 1.1
+     */
+    public boolean hasFetchInnerJoin(String field);
+
+    /**
+     * Adds <code>field</code> to the set of fully-qualified field names to
+     * eagerly join when loading objects. Each class can have at most
+     * one to-many eagerly joined fields.
+     *
+     * @since 1.1
+     */
+    public JDBCFetchConfiguration addFetchInnerJoin(String field);
+
+    /**
+     * Adds <code>fields</code> to the set of fully-qualified field names to
+     * eagerly join when loading objects. Each class can have at most
+     * one to-many eagerly joined fields.
+     *
+     * @since 1.1
+     */
+    public JDBCFetchConfiguration addFetchInnerJoins(Collection fields);
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCFetchConfigurationImpl.java

@@ -69,6 +69,7 @@ public class JDBCFetchConfigurationImpl
         public int size = 0;
         public int syntax = 0;
         public Set joins = null;
+        public Set fetchInnerJoins = null;
         public int isolationLevel = -1;
     }
 
@@ -345,4 +346,37 @@ public class JDBCFetchConfigurationImpl
             return null;
         return (JDBCConfiguration) conf;
     }
+
+    public Set getFetchInnerJoins() {
+        return (_state.fetchInnerJoins == null) ? Collections.EMPTY_SET
+            : _state.fetchInnerJoins;
+    }
+
+    public boolean hasFetchInnerJoin(String field) {
+        return _state.fetchInnerJoins != null &&
+            _state.fetchInnerJoins.contains(field);
+    }
+
+    public JDBCFetchConfiguration addFetchInnerJoin(String join) {
+        if (StringUtils.isEmpty(join))
+            throw new UserException(_loc.get("null-join"));
+        
+        lock();
+        try {
+            if (_state.fetchInnerJoins == null)
+                _state.fetchInnerJoins = new HashSet();
+            _state.fetchInnerJoins.add(join);
+        } finally {
+            unlock();
+        }
+        return this;
+    }
+
+    public JDBCFetchConfiguration addFetchInnerJoins(Collection joins) {
+        if (joins == null || joins.isEmpty())
+            return this;
+        for (Iterator itr = joins.iterator(); itr.hasNext();)
+            addFetchInnerJoin((String) itr.next());
+        return this;
+    }
 }

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCStoreQuery.java

@@ -150,6 +150,8 @@ public class JDBCStoreQuery
             fetch.addFields(Arrays.asList(exps[0].fetchPaths));
             fetch.addJoins(Arrays.asList(exps[0].fetchPaths));
         }
+        if (exps[0].fetchInnerPaths != null)
+            fetch.addFetchInnerJoins(Arrays.asList(exps[0].fetchInnerPaths));
 
         int eager = calculateEagerMode(exps[0], range.start, range.end);
         int subclassMode = fetch.getSubclassFetchMode((ClassMapping) base);
@@ -600,6 +602,8 @@ public class JDBCStoreQuery
             fetch.addFields(Arrays.asList(exps[0].fetchPaths));
             fetch.addJoins(Arrays.asList(exps[0].fetchPaths));
         }
+        if (exps[0].fetchInnerPaths != null)
+            fetch.addFetchInnerJoins(Arrays.asList(exps[0].fetchInnerPaths));
 
         int eager = calculateEagerMode(exps[0], range.start, range.end);
         eager = Math.min(eager, JDBCFetchConfiguration.EAGER_JOIN);

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/RelationFieldStrategy.java

@@ -409,9 +409,11 @@ public class RelationFieldStrategy
         // clone it for a to-many eager select can result in a clone that
         // produces invalid SQL
         ClassMapping cls = field.getIndependentTypeMappings()[0];
+        boolean forceInner = fetch.hasFetchInnerJoin(field.getFullName(false)) ?
+                true : false;
         sel.select(cls, field.getSelectSubclasses(), store, fetch,
             JDBCFetchConfiguration.EAGER_JOIN,
-            eagerJoin(sel.newJoins(), cls, false));
+            eagerJoin(sel.newJoins(), cls, forceInner));
     }
 
     /**

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/StoreCollectionFieldStrategy.java

@@ -24,7 +24,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.apache.openjpa.conf.OpenJPAConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration;
 import org.apache.openjpa.jdbc.kernel.JDBCStore;
 import org.apache.openjpa.jdbc.meta.ClassMapping;
@@ -168,10 +167,13 @@ public abstract class StoreCollectionFieldStrategy
         // we limit further eager fetches to joins, because after this point
         // the select has been modified such that parallel clones may produce
         // invalid sql
+        boolean outer = field.getNullValue() != FieldMapping.NULL_EXCEPTION;
+        // force inner join for inner join fetch 
+        if (fetch.hasFetchInnerJoin(field.getFullName(false)))
+            outer = false;
         selectEager(sel, getDefaultElementMapping(true), sm, store, fetch, 
             JDBCFetchConfiguration.EAGER_JOIN, false,
-            field.getNullValue()
-                != FieldMapping.NULL_EXCEPTION);
+            outer);
     }
 
     public boolean isEagerSelectToMany() {

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/QueryExpressions.java

@@ -63,6 +63,7 @@ public class QueryExpressions {
     public int operation = QueryOperations.OP_SELECT;
     public ClassMetaData[] accessPath = StoreQuery.EMPTY_METAS;
     public String[] fetchPaths = StoreQuery.EMPTY_STRINGS;
+    public String[] fetchInnerPaths = StoreQuery.EMPTY_STRINGS;
     public Value[] range = EMPTY_VALUES;
     private Boolean _aggregate = null;
 

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java

@@ -440,6 +440,7 @@ public class JPQLExpressionBuilder
 
         // handle JOIN FETCH
         Set joins = null;
+        Set innerJoins = null;
 
         JPQLNode[] outers = root().findChildrenByID(JJTOUTERFETCHJOIN);
         for (int i = 0; outers != null && i < outers.length; i++)
@@ -447,13 +448,20 @@ public class JPQLExpressionBuilder
                 add(getPath(onlyChild(outers[i])).last().getFullName(false));
 
         JPQLNode[] inners = root().findChildrenByID(JJTINNERFETCHJOIN);
-        for (int i = 0; inners != null && i < inners.length; i++)
-            (joins == null ? joins = new TreeSet() : joins).
-                add(getPath(onlyChild(inners[i])).last().getFullName(false));
+        for (int i = 0; inners != null && i < inners.length; i++) {
+            String path = getPath(onlyChild(inners[i])).last()
+                .getFullName(false);
+            (joins == null ? joins = new TreeSet() : joins).add(path);
+            (innerJoins == null ? innerJoins = new TreeSet() : innerJoins).
+                add(path);
+        }
 
         if (joins != null)
             exps.fetchPaths = (String[]) joins.
                 toArray(new String[joins.size()]);
+        if (innerJoins != null)
+            exps.fetchInnerPaths = (String[]) innerJoins.
+                toArray(new String[innerJoins.size()]);
 
         return filter;
     }
@@ -494,7 +502,7 @@ public class JPQLExpressionBuilder
             else if (node.id == JJTINNERJOIN)
                 exp = addJoin(node, true, exp);
             else if (node.id == JJTINNERFETCHJOIN)
-                exp = addJoin(node, true, exp);
+                ; // we handle inner fetch joins in the evalFetchJoins() method
             else if (node.id == JJTOUTERFETCHJOIN)
                 ; // we handle outer fetch joins in the evalFetchJoins() method
             else