[Blazebit#812] Fix missing joins in final queries due to wrong cardin…

…ality mandatory computation

core/impl/src/main/java/com/blazebit/persistence/impl/ImplicitGroupByClauseDependencyRegistrationVisitor.java

@@ -39,7 +39,7 @@ public void visit(PathExpression expr) {
             // This can only be a select alias
             ((SelectInfo) aliasManager.getAliasInfo(expr.toString())).getExpression().accept(this);
         } else {
-            node.updateClauseDependencies(ClauseType.GROUP_BY);
+            node.updateClauseDependencies(ClauseType.GROUP_BY, null);
         }
     }
 

core/impl/src/main/java/com/blazebit/persistence/impl/JoinManager.java

@@ -2078,7 +2078,7 @@ public void implicitJoin(Expression expression, boolean joinAllowed, boolean obj
             // Don't forget to update the clause dependencies, but only for normal attribute accesses, that way paginated queries can prevent joins in certain cases
             if (fromClause != null) {
                 try {
-                    result.baseNode.updateClauseDependencies(fromClause, false, new LinkedHashSet<JoinNode>());
+                    result.baseNode.updateClauseDependencies(fromClause, new LinkedHashSet<JoinNode>());
                 } catch (IllegalStateException ex) {
                     throw new IllegalArgumentException("Implicit join in expression '" + expression + "' introduces cyclic join dependency!", ex);
                 }
@@ -2439,7 +2439,7 @@ public void visit(PathExpression pathExpr) {
             }
 
         });
-        joinNode.updateClauseDependencies(ClauseType.JOIN, false, new LinkedHashSet<JoinNode>());
+        joinNode.updateClauseDependencies(ClauseType.JOIN, new LinkedHashSet<JoinNode>());
     }
 
     private void generateAndApplyOnPredicate(JoinNode joinNode, ArrayExpression arrayExpr) {
@@ -2990,7 +2990,7 @@ public void visit(PathExpression expression) {
 
             });
             joinNode.setOnPredicate((CompoundPredicate) predicate);
-            joinNode.updateClauseDependencies(ClauseType.JOIN, false, new LinkedHashSet<JoinNode>());
+            joinNode.updateClauseDependencies(ClauseType.JOIN, new LinkedHashSet<JoinNode>());
         }
     }
 }

core/impl/src/main/java/com/blazebit/persistence/impl/JoinNode.java

@@ -449,23 +449,12 @@ public EnumSet<ClauseType> getClauseDependencies() {
         return clauseDependencies;
     }
 
