Skip to content

perf(users): avoid full-table sorts in list queries - #774

Open
dr-hoseyn wants to merge 1 commit into
PasarGuard:devfrom
dr-hoseyn:perf/user-list-count-and-sort
Open

perf(users): avoid full-table sorts in list queries#774
dr-hoseyn wants to merge 1 commit into
PasarGuard:devfrom
dr-hoseyn:perf/user-list-count-and-sort

Conversation

@dr-hoseyn

@dr-hoseyn dr-hoseyn commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove page ordering and wide user columns from the /api/users total-count query
  • add a direction-matched id tie-breaker for deterministic offset pagination
  • add (created_at, id) for the dashboard's default user-list order
  • build/drop the PostgreSQL index concurrently to keep user writes available during upgrades
  • add cross-dialect SQL and index regression tests

Closes #773

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Refactor / cleanup
  • Documentation
  • Tests / CI

Checklist

  • I tested the change locally or explained why it cannot be tested.
  • I added or updated tests for behavior changes.
  • I updated documentation, translations, or examples if needed. (Not applicable: API schema is unchanged.)
  • I checked database migrations when models or schema changed.
  • I did not include secrets, tokens, private keys, or unrelated changes.

Testing

  • uv run ruff check . — passed
  • targeted Ruff format check — passed
  • uv run pytest -q tests/test_user_list_query_scaling.py — 4 passed
  • targeted endpoint/count tests — 6 passed
  • uv run pytest -q tests/api/test_user.py — 82 passed
  • uv run alembic check on SQLite — no new upgrade operations
  • empty SQLite migration chain to head — passed
  • one-revision downgrade/upgrade and index introspection — passed
  • PostgreSQL offline SQL — CREATE INDEX CONCURRENTLY / DROP INDEX CONCURRENTLY
  • MySQL migration compilation — passed

Synthetic SQLite benchmark with 500,000 users and a 50-row page:

Operation Before After
Total count 203.143 ms median 0.224 ms median
Default page 44.950 ms median 0.052 ms median

Before the change both plans used a full scan and temporary B-tree sort. The optimized page uses idx_users_created_at_id; the count no longer sorts. The benchmark is directional, not a production guarantee.

Screenshots

Not applicable (backend/database-only change).

Notes for reviewers

  • Response fields, filters, and pagination parameters are unchanged.
  • Owner/admin joins remain in the count statement; regression coverage verifies this.
  • PostgreSQL concurrent DDL is isolated with Alembic's autocommit_block().
  • This PR and perf(groups): scale membership summaries #772 both independently target the current dev migration head. If one lands first, the other migration should be rebased onto the new head before merge to avoid parallel Alembic heads.

Summary by CodeRabbit

  • Performance

    • Improved user list loading and counting, especially for filtered results.
    • Added a database index to support faster, more consistent user sorting.
  • Bug Fixes

    • User lists now maintain stable ordering when multiple users share the same creation time.
    • User counts preserve existing filters and owner-related conditions accurately.

@dr-hoseyn

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ebc329ec-9c3f-4c8d-9c17-d974f3d21ad7

📥 Commits

Reviewing files that changed from the base of the PR and between 3331421 and 3b4e1cd.

📒 Files selected for processing (4)
  • app/db/crud/user.py
  • app/db/migrations/versions/b7e2c4d91f60_add_user_list_sort_index.py
  • app/db/models.py
  • tests/test_user_list_query_scaling.py

Walkthrough

User listing now uses deterministic sorting and narrow filtered count queries. The database schema adds a (created_at, id) index, with dialect-specific migration handling. Tests cover query construction, filters, sorting, and index metadata.

Changes

User list query scaling

Layer / File(s) Summary
Deterministic sorting and filtered counts
app/db/crud/user.py, tests/test_user_list_query_scaling.py
get_users now uses centralized sort clauses with an ID tie-breaker matching the final direction. Count queries remove ordering, count User.id, preserve joins and filters, and exclude wide user columns.
Composite sort index and migration
app/db/models.py, app/db/migrations/versions/..., tests/test_user_list_query_scaling.py
The User model defines the (created_at, id) index. The migration creates and removes it, using concurrent PostgreSQL operations and standard operations for other dialects. Tests verify the index name and column order.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 3b4e1

This change optimizes user-list counting and pagination while preserving the API contract; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: immohammad20000, m03ed

Poem

A rabbit sorted users in line,
With dates and IDs aligned just fine.
Counts grew light, orders stayed true,
An index joined the query crew.
“Hop!” said the rabbit, “Less work to do!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary performance change for user list queries.
Linked Issues check ✅ Passed The changes implement the count rewrite, deterministic sorting, composite index, concurrent PostgreSQL DDL, and tests required by [#773].
Out of Scope Changes check ✅ Passed All code changes directly support the user-list query performance and compatibility objectives in [#773].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant