Conversation
Apply DISTINCT only when the criteria query joins related tables, preserving deduplication for attribute searches while allowing plain user listings to use database indexes efficiently. Closes keycloak#51385 Signed-off-by: Ravi Ruhela <51108294+MedicPro@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The test does not verify that plain user searches omit DISTINCT, leaving the primary regression fix uncovered.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Optimizes JPA user searches by applying DISTINCT only when joins can create duplicate rows.
Changes:
- Conditionally enables
DISTINCTbased on query joins. - Adds coverage ensuring attribute joins still deduplicate users.
| File | Description |
|---|---|
model/jpa/.../JpaUserProvider.java |
Avoids unnecessary DISTINCT for plain searches. |
tests/base/.../UserModelTest.java |
Verifies deduplication for multi-valued attributes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| predicates.addAll(AdminPermissionsSchema.SCHEMA.applyAuthorizationFilters(session, AdminPermissionsSchema.USERS, this, realm, builder, queryBuilder, root)); | ||
|
|
||
| queryBuilder.distinct(true).where(predicates).orderBy(builder.asc(root.get(UserModel.USERNAME))); | ||
| queryBuilder.distinct(!root.getJoins().isEmpty()).where(predicates).orderBy(builder.asc(root.get(UserModel.USERNAME))); |
|
Hi @MedicPro, I’m interested in independently validating the performance impact of this change for #51385. My initial scope would be a reproducible PostgreSQL comparison of plain user listing before and after the patch, followed by a small correctness check for searches involving multi-valued user attributes. I’m new to contributing to Keycloak and working on this in my spare time with limited hardware, so I can’t commit to a completion date yet. I can post an initial update within a week on whether I can reproduce the issue and run the comparison. Would this be useful, and is anyone already working on similar validation? Is there a preferred baseline commit or dataset? Please don’t hold up the PR on my account. |
Apply DISTINCT only when the criteria query joins related tables, preserving deduplication for attribute searches while allowing plain user listings to use database indexes efficiently.
Closes #51385