refactor(speakers): replace nested IN(SELECT DISTINCT) with correlated EXISTS in getUniqueActivitiesCountBySummit#560
Conversation
…d EXISTS in getUniqueActivitiesCountBySummit The previous implementation wrapped the filter subquery output in IN(SELECT DISTINCT speaker_ids), forcing MySQL to materialise the full intermediate set and probe it per outer presentation row. Restructured as a single correlated EXISTS rooted at PresentationSpeaker: the inner QB checks assignment or moderator membership against the outer presentation p directly, with filter predicates appended inline. MySQL can now use the Presentation_Speakers(PresentationID) index and short-circuit on the first match per row.
📝 WalkthroughWalkthrough
ChangesUnique Activities Count Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-560/ This page is automatically updated on each push to this PR. |
…nate correlated subquery Replaced the correlated EXISTS (which scanned all PresentationSpeaker rows per outer presentation) with two non-correlated IN subqueries — one for the assignment path, one for the moderator path — each embedding the filtered speaker set once. Presentation ID lists are merged and deduplicated in PHP. MySQL can now materialise the speaker set once rather than re-executing per presentation row.
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-560/ This page is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/Repositories/Summit/DoctrineSpeakerRepository.php (1)
723-750: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDeduplicate assignment IDs before hydrating them.
Line 724 selects raw
p.idfrom a many-speaker join, so a presentation with multiple qualifying assigned speakers still returns duplicate rows and relies on PHP to discard them. Adddistinct(true)to reduce DB result size while keeping the finalarray_uniquefor assignment/moderator overlap.♻️ Proposed change
$assignmentQb = $this->getEntityManager()->createQueryBuilder() + ->distinct(true) ->select('p.id') ->from('models\summit\Presentation', 'p') ->join('p.speakers', 'a')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Repositories/Summit/DoctrineSpeakerRepository.php` around lines 723 - 750, The query in the assignmentQb query builder is joining presentations with multiple speakers, which causes duplicate presentation IDs to be returned from the database. Add distinct(true) to the assignmentQb query builder (the one that selects presentation IDs where speakers are qualifying speakers using the join on 'p.speakers') to eliminate duplicates at the database level. Keep the final array_unique call on the merged results to handle any overlap between assignment and moderator presentation IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/Repositories/Summit/DoctrineSpeakerRepository.php`:
- Around line 723-750: The query in the assignmentQb query builder is joining
presentations with multiple speakers, which causes duplicate presentation IDs to
be returned from the database. Add distinct(true) to the assignmentQb query
builder (the one that selects presentation IDs where speakers are qualifying
speakers using the join on 'p.speakers') to eliminate duplicates at the database
level. Keep the final array_unique call on the merged results to handle any
overlap between assignment and moderator presentation IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef47d5c4-f281-48a5-b1b4-9cee81351920
📒 Files selected for processing (1)
app/Repositories/Summit/DoctrineSpeakerRepository.php
|
supersede by #563 |
2 participants
ref: https://app.clickup.com/t/9014802374/86b9b1qrk
Summary
getUniqueActivitiesCountBySummitpreviously wrapped the filtered speaker set inIN(SELECT DISTINCT speaker_ids), forcing MySQL to materialise the full intermediate result and probe it per outer presentation row — observed at ~20s on moderately-sized data sets.EXISTSrooted atPresentationSpeaker: the inner QB checks assignment or moderator membership against the outer presentationpdirectly, with filter predicates appended inline viaapply2Query.Presentation_Speakers(PresentationID)index and short-circuit on the first match per presentation row.Test plan
OAuth2SummitSpeakersApiTest,SubmitterRepositoryTest)Summary by CodeRabbit