-    public void updateClauseDependencies(ClauseType clauseDependency) {
-        // update the ON clause dependent nodes to also have a clause dependency
-        for (JoinNode dependency : dependencies) {
-            dependency.updateClauseDependencies(clauseDependency);
-        }
-
-        clauseDependencies.add(clauseDependency);
-
-        // If the parent node was a dependency, we are done with cycle checking
-        // as it has been checked by the recursive call before
-        if (parent != null && !dependencies.contains(parent)) {
-            parent.updateClauseDependencies(clauseDependency);
-        }
+    public boolean updateClauseDependencies(ClauseType clauseDependency, Set<JoinNode> seenNodes) {
+        return updateClauseDependencies(clauseDependency, false, false, seenNodes);
     }
 
-    public boolean updateClauseDependencies(ClauseType clauseDependency, boolean forceAdd, Set<JoinNode> seenNodes) {
-        if (!seenNodes.add(this)) {
+    private boolean updateClauseDependencies(ClauseType clauseDependency, boolean forceAdd, boolean forceAddAll, Set<JoinNode> seenNodes) {
+        if (seenNodes != null && !seenNodes.add(this)) {
             StringBuilder errorSb = new StringBuilder();
             errorSb.append("Cyclic join dependency between nodes: ");
             for (JoinNode seenNode : seenNodes) {
@@ -480,15 +469,19 @@ public boolean updateClauseDependencies(ClauseType clauseDependency, boolean for
             throw new IllegalStateException(errorSb.toString());
         }
 
+        if (clauseDependencies.contains(clauseDependency)) {
+            return true;
+        }
+
         // By default, we add all clause dependency, but we try to reduce JOIN clause dependencies
         boolean add;
 
-        if (clauseDependency != ClauseType.JOIN) {
+        if (clauseDependency != ClauseType.JOIN || forceAddAll) {
             add = true;
         } else {
             // We don't need a JOIN clause dependencies on unrestricted non-optional or outer joins
             if (joinType == JoinType.INNER) {
-                add = parentTreeNode == null || parentTreeNode.isOptional() || !isEmptyCondition();
+                add = forceAddAll = parentTreeNode == null || parentTreeNode.isOptional() || !isEmptyCondition();
             } else {
                 // We never need left joins to retain the parent's cardinality
                 add = false;
@@ -497,21 +490,23 @@ public boolean updateClauseDependencies(ClauseType clauseDependency, boolean for
 
         // update the ON clause dependent nodes to also have a clause dependency
         for (JoinNode dependency : dependencies) {
-            dependency.updateClauseDependencies(clauseDependency, add, seenNodes);
+            dependency.updateClauseDependencies(clauseDependency, add, forceAddAll, seenNodes);
         }
 
         // If the parent node was a dependency, we are done with cycle checking
         // as it has been checked by the recursive call before
         if (parent != null && !dependencies.contains(parent)) {
-            add = parent.updateClauseDependencies(clauseDependency, add, seenNodes);
+            add = parent.updateClauseDependencies(clauseDependency, add, forceAddAll, seenNodes);
         }
 
         add = add || forceAdd;
         if (add) {
             clauseDependencies.add(clauseDependency);
         }
 
-        seenNodes.remove(this);
+        if (seenNodes != null) {
+            seenNodes.remove(this);
+        }
         return add;
     }
 

core/testsuite/src/test/java/com/blazebit/persistence/testsuite/JoinOnTest.java

@@ -18,7 +18,13 @@
 
 import static org.junit.Assert.assertEquals;
 
+import com.blazebit.persistence.PaginatedCriteriaBuilder;
+import com.blazebit.persistence.testsuite.base.jpa.category.NoDatanucleus4;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoEclipselink;
+import com.blazebit.persistence.testsuite.base.jpa.category.NoHibernate42;
+import com.blazebit.persistence.testsuite.base.jpa.category.NoHibernate43;
+import com.blazebit.persistence.testsuite.base.jpa.category.NoHibernate50;
+import com.blazebit.persistence.testsuite.base.jpa.category.NoOpenJPA;
 import org.junit.Test;
 
 import com.blazebit.persistence.CriteriaBuilder;
@@ -128,4 +134,27 @@ public void testLeftJoinOnSubquery() {
                         + onClause(joinAliasValue("l") + " IN (SELECT p.name FROM Person p)"), crit.getQueryString());
         crit.getResultList();
     }
+
+    @Test
+    @Category({ NoHibernate42.class, NoHibernate43.class, NoHibernate50.class, NoDatanucleus4.class, NoOpenJPA.class})
+    public void testJoinAvoidance() {
+        CriteriaBuilder<String> crit = cbf.create(em, String.class).from(Document.class, "e");
+        crit.select("e.responsiblePerson.name");
+        crit.orderByAsc("e.id");
+
+        CriteriaBuilder<String> criteriaBuilder = crit.copy(String.class);
+        criteriaBuilder.innerJoinOn("e.responsiblePerson.friend", Document.class, "d2")
+                .on("e.responsiblePerson.friend.name").eqExpression("d2.name")
+                .end();
+        criteriaBuilder.select("d2.name");
+
+        String expectedObjectQuery = "SELECT " + joinAliasValue("d2", "name") + " FROM Document e "
+                + "LEFT JOIN e.responsiblePerson responsiblePerson_1 "
+                + "LEFT JOIN responsiblePerson_1.friend friend_1 "
+                + "JOIN Document d2" + onClause("friend_1.name = d2.name")
+                + " ORDER BY e.id ASC";
+
+        assertEquals(expectedObjectQuery, criteriaBuilder.getQueryString());
+        criteriaBuilder.getResultList();
+    }
 }