Skip to content

Commit 2c4fa12

Browse files
aarondfrancisclaude
andcommitted
Fix #73: Add default order by primary key when no order is specified
When using fastPaginate without an explicit ORDER BY clause, MySQL may return rows in an unpredictable order, which can cause inconsistent pagination results (e.g., missing the first row). This fix adds a default ORDER BY on the primary key when no explicit order is specified, ensuring deterministic results. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 9eaf5a3 commit 2c4fa12

File tree

3 files changed

+41
-13
lines changed

3 files changed

+41
-13
lines changed

src/FastPaginate.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ protected function paginate(string $paginationMethod, Closure $paginatorOutput)
6969
return $this->{$paginationMethod}($perPage, $columns, $pageName, $page);
7070
}
7171

72+
// If no order is specified, we need to add a default order by
73+
// primary key to ensure deterministic results. Without this,
74+
// MySQL may return rows in an unpredictable order.
75+
// https://github.com/aarondfrancis/fast-paginate/issues/73
76+
if (empty($base->orders)) {
77+
$this->orderBy("$table.$key");
78+
}
79+
7280
// This is the copy of the query that becomes
7381
// the inner query that selects keys only.
7482
$paginator = $this->clone()

tests/Integration/BuilderTest.php

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function basic_test()
3838
$this->assertCount(3, $queries);
3939

4040
$this->assertEquals(
41-
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) limit 16 offset 0',
41+
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) order by `users`.`id` asc limit 16 offset 0',
4242
$queries[2]['query']
4343
);
4444

@@ -58,7 +58,7 @@ public function different_page_size()
5858
$this->assertEquals(5, $results->count());
5959

6060
$this->assertEquals(
61-
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5) limit 6 offset 0',
61+
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5) order by `users`.`id` asc limit 6 offset 0',
6262
$queries[2]['query']
6363
);
6464

@@ -78,7 +78,7 @@ public function page_2()
7878
$this->assertEquals(5, $results->count());
7979

8080
$this->assertEquals(
81-
'select * from `users` where `users`.`id` in (6, 7, 8, 9, 10) limit 6 offset 0',
81+
'select * from `users` where `users`.`id` in (6, 7, 8, 9, 10) order by `users`.`id` asc limit 6 offset 0',
8282
$queries[2]['query']
8383
);
8484

@@ -98,7 +98,7 @@ public function pk_attribute_mutations_are_skipped()
9898
$this->assertEquals(5, $results->count());
9999

100100
$this->assertEquals(
101-
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5) limit 6 offset 0',
101+
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5) order by `users`.`id` asc limit 6 offset 0',
102102
$queries[2]['query']
103103
);
104104
}
@@ -114,7 +114,7 @@ public function custom_page_is_preserved()
114114
$this->assertEquals(2, $results->count());
115115

116116
$this->assertEquals(
117-
'select * from `users` where `users`.`id` in (1, 2) limit 3 offset 0',
117+
'select * from `users` where `users`.`id` in (1, 2) order by `users`.`id` asc limit 3 offset 0',
118118
$queries[2]['query']
119119
);
120120

@@ -150,6 +150,26 @@ public function custom_table_is_preserved()
150150
UserCustomTable::query()->fastPaginate();
151151
}
152152

153+
#[Test]
154+
public function default_order_by_primary_key_when_no_order_specified()
155+
{
156+
$queries = $this->withQueriesLogged(function () use (&$results) {
157+
$results = User::query()->fastPaginate(5);
158+
});
159+
160+
// Inner query should have order by primary key to ensure deterministic results
161+
$this->assertEquals(
162+
'select `users`.`id` from `users` order by `users`.`id` asc limit 5 offset 0',
163+
$queries[1]['query']
164+
);
165+
166+
// Outer query should also preserve the order
167+
$this->assertEquals(
168+
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5) order by `users`.`id` asc limit 6 offset 0',
169+
$queries[2]['query']
170+
);
171+
}
172+
153173
#[Test]
154174
public function order_is_propagated()
155175
{
@@ -211,15 +231,15 @@ public function selects_are_overwritten()
211231
$results = User::query()->selectRaw('(select 1 as complicated_subquery)')->fastPaginate();
212232
});
213233

214-
// Dropped for our inner query
234+
// Dropped for our inner query (default order by primary key is added)
215235
$this->assertEquals(
216-
'select `users`.`id` from `users` limit 15 offset 0',
236+
'select `users`.`id` from `users` order by `users`.`id` asc limit 15 offset 0',
217237
$queries[1]['query']
218238
);
219239

220-
// Restored for the user's query
240+
// Restored for the user's query (with default order)
221241
$this->assertEquals(
222-
'select (select 1 as complicated_subquery) from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) limit 16 offset 0',
242+
'select (select 1 as complicated_subquery) from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) order by `users`.`id` asc limit 16 offset 0',
223243
$queries[2]['query']
224244
);
225245
}
@@ -363,7 +383,7 @@ public function basic_simple_test()
363383
$this->assertCount(2, $queries);
364384

365385
$this->assertEquals(
366-
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) limit 16 offset 0',
386+
'select * from `users` where `users`.`id` in (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15) order by `users`.`id` asc limit 16 offset 0',
367387
$queries[1]['query']
368388
);
369389

@@ -385,7 +405,7 @@ public function basic_simple_test_page_two()
385405
$this->assertCount(2, $queries);
386406

387407
$this->assertEquals(
388-
'select * from `users` where `users`.`id` in (6, 7, 8, 9, 10) limit 6 offset 0',
408+
'select * from `users` where `users`.`id` in (6, 7, 8, 9, 10) order by `users`.`id` asc limit 6 offset 0',
389409
$queries[1]['query']
390410
);
391411

@@ -407,7 +427,7 @@ public function basic_simple_test_from_relation()
407427
$this->assertCount(3, $queries);
408428

409429
$this->assertEquals(
410-
'select * from `posts` where `posts`.`user_id` = ? and `posts`.`user_id` is not null and `posts`.`id` in (1) limit 16 offset 0',
430+
'select * from `posts` where `posts`.`user_id` = ? and `posts`.`user_id` is not null and `posts`.`id` in (1) order by `posts`.`id` asc limit 16 offset 0',
411431
$queries[2]['query']
412432
);
413433

tests/Integration/RelationTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function basic_test()
2020
});
2121

2222
$this->assertEquals(
23-
'select * from `posts` where `posts`.`user_id` = ? and `posts`.`user_id` is not null and `posts`.`id` in (1) limit 16 offset 0',
23+
'select * from `posts` where `posts`.`user_id` = ? and `posts`.`user_id` is not null and `posts`.`id` in (1) order by `posts`.`id` asc limit 16 offset 0',
2424
$queries[2]['query']
2525
);
2626
}

0 commit comments

Comments
 (0)