Skip to content

fix(handlers): replace N+1 DB queries with single IN query in handle_batch_refresh_prs#278

Open
ojaswa072 wants to merge 1 commit intoOWASP-BLT:mainfrom
ojaswa072:fix/batch-refresh-n-plus-one-db-queries
Open

fix(handlers): replace N+1 DB queries with single IN query in handle_batch_refresh_prs#278
ojaswa072 wants to merge 1 commit intoOWASP-BLT:mainfrom
ojaswa072:fix/batch-refresh-n-plus-one-db-queries

Conversation

@ojaswa072
Copy link
Copy Markdown
Contributor

@ojaswa072 ojaswa072 commented Mar 9, 2026

Summary

Fixed an N+1 database query problem in handle_batch_refresh_prs.

Problem

The original code fetched PR details from the database in a loop — one query per PR ID:

for pr_id in pr_ids:
    stmt = db.prepare('SELECT ... FROM prs WHERE id = ?').bind(pr_id)
    result = await stmt.first()

For 100 PRs (the maximum allowed), this meant 100 sequential database round-trips before the GitHub batch API call even started.

Fix

Replaced the loop with a single WHERE id IN (...) query that fetches all PRs in one round-trip:

placeholders = ','.join('?' * len(pr_ids))
stmt = db.prepare(
    f'SELECT id, pr_url, repo_owner, repo_name, pr_number, etag FROM prs WHERE id IN ({placeholders})'
).bind(*pr_ids)
all_results = await stmt.all()

Not-found IDs are still logged individually for debugging.

Impact

  • Reduces DB round-trips from N to 1 for batch refresh
  • No change to API behavior or response format

Files Changed

  • src/handlers.pyhandle_batch_refresh_prs function

Summary by CodeRabbit

  • Refactor

    • Optimized batch pull request retrieval to use a single database query instead of multiple individual lookups, reducing round-trips and improving performance.
  • Bug Fixes

    • Enhanced detection and warning messages for missing pull requests during batch operations.

@owasp-blt
Copy link
Copy Markdown

owasp-blt bot commented Mar 9, 2026

👋 Thanks for opening this pull request, @ojaswa072!

Before your PR is reviewed, please ensure:

  • Your code follows the project's coding style and guidelines.
  • You have written or updated tests for your changes.
  • The commit messages are clear and descriptive.
  • You have linked any relevant issues (e.g., Closes #123).

🔍 Our team will review your PR shortly. If you have questions, feel free to ask in the comments.

🚀 Keep up the great work! — OWASP BLT

@owasp-blt
Copy link
Copy Markdown

owasp-blt bot commented Mar 9, 2026

📊 Monthly Leaderboard

Hi @ojaswa072! Here's how you rank for March 2026:

Rank User Open PRs PRs (merged) PRs (closed) Reviews Comments Total
🥇 #1 @Nachiket-Roy 6 47 13 8 6 502
🥈 #2 @ojaswa072 13 30 0 0 1 315
#3 @e-esakman 2 26 0 6 4 300

Scoring this month (across OWASP-BLT org): Open PRs (+1 each), Merged PRs (+10), Closed (not merged) (−2), Reviews (+5; first two per PR in-month), Comments (+2, excludes CodeRabbit). Run /leaderboard on any issue or PR to see your rank!

@owasp-blt
Copy link
Copy Markdown

owasp-blt bot commented Mar 9, 2026

👋 Hi @ojaswa072!

This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:

  • The PR author
  • coderabbitai
  • copilot

Once a valid peer review is submitted, this check will pass automatically. Thank you!

⚠️ Peer review enforcement is active.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 9, 2026

🍃 PR Readiness Check

Check the readiness of this PR on Leaf:
👉 Open on Leaf

Leaf reviews pull requests for operational readiness, security risks, and production-impacting changes before they ship.

@github-actions github-actions bot added the files-changed: 1 PR changes 1 file label Mar 9, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: OWASP-BLT/coderabbit/.coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 0b3f579c-d191-4513-adb8-b6311c924944

📥 Commits

Reviewing files that changed from the base of the PR and between ee8eb8a and c9a5646.

📒 Files selected for processing (1)
  • src/handlers.py

Walkthrough

Optimizes handle_batch_refresh_prs to replace individual PR database lookups with a single IN query. Missing PR IDs are tracked and skipped with warning messages. Subsequent batch processing logic remains unchanged.

Changes

Cohort / File(s) Summary
Optimized PR Batch Lookup
src/handlers.py
Consolidates per-ID database queries into a single IN query for improved efficiency. Tracks found PR IDs to detect missing entries, emitting skip messages for any not found. Maintains existing behavior for valid PRs in subsequent batch processing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

quality: medium

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing N+1 database queries with a single IN query in the handle_batch_refresh_prs function, which directly matches the changeset's primary objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@owasp-blt owasp-blt bot added has-peer-review PR has received peer review and removed needs-peer-review PR needs peer review labels Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants