Skip to content

Commit

Permalink
Fix archetypeRef/@/name query on generic repo
Browse files Browse the repository at this point in the history 
When dereferencing using the "@" operator, we now tell
Hibernate to use the specific target (joined) type by using
root joins.

However, the SQL that is generated sometimes suboptimal,
as it includes useless join on M_OBJECT table.

Related to MID-9427. To be reviewed.
  • Loading branch information
@mederly
mederly committed Jan 23, 2024
1 parent a40a6fa commit 00e4c27
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 84 deletions.
192 changes: 134 additions & 58 deletions ...repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2284,7 +2284,6 @@ public void test160QueryObjectTypeByTypeUserAndLocality() throws Exception {
+ " _ou.localityFocus.norm = :norm\n"
+ " )\n"
+ " )");
checkQueryTypeAlias(hqlToSql(real), "m_user", "localityFocus_orig", "localityFocus_norm");
} finally {
close(session);
}
Expand Down Expand Up @@ -2344,7 +2343,6 @@ public void test162QueryObjectTypeByTypeOrgAndLocality() throws Exception {
+ " _oo.localityFocus.norm = :norm\n"
+ " )\n"
+ " )");
checkQueryTypeAlias(hqlToSql(real), "m_org", "localityFocus_orig", "localityFocus_norm");
} finally {
close(session);
}
Expand Down Expand Up @@ -3333,35 +3331,36 @@ public void test322AssignmentQueryByTargetName() throws Exception {
.build();

String real = getInterpretedQuery(session, AssignmentType.class, query, false);
assertThat(real).isEqualToIgnoringWhitespace("select\n"
+ " _a.ownerOid,\n"
+ " _a.id,\n"
+ " _a.order,\n"
+ " _a.lifecycleState,\n"
+ " _a.activation,\n"
+ " _a.targetRef,\n"
+ " _a.tenantRef,\n"
+ " _a.orgRef,\n"
+ " _a.resourceRef,\n"
+ " _a.createTimestamp,\n"
+ " _a.creatorRef,\n"
+ " _a.createChannel,\n"
+ " _a.modifyTimestamp,\n"
+ " _a.modifierRef,\n"
+ " _a.modifyChannel\n"
+ "from\n"
+ " RAssignment _a\n"
+ " left join _a.targetRef.target _t\n"
+ "where\n"
+ " (\n"
+ " _a.targetRef.targetType = :targetType and\n"
+ " (\n"
+ " _t.name.orig = :orig and\n"
+ " _t.name.norm = :norm\n"
+ " ) and\n"
+ " _a.ownerOid in (:ownerOid)\n"
+ " )\n"
+ "order by _t.name.orig asc");
assertThat(real).isEqualToIgnoringWhitespace("""
select
_a.ownerOid,
_a.id,
_a.order,
_a.lifecycleState,
_a.activation,
_a.targetRef,
_a.tenantRef,
_a.orgRef,
_a.resourceRef,
_a.createTimestamp,
_a.creatorRef,
_a.createChannel,
_a.modifyTimestamp,
_a.modifierRef,
_a.modifyChannel
from
RAssignment _a
left join RObject _t on (_a.targetRef.target = _t)
where
(
_a.targetRef.targetType = :targetType and
(
_t.name.orig = :orig and
_t.name.norm = :norm
) and
_a.ownerOid in (:ownerOid)
)
order by _t.name.orig asc""");
} finally {
close(session);
}
Expand All @@ -3381,17 +3380,19 @@ public void test400DereferenceLink() throws Exception {
.build();

String real = getInterpretedQuery(session, UserType.class, query);
assertThat(real).isEqualToIgnoringWhitespace("select\n" +
" _u.oid, _u.fullObject\n" +
"from\n" +
" RUser _u\n" +
" left join _u.linkRef _l\n" +
" left join _l.target _t\n" +
"where\n" +
" (\n" +
" _t.nameCopy.orig like :orig escape '!' and\n" +
" _t.nameCopy.norm like :norm escape '!'\n" +
" )\n");
assertThat(real).isEqualToIgnoringWhitespace("""
select
_u.oid, _u.fullObject
from
RUser _u
left join _u.linkRef _l
left join RShadow _t on (_l.target = _t)
where
(
_t.nameCopy.orig like :orig escape '!' and
_t.nameCopy.norm like :norm escape '!'
)
""");
} finally {
close(session);
}
Expand All @@ -3413,22 +3414,53 @@ public void test402DereferenceLinkedResourceName() throws Exception {
.build();

String real = getInterpretedQuery(session, UserType.class, query);
assertThat(real).isEqualToIgnoringWhitespace("select\n" +
" _u.oid, _u.fullObject\n" +
"from\n" +
" RUser _u\n" +
" left join _u.linkRef _l\n" +
" left join _l.target _t\n" +
" left join _t.resourceRef.target _t2\n" +
"where\n" +
" _t2.name.norm like :norm escape '!'\n");
assertThat(real).isEqualToIgnoringWhitespace("""
select
_u.oid, _u.fullObject
from
RUser _u
left join _u.linkRef _l
left join RShadow _t on (_l.target = _t)
left join RObject _t2 on (_t.resourceRef.target = _t2)
where
_t2.name.norm like :norm escape '!'
""");

} finally {
close(session);
}
}

