Avoid wide distinct scans for visible captures#1372
Conversation
Assisted-by: Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
👷 Deploy request for antenna-ssec pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for antenna-preview canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesVisibility deduplication
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ami/main/tests.py`:
- Around line 3235-3239: Update
test_visible_for_user_deduplicates_captures_in_a_primary_key_subquery to create
a second membership matching the same project, query visibility as the project
owner, and assert the returned primary-key values are unique. Retain the
assertion that the query SQL does not contain SELECT DISTINCT so the test
specifically exercises duplicate membership joins.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65ceefce-73ac-4cc9-bfd5-23ddfb5e9074
📒 Files selected for processing (2)
ami/base/models.pyami/main/tests.py
There was a problem hiding this comment.
Pull request overview
This PR updates the core BaseQuerySet.visible_for_user() permission-scoping helper to avoid emitting wide-row SELECT DISTINCT queries (notably for captures / SourceImage), by filtering the outer queryset via a primary-key subquery instead. This targets a known Postgres temp-file spill issue on large deployments while keeping visibility semantics consistent.
Changes:
- Replace
.distinct()-based deduplication invisible_for_user()with apk__inprimary-key subquery that clears ordering in the inner query. - Add a regression test asserting that capture visibility works for a member and that the generated SQL does not include
SELECT DISTINCT.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ami/base/models.py |
Reworks BaseQuerySet.visible_for_user() to avoid full-row DISTINCT by using a primary-key subquery filter. |
ami/main/tests.py |
Adds a regression test to confirm visible captures are returned without generating SELECT DISTINCT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| visible_project_ids = self.filter(filter_condition).order_by().values_list("pk", flat=True) | ||
| return self.filter(pk__in=visible_project_ids) |
There was a problem hiding this comment.
Addressed in 50a4909: anonymous project visibility now returns the simple draft=False filter directly, avoiding the unnecessary self-referential primary-key subquery.
Assisted-by: Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Avoid wide distinct scans for visible captures
Summary
Capture-list visibility filtering previously used a full-row
DISTINCTafter the project-membership join, which can cause PostgreSQL to sort wide capture rows and spill large temporary files. This change filters the outer queryset by a narrow primary-key subquery instead, preserving the visibility rules while avoiding the wide-row distinct operation.List of Changes
SELECT DISTINCTacross full model rows.SELECT DISTINCT.Related Issues
Fixes #1370
Detailed Description
The owner-or-member visibility condition can join project memberships and multiply result rows. The inner queryset now selects only matching primary keys with default ordering removed; the outer queryset applies
pk__inand remains available for callers to order, annotate, paginate, and select the full capture rows normally.This is autonomous AI-agent work that @Dodothereal operates, monitors, and is accountable for.
How to Test the Changes
Run in the required sandbox:
sandbox-run "python -m py_compile ami/base/models.py ami/main/tests.py"— passed.sandbox-run "pip install Django==4.2.10 && python - <<'PY' ..."— passed an isolated Django ORM check confirming the primary-key subquery preserves ordered results and emits noDISTINCT.sandbox-run "python manage.py test ami.main.tests.TestDraftProjectPermissions --keepdb"— could not start because the sandbox had no Django installed.sandbox-run "pip install -r requirements/base.txt && python manage.py test ami.main.tests.TestDraftProjectPermissions --keepdb"— blocked during dependency resolution because the repository pinspsycopg[binary]==3.1.9, for which no package is available on the sandbox's Python 3.12 platform.sandbox-run "docker compose -f docker-compose.ci.yml run --rm django python manage.py test ami.main.tests.TestDraftProjectPermissions --keepdb"— unavailable because Docker is not installed in the sandbox.Deployment Notes
No migration or configuration change is required.
Checklist
AI assistance disclosure
This contribution was produced by an autonomous AI coding agent (Claude Code) that @Dodothereal operates and monitors. @Dodothereal is accountable for it, will address review feedback promptly, and will close this PR immediately if this kind of contribution is unwelcome in this project. Commits carry an
Assisted-by: Claude Codetrailer.Summary by CodeRabbit