Skip to content

Fix incorrect next-page offset calculation#115

Merged
luisina-santos merged 1 commit intomainfrom
fix/offset-pagination-calculation
Apr 1, 2026
Merged

Fix incorrect next-page offset calculation#115
luisina-santos merged 1 commit intomainfrom
fix/offset-pagination-calculation

Conversation

@c1-dev-bot
Copy link
Copy Markdown
Contributor

@c1-dev-bot c1-dev-bot bot commented Mar 27, 2026

Summary

  • Fix offset pagination in pkg/bsql/query.go where pCtx.Offset was incorrectly treated as a page number instead of a row offset
  • Changed pCtx.Offset * pageSize + pageSize to pCtx.Offset + pageSize
  • Previously, pagination broke after 200 records (offset jumped from 200 to 10,100 with pageSize=100)

Test plan

  • All existing tests pass (go test ./...)
  • Verify pagination works correctly beyond 200 records with an offset-based SQL query

Fixes CXH-1261

pCtx.Offset is already a row offset, not a page number. The previous
formula `pCtx.Offset * pageSize + pageSize` treated it as a page index,
causing the offset to jump from 200 to 10100 after the second page
(with pageSize=100), which broke pagination beyond 200 records.

The correct formula is simply `pCtx.Offset + pageSize`.

Fixes CXH-1261
@c1-dev-bot c1-dev-bot bot requested a review from a team March 27, 2026 18:24
@linear
Copy link
Copy Markdown

linear bot commented Mar 27, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Claude finished @c1-dev-bot[bot]'s task in 53s —— View job


PR Review: Fix incorrect next-page offset calculation

  • Gather context and read changed files
  • Analyze the fix correctness
  • Post review feedback

Verdict: Approve

The fix is correct. Here's the trace confirming the bug and the fix:

In setupPagination (line 274), the pagination token is parsed directly as a row offset:

offset, err := strconv.ParseInt(pToken.Token, 10, 64)
ret.Offset = offset  // This IS a row offset, not a page number

Before (broken): pCtx.Offset * pageSize + pageSize at pkg/bsql/query.go:223

Page Token Offset Next token
1 "" 0 0*100+100 = 100
2 "100" 100 100*100+100 = 10100
3 "10100" 10100 10100*100+100 = 1010100

After (fixed): pCtx.Offset + pageSize

Page Token Offset Next token
1 "" 0 0+100 = 100
2 "100" 100 100+100 = 200
3 "200" 200 200+100 = 300

The old formula would have been correct if Offset were a zero-based page index (0, 1, 2...), but since setupPagination stores the raw row offset from the token, the multiplication was wrong.

One observation: There are no unit tests covering nextPageToken with the offset strategy. Adding a test would prevent regression. Here's a quick way to do that:

Add offset pagination test →

Overall this is a clean, minimal one-line fix for a real bug. LGTM.

@btipling btipling removed their assignment Apr 1, 2026
@luisina-santos luisina-santos merged commit 9ed4c70 into main Apr 1, 2026
8 checks passed
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.

2 participants