Native repo: LEFT JOIN is used when ordering by ref target item

This allows ordering by the target item for reference search where
it makes sense even for multi-value refs, because each ref value has
its own result row.
Target conditions are still applied by EXISTS (semi-join), but that's
not a correctness problem; possible improvements mentioned in comments.
Also, unique aliases are now not checked against JOINs of the Querydsl
query, but tracked explicitly in a Set in the root query context.

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleQueryContext.java

@@ -94,6 +94,7 @@ public static <S, Q extends FlexibleRelationalPathBase<R>, R> SqaleQueryContext<
                 rootPath, rootMapping, sqlRepoContext, query, objectLoader);
     }
 
+    /** Constructor for the root context. */
     private SqaleQueryContext(
             Q entityPath,
             SqaleTableMapping<S, Q, R> mapping,
@@ -103,6 +104,9 @@ private SqaleQueryContext(
         super(entityPath, mapping, sqlRepoContext, query);
         this.objectLoader = objectLoader;
         this.queryMapping = mapping;
+        // We want to register the initial FROM alias just in case the same mapping is needed again.
+        // This can happen with ref target dereferencing or with referenced-by filters.
+        uniqueAliasName(mapping.defaultAliasName());
     }
 
     private SqaleQueryContext(

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/filtering/RefItemFilterProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2022 Evolveum and contributors
+ * Copyright (C) 2010-2023 Evolveum and contributors
  *
  * This work is dual-licensed under the Apache License 2.0
  * and European Union Public License. See LICENSE file for details.
@@ -121,6 +121,10 @@ private Predicate targetFilterPredicate(@Nullable QName targetType, ObjectFilter
             throws RepositoryException {
         targetType = targetType != null ? targetType : ObjectType.COMPLEX_TYPE;
         var targetClass = context.prismContext().getSchemaRegistry().getCompileTimeClassForObjectType(targetType);
+        // TODO: This works fine, but LEFT JOIN should be considered for cases when the query is ordered by the target item.
+        // This is relevant for referenceSearch. When only where is used, subquery (AKA semi-join) is fine, often even preferable.
+        // But when ordering is added, we end up with both EXISTS subquery and LEFT JOIN for the order.
+        // See also: com.evolveum.midpoint.repo.sqale.mapping.RefTableTargetResolver#resolve()
         var subquery = context.subquery(context.repositoryContext().getMappingBySchemaType(targetClass));
         var targetPath = subquery.path(QObject.class);
         subquery.sqlQuery().where(oidPath.eq(targetPath.oid));

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/mapping/RefTableTargetResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2021 Evolveum and contributors
+ * Copyright (C) 2010-2023 Evolveum and contributors
  *
  * This work is dual-licensed under the Apache License 2.0
  * and European Union Public License. See LICENSE file for details.
@@ -8,7 +8,6 @@
 
 import java.util.function.Supplier;
 
-import com.querydsl.sql.SQLQuery;
 import org.jetbrains.annotations.NotNull;
 
 import com.evolveum.midpoint.repo.sqale.qmodel.object.MObject;
@@ -49,18 +48,26 @@ public RefTableTargetResolver(
     @Override
     public ResolutionResult<TQ, TR> resolve(SqlQueryContext<?, Q, R> context) {
         /*
-        Technically JOIN seems nicer as we are already inside EXISTS subquery for the reference
-        table, but the EXPLAIN plan doesn't seem to be any better, so we leave nested EXISTS here.
-        This may be revisited with big volume DB, but this is probably non-critical overall.
+        Currently, the JOIN does not reuse existing alias for the same mapping - which would be better for targetOid.
+        Also, this is used for ordering instructions, but not for WHERE clauses where EXISTS subquery is still used.
+        See also: com.evolveum.midpoint.repo.sqale.filtering.RefItemFilterProcessor#targetFilterPredicate()
+        */
         SqlQueryContext<?, TQ, TR> subcontext = context.leftJoin(
-                targetMappingSupplier.get(), context.path().targetOid.eq(subcontext.path().oid));
+                targetMappingSupplier.get(), (q, p) -> q.targetOid.eq(p.oid));
         return new ResolutionResult<>(subcontext, subcontext.mapping());
-        */
+
+        /*
+        EXISTS is also possible - and perhaps even preferable for WHERE, but does not allow
+        to use the target of the reference for ordering.
+        Ordering by the ref target is not possible for multi-value refs with object/container search.
+        It is possible for reference search, as each ref has its own role and a single target, so
+        LEFT JOIN is actually beneficial there.
 
         SqlQueryContext<?, TQ, TR> subcontext = context.subquery(targetMappingSupplier.get());
         SQLQuery<?> subquery = subcontext.sqlQuery();
         subquery.where(context.path().targetOid.eq(subcontext.path().oid));
 
         return new ResolutionResult<>(subcontext, subcontext.mapping(), true);
+        */
     }
 }

repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java

@@ -2533,7 +2533,7 @@ public void test810SearchLinkRefs() throws SchemaException {
     }
 
     @Test
-    public void test811CountLinkRefs() throws SchemaException {
+    public void test811CountLinkRefs() {
         when("counting link references (pure owned-by filter)");
         OperationResult operationResult = createOperationResult();
         ObjectQuery refQuery = prismContext
@@ -2549,19 +2549,26 @@ public void test811CountLinkRefs() throws SchemaException {
     }
 
     @Test
-    public void test819ReferenceQueryFailsOnWrongTopLevelPath() {
-        assertThatThrownBy(
-                () -> prismContext
-                        .queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
-                        .or()
-                        .item(UserType.F_ROLE_MEMBERSHIP_REF).ref("target-oid") // this fails
-                        .build())
-                .isInstanceOf(IllegalArgumentException.class)
-                .hasMessageStartingWith("Reference search only supports REF filter with SELF path (.) on the top level.");
+    public void test820SearchReferenceCanBeOrderedByTargetItem() throws SchemaException {
+        when("searching role membership references ordered by the target name");
+        OperationResult operationResult = createOperationResult();
+        ObjectQuery refQuery = prismContext.queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
+                .and()
+                .ref(ItemPath.SELF_PATH)
+                .item(F_NAME).eq(PolyString.fromOrig("role-one-more"), PolyString.fromOrig("role-other")).matchingOrig()
+                .desc(ItemPath.create(T_OBJECT_REFERENCE, F_NAME))
+                .build();
+        SearchResultList<ObjectReferenceType> result = searchReferences(refQuery, operationResult, null);
+
+        then("expected refs are returned in the required order");
+        assertThat(result)
+                .extracting(r -> r.getOid())
+                .containsExactly(roleOtherOid, roleOneMoreOid);
     }
 
     // TODO test wrong ref type
     // TODO test multiple owned-by (failing)
+    // TODO test non AND filter (OR and NOT on the top level have no meaning, this needs fixing in repo too)
 
     @Test
     public void test890ReferenceSearchWithNullQueryFails() {
@@ -2584,6 +2591,18 @@ public void test891ReferenceSearchWithNullFilterFails() {
                 .isInstanceOf(SystemException.class)
                 .hasMessageStartingWith("Invalid filter for reference search: null");
     }
+
+    @Test
+    public void test892ReferenceQueryFailsOnWrongTopLevelPath() {
+        assertThatThrownBy(
+                () -> prismContext
+                        .queryForReferenceOwnedBy(UserType.class, UserType.F_ROLE_MEMBERSHIP_REF)
+                        .or()
+                        .item(UserType.F_ROLE_MEMBERSHIP_REF).ref("target-oid") // this fails
+                        .build())
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageStartingWith("Reference search only supports REF filter with SELF path (.) on the top level.");
+    }
     // endregion
 
     // region special cases

repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/SqlQueryContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2022 Evolveum and contributors
+ * Copyright (C) 2010-2023 Evolveum and contributors
  *
  * This work is dual-licensed under the Apache License 2.0
  * and European Union Public License. See LICENSE file for details.
@@ -102,6 +102,8 @@ public abstract class SqlQueryContext<S, Q extends FlexibleRelationalPathBase<R>
     // options stored to modify select clause and also to affect mapping
     protected Collection<SelectorOptions<GetOperationOptions>> options;
 
+    private final Set<String> usedAliases;
+
     /** Constructor for root query context. */
     protected SqlQueryContext(
             Q entityPath,
@@ -113,6 +115,7 @@ protected SqlQueryContext(
         this.sqlRepoContext = sqlRepoContext;
         this.sqlQuery = query;
         this.parent = null;
+        this.usedAliases = new HashSet<>();
     }
 
     /** Constructor for derived context or sub-context, e.g. JOIN, EXISTS, etc. */
@@ -126,6 +129,7 @@ protected SqlQueryContext(
         this.sqlRepoContext = parentContext.repositoryContext();
         this.sqlQuery = sqlQuery;
         this.parent = parentContext;
+        this.usedAliases = null;
     }
 
     public Q root() {
@@ -418,19 +422,21 @@ public <TS, TQ extends FlexibleRelationalPathBase<TR>, TR> SqlQueryContext<TS, T
     SqlQueryContext<TS, TQ, TR> newSubcontext(
             TQ newPath, QueryTableMapping<TS, TQ, TR> newMapping, SQLQuery<?> query);
 
+    /** Adjusts the alias name to make it unique and registers the name as used. */
     public String uniqueAliasName(String baseAliasName) {
-        Set<String> joinAliasNames =
-                sqlQuery.getMetadata().getJoins().stream()
-                        .map(j -> j.getTarget().toString())
-                        .collect(Collectors.toSet());
+        if (parent != null) {
+            // We always want to use root context to make the names unique for the whole query.
+            return parent.uniqueAliasName(baseAliasName);
+        }
 
-        // number the alias if not unique (starting with 2, implicit 1 is without number)
+        // Number the alias if not unique (starting with 2, implicit 1 is without number).
         String aliasName = baseAliasName;
         int sequence = 1;
-        while (joinAliasNames.contains(aliasName)) {
+        while (usedAliases.contains(aliasName)) {
             sequence += 1;
             aliasName = baseAliasName + sequence;
         }
+        usedAliases.add(aliasName);
         return aliasName;
     }
 

repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/mapping/TableRelationResolver.java

@@ -18,7 +18,7 @@
 
 /**
  * Resolver that knows how to traverse to the specified target query type.
- * By default EXISTS subquery is used which is better for multi-value table stored items
+ * By default, EXISTS subquery is used which is better for multi-value table stored items
  * to avoid result multiplication.
  * The resolver supports mapping supplier to avoid call cycles during mapping initialization.
  *
@@ -34,13 +34,9 @@ public class TableRelationResolver<
         implements ItemRelationResolver<Q, R, TQ, TR> {
 
     protected final Supplier<QueryTableMapping<TS, TQ, TR>> targetMappingSupplier;
-    protected final BiFunction<Q, TQ, Predicate> correlationPredicate;
+    protected final BiFunction<Q, TQ, Predicate> correlationPredicateFunction;
     private final boolean useSubquery;
 
-    public BiFunction<Q, TQ, Predicate> getCorrelationPredicate() {
-        return correlationPredicate;
-    }
-
     public Supplier<QueryTableMapping<TS, TQ, TR>> getTargetMappingSupplier() {
         return targetMappingSupplier;
     }
@@ -73,7 +69,7 @@ protected TableRelationResolver(
             @NotNull QueryTableMapping<TS, TQ, TR> targetMapping,
             @NotNull BiFunction<Q, TQ, Predicate> correlationPredicateFunction) {
         this.targetMappingSupplier = () -> targetMapping;
-        this.correlationPredicate = correlationPredicateFunction;
+        this.correlationPredicateFunction = correlationPredicateFunction;
         this.useSubquery = true;
     }
 
@@ -87,7 +83,7 @@ private TableRelationResolver(
             @NotNull Supplier<QueryTableMapping<TS, TQ, TR>> targetMappingSupplier,
             @NotNull BiFunction<Q, TQ, Predicate> correlationPredicateFunction) {
         this.targetMappingSupplier = targetMappingSupplier;
-        this.correlationPredicate = correlationPredicateFunction;
+        this.correlationPredicateFunction = correlationPredicateFunction;
         this.useSubquery = false;
     }
 
@@ -102,32 +98,32 @@ public ResolutionResult<TQ, TR> resolve(SqlQueryContext<?, Q, R> context) {
         if (useSubquery) {
             SqlQueryContext<TS, TQ, TR> subcontext = context.subquery(targetMappingSupplier.get());
             SQLQuery<?> subquery = subcontext.sqlQuery();
-            subquery.where(correlationPredicate.apply(context.path(), subcontext.path()));
+            subquery.where(correlationPredicateFunction.apply(context.path(), subcontext.path()));
 
             return new ResolutionResult<>(subcontext, subcontext.mapping(), true);
         } else {
             SqlQueryContext<TS, TQ, TR> subcontext = context.leftJoin(
-                    targetMappingSupplier.get(), correlationPredicate);
+                    targetMappingSupplier.get(), correlationPredicateFunction);
             return new ResolutionResult<>(subcontext, subcontext.mapping());
         }
     }
 
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     public TableRelationResolver<Q, R, TS, TQ, TR> replaceTable(QueryTableMapping<? extends TS, TQ, TR> target) {
         // FIXME: Add check
 
-        return new TableRelationResolver(() -> target, correlationPredicate);
+        return new TableRelationResolver(() -> target, correlationPredicateFunction);
     }
 
     public TableRelationResolver<Q, R, TS, TQ, TR> withSubquery() {
-        return usingSubquery(targetMappingSupplier.get(), correlationPredicate);
+        return usingSubquery(targetMappingSupplier.get(), correlationPredicateFunction);
     }
 
     public <AQ extends FlexibleRelationalPathBase<AR>, AS, AR>
     TableRelationResolver<TQ, TR, AS, AQ, AR> reverse(
             @NotNull QueryTableMapping<AS, AQ, AR> targetMapping) {
         //noinspection unchecked
-        return new TableRelationResolver<>(targetMapping, (t, a) -> correlationPredicate.apply((Q) a, t));
+        return new TableRelationResolver<>(targetMapping, (t, a) -> correlationPredicateFunction.apply((Q) a, t));
     }
 
     public QueryTableMapping<TS, TQ, TR> mapping() {