QI2: Eliminating redundant joins for singletons.

repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreter2Test.java

@@ -118,6 +118,7 @@
 import static com.evolveum.midpoint.xml.ns._public.common.common_3.AccessCertificationDecisionType.F_RESPONSE;
 import static com.evolveum.midpoint.xml.ns._public.common.common_3.AccessCertificationDecisionType.F_STAGE_NUMBER;
 import static com.evolveum.midpoint.xml.ns._public.common.common_3.AccessCertificationResponseType.NO_RESPONSE;
+import static com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType.F_EXTENSION;
 import static com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType.F_NAME;
 
 /**
@@ -566,6 +567,40 @@ public void test070QueryGenericLong() throws Exception {
         }
     }
 
+    @Test
+    public void test071QueryGenericLongTwice() throws Exception {
+        Session session = open();
+        try {
+            ObjectQuery query = QueryBuilder.queryFor(GenericObjectType.class, prismContext)
+                    .item(F_NAME).eqPoly("generic object", "generic object").matchingNorm()
+                    .and().item(F_EXTENSION, new QName("intType")).ge(100)
+                    .and().item(F_EXTENSION, new QName("intType")).lt(200)
+                    .and().item(F_EXTENSION, new QName("longType")).eq(335)
+                    .build();
+
+            String real = getInterpretedQuery2(session, GenericObjectType.class, query);
+
+            String expected = "select\n" +
+                    "  g.fullObject, g.stringsCount, g.longsCount, g.datesCount, g.referencesCount, g.polysCount, g.booleansCount\n" +
+                    "from\n" +
+                    "  RGenericObject g\n" +
+                    "    left join g.longs l with ( l.ownerType = :ownerType and l.name = :name )\n" +
+                    "    left join g.longs l2 with ( l2.ownerType = :ownerType2 and l2.name = :name2 )\n" +
+                    "where\n" +
+                    "  (\n" +
+                    "    g.name.norm = :norm and\n" +
+                    "    l.value >= :value and\n" +
+                    "    l.value < :value2 and\n" +
+                    "    l2.value = :value3\n" +
+                    "  )";
+
+            // note l and l2 cannot be merged as they point to different extension properties (intType, longType)
+            assertEqualsIgnoreWhitespace(expected, real);
+        } finally {
+            close(session);
+        }
+    }
+
     @Test
     public void test072QueryAccountByAttribute() throws Exception {
         Session session = open();
@@ -2583,16 +2618,12 @@ public void test747QueryCertCaseReviewerAndEnabledByRequestedDesc() throws Excep
                     .build();
             String real = getInterpretedQuery2(session, AccessCertificationCaseType.class, query, false);
 
-            // TODO TODO TODO
-            // when referencing singleton more times (like a.owner here) we should do only 1 join
-
             String expected = "select\n" +
                     "  a.fullObject\n" +
                     "from\n" +
                     "  RAccessCertificationCase a\n" +
                     "    left join a.reviewerRef r\n" +
                     "    left join a.owner o\n" +
-                    "    left join a.owner o2\n" +
                     "where\n" +
                     "  (\n" +
                     "    (\n" +
@@ -2607,7 +2638,7 @@ public void test747QueryCertCaseReviewerAndEnabledByRequestedDesc() throws Excep
                     "        o.stageNumber is null\n" +
                     "      )\n" +
                     "    ) and\n" +
-                    "    o2.state = :state\n" +
+                    "    o.state = :state\n" +
                     "  )\n" +
                     "order by a.reviewRequestedTimestamp desc";
             assertEqualsIgnoreWhitespace(expected, real);
@@ -3037,7 +3068,6 @@ public void test925DecisionsNotAnsweredOrderBy() throws Exception {
                     "  RAccessCertificationCase a\n" +
                     "    left join a.decision d\n" +
                     "    left join a.owner o\n" +
-                    "    left join a.owner o2\n" +
                     "where\n" +
                     "  (\n" +
                     "    (\n" +
@@ -3056,7 +3086,7 @@ public void test925DecisionsNotAnsweredOrderBy() throws Exception {
                     "      d.response = :response\n" +
                     "    )\n" +
                     "  )\n" +
-                    "order by o.name.orig asc, a.id asc, o2.oid asc\n";
+                    "order by o.name.orig asc, a.id asc, o.oid asc\n";
             assertEqualsIgnoreWhitespace(expected, real);
         } finally {
             close(session);

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/EntityReference.java

@@ -95,4 +95,13 @@ public boolean containsAlias(String alias) {
         }
         return false;
     }
+
+    public JoinSpecification findJoinFor(String path) {
+        for (JoinSpecification join : joins) {
+            if (path.equals(join.getPath())) {
+                return join;
+            }
+        }
+        return null;
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/AndCondition.java

@@ -37,4 +37,6 @@ public AndCondition(RootHibernateQuery rootHibernateQuery, Collection<Condition>
     public void dumpToHql(StringBuilder sb, int indent) {
         super.dumpToHql(sb, indent, "and");
     }
+
+    // inherited "equals" is OK
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/InCondition.java

@@ -55,4 +55,25 @@ public void dumpToHql(StringBuilder sb, int indent) {
             sb.append(propertyPath).append(" in (").append(innerQueryText).append(")");
         }
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+
+        InCondition that = (InCondition) o;
+
+        if (values != null ? !values.equals(that.values) : that.values != null) return false;
+        return !(innerQueryText != null ? !innerQueryText.equals(that.innerQueryText) : that.innerQueryText != null);
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = super.hashCode();
+        result = 31 * result + (values != null ? values.hashCode() : 0);
+        result = 31 * result + (innerQueryText != null ? innerQueryText.hashCode() : 0);
+        return result;
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/IsNotNullCondition.java

@@ -33,4 +33,6 @@ public void dumpToHql(StringBuilder sb, int indent) {
         HibernateQuery.indent(sb, indent);
         sb.append(propertyPath).append(" is not null");
     }
+
+    // inherited "equals" is OK
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/IsNullCondition.java

@@ -33,4 +33,6 @@ public void dumpToHql(StringBuilder sb, int indent) {
         HibernateQuery.indent(sb, indent);
         sb.append(propertyPath).append(" is null");
     }
+
+    // inherited "equals" is OK
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/JunctionCondition.java

@@ -69,4 +69,20 @@ public void dumpToHql(StringBuilder sb, int indent, String logicalOperation) {
             sb.append(")");
         }
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        JunctionCondition that = (JunctionCondition) o;
+
+        return components.equals(that.components);
+
+    }
+
+    @Override
+    public int hashCode() {
+        return components.hashCode();
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/NotCondition.java

@@ -39,4 +39,20 @@ public void dumpToHql(StringBuilder sb, int indent) {
         sb.append("not ");
         child.dumpToHql(sb, -1);
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        NotCondition that = (NotCondition) o;
+
+        return child.equals(that.child);
+
+    }
+
+    @Override
+    public int hashCode() {
+        return child.hashCode();
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/OrCondition.java

@@ -32,4 +32,5 @@ public void dumpToHql(StringBuilder sb, int indent) {
         super.dumpToHql(sb, indent, "or");
     }
 
+    // inherited "equals" is OK
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/PropertyCondition.java

@@ -50,4 +50,20 @@ protected String createParameterName(String propertyPath) {
             }
         }
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        PropertyCondition that = (PropertyCondition) o;
+
+        return propertyPath.equals(that.propertyPath);
+
+    }
+
+    @Override
+    public int hashCode() {
+        return propertyPath.hashCode();
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/PropertyPropertyComparisonCondition.java

@@ -54,4 +54,27 @@ public void dumpToHql(StringBuilder sb, int indent) {
 
         sb.append(finalPropertyPath).append(" ").append(operator).append(" ").append(finalRightSidePropertyPath);
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+
+        PropertyPropertyComparisonCondition that = (PropertyPropertyComparisonCondition) o;
+
+        if (ignoreCase != that.ignoreCase) return false;
+        if (!rightSidePath.equals(that.rightSidePath)) return false;
+        return operator.equals(that.operator);
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = super.hashCode();
+        result = 31 * result + rightSidePath.hashCode();
+        result = 31 * result + operator.hashCode();
+        result = 31 * result + (ignoreCase ? 1 : 0);
+        return result;
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/hqm/condition/SimpleComparisonCondition.java

@@ -60,4 +60,27 @@ public void dumpToHql(StringBuilder sb, int indent) {
         String parameterName = rootHibernateQuery.addParameter(parameterNamePrefix, finalPropertyValue);
         sb.append(finalPropertyPath).append(" ").append(operator).append(" :").append(parameterName);
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+
+        SimpleComparisonCondition that = (SimpleComparisonCondition) o;
+
+        if (ignoreCase != that.ignoreCase) return false;
+        if (value != null ? !value.equals(that.value) : that.value != null) return false;
+        return operator.equals(that.operator);
+
+    }
+
+    @Override
+    public int hashCode() {
+        int result = super.hashCode();
+        result = 31 * result + (value != null ? value.hashCode() : 0);
+        result = 31 * result + operator.hashCode();
+        result = 31 * result + (ignoreCase ? 1 : 0);
+        return result;
+    }
 }

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/resolution/ItemPathResolver.java

@@ -35,6 +35,7 @@
 import com.evolveum.midpoint.repo.sql.util.RUtil;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
+import org.apache.commons.lang.ObjectUtils;
 
 import javax.xml.namespace.QName;
 import java.lang.reflect.InvocationTargetException;
@@ -90,7 +91,44 @@ String addJoin(JpaLinkDefinition joinedItemDefinition, String currentHqlPath) th
         RootHibernateQuery hibernateQuery = context.getHibernateQuery();
         String joinedItemJpaName = joinedItemDefinition.getJpaName();
         String joinedItemFullPath = currentHqlPath + "." + joinedItemJpaName;
-        String joinedItemAlias = hibernateQuery.createAlias(joinedItemDefinition);
+        String joinedItemAlias;
+        if (!joinedItemDefinition.isMultivalued()) {
+            /*
+             * Let's check if we were already here i.e. if we had already created this join.
+             * This is to avoid useless creation of redundant joins for singleton items.
+             *
+             * But how can we be sure that item is singleton if we look only at the last segment (which is single-valued)?
+             * Imagine we are creating join for single-valued entity of u.abc.(...).xyz.ent where
+             * ent is single-valued and not embedded (so we are to create something like "left join u.abc.(...).xyz.ent e").
+             * Then "u" is certainly a single value: either the root object, or some value pointed to by Exists restriction.
+             * Also, abc, ..., xyz are surely single-valued: otherwise they would be connected by a join. So,
+             * u.abc.(...).xyz.ent is a singleton.
+             *
+             * Look at it in other way: if e.g. xyz was multivalued, then we would have something like:
+             * left join u.abc.(...).uvw.xyz x
+             * left join x.ent e
+             * And, therefore we would not be looking for u.abc.(...).xyz.ent.
+             */
+
+            JoinSpecification existingJoin = hibernateQuery.getPrimaryEntity().findJoinFor(joinedItemFullPath);
+            if (existingJoin != null) {
+                // but let's check the condition as well
+                String existingAlias = existingJoin.getAlias();
+                // we have to create condition for existing alias, to be matched to existing condition
+                Condition conditionForExistingAlias = createJoinCondition(existingAlias, joinedItemDefinition, hibernateQuery);
+                if (ObjectUtils.equals(conditionForExistingAlias, existingJoin.getCondition())) {
+                    LOGGER.trace("Reusing alias '{}' for joined path '{}'", existingAlias, joinedItemFullPath);
+                    return existingAlias;
+                }
+            }
+        }
+        joinedItemAlias = hibernateQuery.createAlias(joinedItemDefinition);
+        Condition condition = createJoinCondition(joinedItemAlias, joinedItemDefinition, hibernateQuery);
+        hibernateQuery.getPrimaryEntity().addJoin(new JoinSpecification(joinedItemAlias, joinedItemFullPath, condition));
+        return joinedItemAlias;
+    }
+
+    private Condition createJoinCondition(String joinedItemAlias, JpaLinkDefinition joinedItemDefinition, RootHibernateQuery hibernateQuery) throws QueryException {
         Condition condition = null;
         if (joinedItemDefinition instanceof JpaAnyPropertyLinkDefinition) {
             JpaAnyPropertyLinkDefinition anyLinkDef = (JpaAnyPropertyLinkDefinition) joinedItemDefinition;
@@ -117,8 +155,7 @@ else if (joinedItemDefinition.getCollectionSpecification() instanceof VirtualCol
                 condition = conditions.iterator().next();
             }
         }
-        hibernateQuery.getPrimaryEntity().addJoin(new JoinSpecification(joinedItemAlias, joinedItemFullPath, condition));
-        return joinedItemAlias;
+        return condition;
     }
 
     /**