Skip to content

[6.x] Fix chunked queries not honoring user-defined limits and offsets #11701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jasonvarga merged 4 commits into from
Apr 15, 2025
Merged

[6.x] Fix chunked queries not honoring user-defined limits and offsets #11701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6606fd8
Make the test more easily understandable
duncanmcclean Apr 15, 2025
1db1c1d
Add failing tests
duncanmcclean Apr 15, 2025
dfdb263
Fix chunked queries not honoring user-defined limits and offsets
duncanmcclean Apr 15, 2025
706619f
Apply fix to the eloquent query builder
duncanmcclean Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions src/Query/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,23 +722,32 @@ public function with($relations)
*/
public function chunk($count, callable $callback)
{
$skip = $this->offset;
$remaining = $this->limit;

$page = 1;

do {
// We'll execute the query for the given page and get the results. If there are
// no results we can just break and return from here. When there are results
// we will call the callback with the current chunk of these results here.
$results = $this->forPage($page, $count)->get();
$offset = (($page - 1) * $count) + intval($skip);

$limit = is_null($remaining) ? $count : min($count, $remaining);

if ($limit == 0) {
break;
}

$results = $this->offset($offset)->limit($limit)->get();

$countResults = $results->count();

if ($countResults == 0) {
break;
}

// On each chunk result set, we will pass them to the callback and then let the
// developer take care of everything within the callback, which allows us to
// keep the memory low for spinning through large result sets for working.
if (! is_null($remaining)) {
$remaining = max($remaining - $countResults, 0);
}

if ($callback($results, $page) === false) {
return false;
}
Expand Down
43 changes: 36 additions & 7 deletions src/Query/EloquentQueryBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,23 +464,32 @@ public function chunk($count, callable $callback)
{
$this->enforceOrderBy();

$skip = $this->getOffset();
$remaining = $this->getLimit();

$page = 1;

do {
// We'll execute the query for the given page and get the results. If there are
// no results we can just break and return from here. When there are results
// we will call the callback with the current chunk of these results here.
$results = $this->forPage($page, $count)->get();
$offset = (($page - 1) * $count) + intval($skip);

$limit = is_null($remaining) ? $count : min($count, $remaining);

if ($limit == 0) {
break;
}

$results = $this->offset($offset)->limit($limit)->get();

$countResults = $results->count();

if ($countResults == 0) {
break;
}

// On each chunk result set, we will pass them to the callback and then let the
// developer take care of everything within the callback, which allows us to
// keep the memory low for spinning through large result sets for working.
if (! is_null($remaining)) {
$remaining = max($remaining - $countResults, 0);
}

if ($callback($results, $page) === false) {
return false;
}
Expand All @@ -493,6 +502,26 @@ public function chunk($count, callable $callback)
return true;
}

/**
* Get the "limit" value from the query or null if it's not set.
*
* @return mixed
*/
public function getLimit()
{
return $this->builder->getQuery()->getLimit();
}

/**
* Get the "offset" value from the query or null if it's not set.
*
* @return mixed
*/
public function getOffset()
{
return $this->builder->getQuery()->getOffset();
}

/**
* Query lazily, by chunks of the given size.
*
Expand Down
127 changes: 124 additions & 3 deletions tests/Data/Entries/EntryQueryBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,131 @@ public function entries_are_found_using_chunk()
{
$this->createDummyCollectionAndEntries();

$count = 0;
Entry::query()->chunk(2, function ($entries) use (&$count) {
$this->assertCount($count++ == 0 ? 2 : 1, $entries);
$chunks = 0;

Entry::query()->chunk(2, function ($entries, $page) use (&$chunks) {
if ($page === 1) {
$this->assertCount(2, $entries);
$this->assertEquals(['Post 1', 'Post 2'], $entries->map->title->all());
} else {
$this->assertCount(1, $entries);
$this->assertEquals(['Post 3'], $entries->map->title->all());
}

$chunks++;
});

$this->assertEquals(2, $chunks);
}

#[Test]
public function entries_are_found_using_chunk_with_limits_where_limit_is_less_than_total()
{
$this->createDummyCollectionAndEntries();

$chunks = 0;

Entry::query()->limit(2)->chunk(1, function ($entries, $page) use (&$chunks) {
if ($page === 1) {
$this->assertCount(1, $entries);
$this->assertEquals(['Post 1'], $entries->map->title->all());
} else {
$this->assertCount(1, $entries);
$this->assertEquals(['Post 2'], $entries->map->title->all());
}

$chunks++;
});

$this->assertEquals(2, $chunks);
}

#[Test]
public function entries_are_found_using_chunk_with_limits_where_limit_is_more_than_total()
{
$this->createDummyCollectionAndEntries();

$chunks = 0;

Entry::query()->limit(10)->chunk(2, function ($entries, $page) use (&$chunks) {
if ($page === 1) {
$this->assertCount(2, $entries);
$this->assertEquals(['Post 1', 'Post 2'], $entries->map->title->all());
} elseif ($page === 2) {
$this->assertCount(1, $entries);
$this->assertEquals(['Post 3'], $entries->map->title->all());
} else {
$this->fail('Should have had two pages.');
}

$chunks++;
});

$this->assertEquals(2, $chunks);
}

#[Test]
public function entries_are_found_using_chunk_with_offset()
{
$this->createDummyCollectionAndEntries();

$chunks = 0;

Entry::query()->offset(1)->chunk(2, function ($entries, $page) use (&$chunks) {
if ($page === 1) {
$this->assertCount(2, $entries);
$this->assertEquals(['Post 2', 'Post 3'], $entries->map->title->all());
} else {
$this->fail('Should only have had one page.');
}

$chunks++;
});

$this->assertEquals(1, $chunks);
}

#[Test]
public function entries_are_found_using_chunk_with_offset_where_more_than_total()
{
$this->createDummyCollectionAndEntries();

$chunks = 0;

Entry::query()->offset(3)->chunk(2, function ($entries, $page) use (&$chunks) {
$chunks++;
});

$this->assertEquals(0, $chunks);
}

#[Test]
public function entries_are_found_using_chunk_with_limits_and_offsets()
{
$this->createDummyCollectionAndEntries();

EntryFactory::id('id-4')->slug('post-4')->collection('posts')->data(['title' => 'Post 4'])->create();
EntryFactory::id('id-5')->slug('post-5')->collection('posts')->data(['title' => 'Post 5'])->create();
EntryFactory::id('id-6')->slug('post-6')->collection('posts')->data(['title' => 'Post 6'])->create();
EntryFactory::id('id-7')->slug('post-7')->collection('posts')->data(['title' => 'Post 7'])->create();

$chunks = 0;

Entry::query()->orderBy('id', 'asc')->offset(2)->limit(3)->chunk(2, function ($entries, $page) use (&$chunks) {
if ($page === 1) {
$this->assertCount(2, $entries);
$this->assertEquals(['Post 3', 'Post 4'], $entries->map->title->all());
} elseif ($page === 2) {
$this->assertCount(1, $entries);
$this->assertEquals(['Post 5'], $entries->map->title->all());
} else {
$this->fail('Should only have had two pages.');
}

$chunks++;
});

$this->assertEquals(2, $chunks);
}

#[Test]
Expand Down