/** See {@link SearchTest#test981SearchByArchetypeName()} */
@Test
public void test402DereferenceArchetypeName() throws Exception {
Session session = open();

try {
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_ARCHETYPE_REF, PrismConstants.T_OBJECT_REFERENCE, F_NAME).eqPoly("System")
.build();

String real = getInterpretedQuery(session, UserType.class, query);
assertThat(real).isEqualToIgnoringWhitespace("""
select
_u.oid,
_u.fullObject
from
RUser _u
left join _u.archetypeRef _a
left join RArchetype _t on (_a.target = _t)
where
(
_t.nameCopy.orig = :orig and
_t.nameCopy.norm = :norm
)""");
} finally {
close(session);
}
}

@Test(expectedExceptions = IllegalArgumentException.class) // at this time
@Test(expectedExceptions = IllegalArgumentException.class) // at this time
public void test404DereferenceAssignedRoleType() throws Exception {
Session session = open();

Expand Down Expand Up @@ -3546,12 +3578,14 @@ public void test520OrderByTargetName() throws Exception {
PrismConstants.T_OBJECT_REFERENCE, F_NAME), ASCENDING));

String real = getInterpretedQuery(session, AccessCertificationCaseType.class, query);
assertThat(real).isEqualToIgnoringWhitespace("select\n" +
" _a.ownerOid, _a.id, _a.fullObject\n" +
"from\n" +
" RAccessCertificationCase _a\n" +
" left join _a.targetRef.target _t\n" +
"order by _t.name.orig asc\n");
assertThat(real).isEqualToIgnoringWhitespace("""
select
_a.ownerOid, _a.id, _a.fullObject
from
RAccessCertificationCase _a
left join RObject _t on (_a.targetRef.target = _t)
order by _t.name.orig asc
""");
} finally {
close(session);
}
Expand Down Expand Up @@ -4998,6 +5032,48 @@ public void test801QueryTaskRecurrence() throws Exception {
}
}

/** @see SearchTest#test943MultiValueRefTargetWithTargetTypeSpecificCondition() */
@Test
public void test802MultiValueRefTargetWithTargetTypeSpecificCondition() throws Exception {
Session session = open();

try {
given();
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_LINK_REF, T_OBJECT_REFERENCE, ShadowType.F_RESOURCE_REF).ref("ef2bc95b-76e0-48e2-86d6-3d4f02d3e1a2")
.build();

when("the query is executed");
String real = getInterpretedQuery(session, UserType.class, query);

then("expected HQL is generated");
assertThat(real).isEqualToIgnoringWhitespace("""
select
_u.oid,
_u.fullObject
from
RUser _u
left join _u.linkRef _l
left join RShadow _t on (_l.target = _t)
where
(
_t.resourceRef.targetOid = :targetOid and
_t.resourceRef.relation in (:relation)
)""");

// Note that the SQL is not particularly efficient - see the unnecessary join on m_object:
//
// SELECT _u.oid, _o.fullObject
// FROM m_user _u
// JOIN m_object _o ON _u.oid = _o.oid
// LEFT JOIN m_reference _ref ON _o.oid = _ref.owner_oid AND (_ref.reference_type = 1)
// LEFT JOIN m_shadow _s ON (_ref.targetOid = _s.oid)
// WHERE (_s.resourceRef_targetOid = ? AND _s.resourceRef_relation IN (?, ?, ?))
} finally {
close(session);
}
}

private Collection<SelectorOptions<GetOperationOptions>> distinct() {
return createCollection(createDistinct());
}
Expand Down
37 changes: 37 additions & 0 deletions repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/SearchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,43 @@ public void test980SearchPasswordCreateTimestamp() throws Exception {
assertEquals("Wrong user name", "atestuserX00002", users.get(0).getName().getOrig());
}

/** MID-9427 */
@Test
public void test981SearchByArchetypeName() throws Exception {
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_ARCHETYPE_REF, T_OBJECT_REFERENCE, F_NAME)
.eqPoly("archetype1")
.build();
OperationResult result = new OperationResult("search");
List<PrismObject<UserType>> users = repositoryService.searchObjects(UserType.class, query, null, result);
assertThatOperationResult(result).isSuccess();
assertThat(users)
.as("users found")
.hasSize(2)
.map(u -> u.getName().getOrig())
.as("user names")
.containsExactlyInAnyOrder("atestuserX00002", "atestuserX00003");

// Note that the SQL is not efficient (useless join on m_object):
//
// SELECT _u.oid, _o.fullObject
// FROM m_user _u
// JOIN m_object _o ON _u.oid = _o.oid
// LEFT JOIN m_reference _ref ON _o.oid = _ref.owner_oid AND (_ref.reference_type = 11)
// LEFT JOIN m_archetype _a ON (_ref.targetOid = _a.oid)
// WHERE (_a.name_orig = ? AND _a.name_norm = ?)

// The same but arbitrary assignment holders now
var objects = repositoryService.searchObjects(AssignmentHolderType.class, query, null, result);
assertThatOperationResult(result).isSuccess();
assertThat(objects)
.as("objects found")
.hasSize(3)
.map(u -> u.getName().getOrig())
.as("object names")
.containsExactlyInAnyOrder("atestuserX00002", "atestuserX00003", "Synchronization: Embedded Test OpenDJ");
}

@Test
public void test999MultipleOrdersAreSupportedByFluentApiAndRepository() throws SchemaException {
given("search users query ordered by family and given name");
Expand Down

0 comments on commit 00e4c27

Please sign in to comment.