Skip to content

fix: make session list LIMIT bound sessions, not joined rows - #787

Open
grvijayan wants to merge 6 commits into
mainfrom
fix-session-list-limit
Open

fix: make session list LIMIT bound sessions, not joined rows#787
grvijayan wants to merge 6 commits into
mainfrom
fix-session-list-limit

Conversation

@grvijayan

@grvijayan grvijayan commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes #782

The session list query joins sessions with user_agents and checks, and the compiled LIMIT was appended to the joined statement. The checks join is one-to-many, so the limit bounded session-check rows instead of sessions: a session with several checks shrank the page below the requested size, the len(sessions) == limit cursor gate then withheld the next cursor (making later sessions unreachable), and a cut landing inside a session's check rows truncated its factor list.

The query now pages in an inner subquery over sessions alone — filter, cursor, ORDER BY and LIMIT compile into it — and the joins run against that page. This works because every sessionSchema field is a sessions column. The outer statement repeats the ORDER BY, since the joins don't preserve the subquery's order and scanSessions derives session order from row order. Same shape in all three dialects; compileRead is unchanged.

New integration coverage in session_test.go, red without the fix on all assertions:

  • TestSessionStatements_List_LimitBoundsSessions — three two-check sessions paged with limit 2 must fill the page, issue a cursor, and return every session exactly once in ID order.
  • TestSessionStatements_List_LimitKeepsFactorsComplete — a two-check session listed with limit 1 must keep both factors.

The tests are order-independent (every session carries two checks) and avoid the helpers #781 introduces, so the branches merge cleanly in either order.

Verified: sqlite integration suite and untagged unit tests pass locally; postgres and spanner run in CI.

🤖 Generated with Claude Code

grvijayan and others added 2 commits August 7, 2026 17:01
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 15:32
@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nextgen Ready Ready Preview Aug 7, 2026 3:47pm
nextgen-docs Ready Ready Preview Aug 7, 2026 3:47pm
nextgen-mock-zitadel Ready Ready Preview Aug 7, 2026 3:47pm

Request Review

@zitadel-kanban zitadel-kanban Bot moved this to Inbox in Engineering Kanban Aug 7, 2026
@grvijayan grvijayan changed the title fix(storage): make session list LIMIT bound sessions, not joined rows fix: make session list LIMIT bound sessions, not joined rows Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes cursor-based pagination for ListSessions across Postgres, SQLite, and Spanner by ensuring LIMIT applies to sessions (the base table) rather than to joined rows from the one-to-many checks join, preventing undersized pages, withheld next cursors, and truncated factor lists.

Changes:

  • Reworked the per-dialect session list SQL to page via an inner subquery over sessions (with filter/cursor/order/limit), then join user_agents and checks onto that page.
  • Re-applied ORDER BY in the outer query to preserve row order for scanSessions (which derives session order from row iteration order).
  • Added integration tests covering multi-check sessions to ensure page fullness, cursor issuance, no unreachable sessions, and complete factor lists.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
internal/storage/v2/stmttest/session_multicheck_test.go Adds integration coverage proving LIMIT bounds sessions (not joined rows) and factors remain complete under pagination.
internal/storage/v2/dialect/sqlite/session.go Wraps session paging in an inner sessions subquery and applies outer ORDER BY after joins to preserve scan order.
internal/storage/v2/dialect/spanner/session.go Same subquery-based paging shape for Spanner to prevent joined-row limiting and preserve ordering post-join.
internal/storage/v2/dialect/postgres/session.go Same subquery-based paging shape for Postgres to ensure correct session-bounded pagination and complete factor lists.

grvijayan and others added 2 commits August 7, 2026 17:38
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ No Changeset found

Latest commit: ef418bf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

Session list LIMIT bounds joined rows, making sessions unreachable and factor lists incomplete

2 participants