diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 3dd0f435e..db63ee866 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -1162,7 +1162,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li } } } else { - throw new LogicException("Couldn't find parent for record $fetchedID on $relationType relation $relationName"); + throw new LogicException("Couldn't find parent for record {$fetched->ID} on $relationType relation $relationName"); } } @@ -1321,17 +1321,21 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li } // Get the join records so we can correctly identify which children belong to which parents - // This also holds extra fields data - $fetchedIDsAsString = implode(',', $fetchedIDs); - $joinRows = DB::query( - 'SELECT * FROM "' . $joinTable - // Only get joins relevant for the parent list - . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')' - // Exclude any children that got filtered out - . ' AND ' . $childIDField . ' IN (' . $fetchedIDsAsString . ')' - // Respect sort order of fetched items - . ' ORDER BY FIELD(' . $childIDField . ', ' . $fetchedIDsAsString . ')' - ); + // If there are no parents and no children, skip this to avoid an error (and to skip an unnecessary DB call) + // Note that $joinRows also holds extra fields data + $joinRows = []; + if (!empty($parentIDs) && !empty($fetchedIDs)) { + $fetchedIDsAsString = implode(',', $fetchedIDs); + $joinRows = DB::query( + 'SELECT * FROM "' . $joinTable + // Only get joins relevant for the parent list + . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')' + // Exclude any children that got filtered out + . ' AND ' . $childIDField . ' IN (' . $fetchedIDsAsString . ')' + // Respect sort order of fetched items + . ' ORDER BY FIELD(' . $childIDField . ', ' . $fetchedIDsAsString . ')' + ); + } // Store the children in an EagerLoadedList against the correct parent foreach ($joinRows as $row) { diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 44ce1e36e..7709048a0 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -12,6 +12,7 @@ use SilverStripe\ORM\DataQuery; use SilverStripe\ORM\DB; use SilverStripe\ORM\EagerLoadedList; use SilverStripe\ORM\ManyManyThroughList; +use SilverStripe\ORM\SS_List; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\EagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneSubEagerLoadObject; @@ -754,6 +755,59 @@ class DataListEagerLoadingTest extends SapphireTest return [$results, $selectCount]; } + /** + * @dataProvider provideEagerLoadRelationsEmpty + */ + public function testEagerLoadRelationsEmpty(string $eagerLoadRelation, int $expectedNumQueries): void + { + EagerLoadObject::create(['Title' => 'test object'])->write(); + $dataList = EagerLoadObject::get()->eagerLoad($eagerLoadRelation); + $this->startCountingSelectQueries(); + foreach ($dataList as $record) { + $relation = $record->$eagerLoadRelation(); + if ($relation instanceof SS_List) { + // The list should be an empty eagerloaded list + $this->assertInstanceOf(EagerLoadedList::class, $relation); + $this->assertCount(0, $relation); + } elseif ($relation !== null) { + // There should be no record here + $this->assertSame($relation->ID, 0); + } + } + $numQueries = $this->stopCountingSelectQueries(); + $this->assertSame($expectedNumQueries, $numQueries); + } + + public function provideEagerLoadRelationsEmpty(): array + { + return [ + 'has_one' => [ + 'eagerLoad' => 'HasOneEagerLoadObject', + 'expectedNumQueries' => 2, + ], + 'belongs_to' => [ + 'eagerLoad' => 'BelongsToEagerLoadObject', + 'expectedNumQueries' => 2, + ], + 'has_many' => [ + 'eagerLoad' => 'HasManyEagerLoadObjects', + 'expectedNumQueries' => 2, + ], + 'many_many' => [ + 'eagerLoad' => 'ManyManyEagerLoadObjects', + 'expectedNumQueries' => 2, + ], + 'many_many through' => [ + 'eagerLoad' => 'ManyManyThroughEagerLoadObjects', + 'expectedNumQueries' => 2, + ], + 'belongs_many_many' => [ + 'eagerLoad' => 'BelongsManyManyEagerLoadObjects', + 'expectedNumQueries' => 2, + ], + ]; + } + public function testEagerLoadFourthLevelException(): void { $eagerLoadRelation = implode('.', [