Skip to content

Commit 0497133

Browse files
graylikemeclaude
andcommitted
fix: correct keyset pagination cursor and stable totalCount
The cursor condition used only `id > after_id` but ORDER BY was `(full_name, id)` / `(name, id)`, causing duplicates and skipped items across pages when row IDs didn't match alphabetical sort order. Additionally, COUNT(*) OVER() included the cursor condition, making totalCount shrink on page 2+. Fix: use a proper composite keyset condition `(sort_col > $val OR (sort_col = $val AND id > $id))` and wrap the base query in a MATERIALIZED CTE when a cursor is present so the window-function count reflects all filtered rows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4770b0e commit 0497133

3 files changed

Lines changed: 54 additions & 23 deletions

File tree

crates/api/src/db/equipment.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub async fn search(
3535
pool: &PgPool,
3636
filter: EquipmentFilter<'_>,
3737
first: i64,
38-
after_id: Option<i32>,
38+
after_cursor: Option<(&str, i32)>,
3939
) -> Result<(Vec<DbEquipment>, i64, bool), AppError> {
4040
// Pre-resolve ammo_for_slug to ID
4141
let resolved_ammo_for_id = if let Some(slug) = filter.ammo_for_slug {
@@ -47,7 +47,15 @@ pub async fn search(
4747
None
4848
};
4949

50-
let mut builder = sqlx::QueryBuilder::<sqlx::Postgres>::new(
50+
let has_cursor = after_cursor.is_some();
51+
52+
let mut builder = sqlx::QueryBuilder::<sqlx::Postgres>::new("");
53+
54+
if has_cursor {
55+
builder.push("WITH filtered AS MATERIALIZED (");
56+
}
57+
58+
builder.push(
5159
r#"SELECT id, slug, name, category::text AS category, tech_base::text AS tech_base,
5260
rules_level::text AS rules_level, tonnage, crits, damage, heat,
5361
range_min, range_short, range_medium, range_long, bv, intro_year,
@@ -91,9 +99,15 @@ pub async fn search(
9199
builder.push(" AND ammo_for_id = ");
92100
builder.push_bind(weapon_id);
93101
}
94-
if let Some(aid) = after_id {
102+
103+
if let Some((sort_val, after_id)) = after_cursor {
104+
builder.push(") SELECT * FROM filtered WHERE (name > ");
105+
builder.push_bind(sort_val);
106+
builder.push(" OR (name = ");
107+
builder.push_bind(sort_val);
95108
builder.push(" AND id > ");
96-
builder.push_bind(aid);
109+
builder.push_bind(after_id);
110+
builder.push("))");
97111
}
98112

99113
builder.push(" ORDER BY name, id LIMIT ");

crates/api/src/db/units.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,24 @@ pub async fn search(
5959
pool: &PgPool,
6060
filter: UnitFilter<'_>,
6161
first: i64,
62-
after_id: Option<i32>,
62+
after_cursor: Option<(&str, i32)>,
6363
) -> Result<(Vec<DbUnit>, i64, bool), AppError> {
6464
let has_mech_filter = filter.is_omnimech.is_some()
6565
|| filter.config.is_some()
6666
|| filter.engine_type.is_some()
6767
|| filter.has_jump.is_some();
68+
let has_cursor = after_cursor.is_some();
6869

69-
let mut builder = sqlx::QueryBuilder::<sqlx::Postgres>::new(
70+
let mut builder = sqlx::QueryBuilder::<sqlx::Postgres>::new("");
71+
72+
// When paginating with a cursor, wrap the base query in a MATERIALIZED CTE
73+
// so that COUNT(*) OVER() reflects the total filtered rows (not just rows
74+
// after the cursor position).
75+
if has_cursor {
76+
builder.push("WITH filtered AS MATERIALIZED (");
77+
}
78+
79+
builder.push(
7080
r#"SELECT u.id, u.slug, u.chassis_id, u.variant, u.full_name,
7181
u.tech_base::text AS tech_base, u.rules_level::text AS rules_level,
7282
u.tonnage, u.bv, u.cost, u.intro_year, u.extinction_year,
@@ -144,12 +154,20 @@ pub async fn search(
144154
builder.push(" AND u.role = ");
145155
builder.push_bind(role);
146156
}
147-
if let Some(aid) = after_id {
148-
builder.push(" AND u.id > ");
149-
builder.push_bind(aid);
157+
158+
// Close CTE and apply keyset cursor condition in the outer query.
159+
// The cursor uses (full_name, id) to match the ORDER BY columns.
160+
if let Some((sort_val, after_id)) = after_cursor {
161+
builder.push(") SELECT * FROM filtered WHERE (full_name > ");
162+
builder.push_bind(sort_val);
163+
builder.push(" OR (full_name = ");
164+
builder.push_bind(sort_val);
165+
builder.push(" AND id > ");
166+
builder.push_bind(after_id);
167+
builder.push("))");
150168
}
151169

152-
builder.push(" ORDER BY u.full_name, u.id LIMIT ");
170+
builder.push(" ORDER BY full_name, id LIMIT ");
153171
builder.push_bind(first + 1);
154172

155173
let mut rows = builder

crates/api/src/graphql/query.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,7 @@ impl QueryRoot {
154154
) -> Result<UnitConnection, AppError> {
155155
let state = ctx.data::<AppState>().unwrap();
156156
let first = first.unwrap_or(20).clamp(1, 100) as i64;
157-
let after_id = after
158-
.as_deref()
159-
.and_then(decode_cursor)
160-
.map(|(_, id)| id);
157+
let after_cursor = after.as_deref().and_then(decode_cursor);
161158

162159
let filter = units::UnitFilter {
163160
name_search: name_search.as_deref(),
@@ -174,8 +171,13 @@ impl QueryRoot {
174171
role: role.as_deref(),
175172
};
176173

177-
let (rows, total_count, has_next) =
178-
units::search(&state.pool, filter, first, after_id).await?;
174+
let (rows, total_count, has_next) = units::search(
175+
&state.pool,
176+
filter,
177+
first,
178+
after_cursor.as_ref().map(|(s, id)| (s.as_str(), *id)),
179+
)
180+
.await?;
179181

180182
let edges: Vec<UnitEdge> = rows
181183
.into_iter()
@@ -195,7 +197,7 @@ impl QueryRoot {
195197
edges,
196198
page_info: PageInfo {
197199
has_next_page: has_next,
198-
has_previous_page: after_id.is_some(),
200+
has_previous_page: after_cursor.is_some(),
199201
start_cursor,
200202
end_cursor,
201203
total_count,
@@ -259,10 +261,7 @@ impl QueryRoot {
259261
) -> Result<EquipmentConnection, AppError> {
260262
let state = ctx.data::<AppState>().unwrap();
261263
let first = first.unwrap_or(20).clamp(1, 100) as i64;
262-
let after_id = after
263-
.as_deref()
264-
.and_then(decode_cursor)
265-
.map(|(_, id)| id);
264+
let after_cursor = after.as_deref().and_then(decode_cursor);
266265

267266
let filter = equipment::EquipmentFilter {
268267
name_search: name_search.as_deref(),
@@ -279,7 +278,7 @@ impl QueryRoot {
279278
&state.pool,
280279
filter,
281280
first,
282-
after_id,
281+
after_cursor.as_ref().map(|(s, id)| (s.as_str(), *id)),
283282
)
284283
.await?;
285284

@@ -301,7 +300,7 @@ impl QueryRoot {
301300
edges,
302301
page_info: PageInfo {
303302
has_next_page: has_next,
304-
has_previous_page: after_id.is_some(),
303+
has_previous_page: after_cursor.is_some(),
305304
start_cursor,
306305
end_cursor,
307306
total_count,

0 commit comments

Comments
 (0)