make chunkById() work for non-incrementing/non-integer ids

laravel · Jun 12, 2018 · 10932f8 · 10932f8
1 parent 3162c65
commit 10932f8
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 5 deletions.
diff --git a/src/Illuminate/Database/Eloquent/Builder.php b/src/Illuminate/Database/Eloquent/Builder.php
@@ -622,7 +622,7 @@ public function chunkById($count, callable $callback, $column = null, $alias = n
 
         $alias = is_null($alias) ? $column : $alias;
 
-        $lastId = 0;
+        $lastId = null;
 
         do {
             $clone = clone $this;

diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php
@@ -1805,16 +1805,19 @@ public function forPage($page, $perPage = 15)
      * Constrain the query to the next "page" of results after a given ID.
      *
      * @param  int  $perPage
-     * @param  int  $lastId
+     * @param  int|null  $lastId
      * @param  string  $column
      * @return \Illuminate\Database\Query\Builder|static
      */
     public function forPageAfterId($perPage = 15, $lastId = 0, $column = 'id')
     {
         $this->orders = $this->removeExistingOrdersFor($column);
 
-        return $this->where($column, '>', $lastId)
-                    ->orderBy($column, 'asc')
+        if ($lastId !== null) {
+            $this->where($column, '>', $lastId);
+        }
+
+        return $this->orderBy($column, 'asc')
                     ->take($perPage);
     }
 
@@ -2088,7 +2091,7 @@ public function chunkById($count, callable $callback, $column = 'id', $alias = n
     {
         $alias = $alias ?: $column;
 
-        $lastId = 0;
+        $lastId = null;
 
         do {
             $clone = clone $this;

diff --git a/tests/Database/DatabaseEloquentIntegrationTest.php b/tests/Database/DatabaseEloquentIntegrationTest.php
@@ -366,6 +366,25 @@ public function testCreatingModelWithEmptyAttributes()
         $this->assertFalse($model->wasRecentlyCreated);
     }
 
+    public function testChunkByIdWithNonIncrementingKey()
+    {
+        EloquentTestNonIncrementingSecond::create(['name' => ' First']);
+        EloquentTestNonIncrementingSecond::create(['name' => ' Second']);
+        EloquentTestNonIncrementingSecond::create(['name' => ' Third']);
+
+        $i = 0;
+        EloquentTestNonIncrementingSecond::query()->chunkById(2, function (Collection $users) use (&$i) {
+            if (! $i) {
+                $this->assertEquals(' First', $users[0]->name);
+                $this->assertEquals(' Second', $users[1]->name);
+            } else {
+                $this->assertEquals(' Third', $users[0]->name);
+            }
+            $i++;
+        }, 'name');
+        $this->assertEquals(2, $i);
+    }
+
     public function testPluck()
     {
         EloquentTestUser::create(['id' => 1, 'email' => 'taylorotwell@gmail.com']);
@@ -1307,6 +1326,11 @@ class EloquentTestNonIncrementing extends Eloquent
     public $timestamps = false;
 }
 
+class EloquentTestNonIncrementingSecond extends EloquentTestNonIncrementing
+{
+    protected $connection = 'second_connection';
+}
+
 class EloquentTestUserWithGlobalScope extends EloquentTestUser
 {
     public static function boot()