Skip to content

findAll and findBy should return list#498

Closed
stchr wants to merge 2 commits intodoctrine:4.1.xfrom
stchr:patch-1
Closed

findAll and findBy should return list#498
stchr wants to merge 2 commits intodoctrine:4.1.xfrom
stchr:patch-1

Conversation

@stchr
Copy link
Copy Markdown

@stchr stchr commented Feb 4, 2026

@stchr stchr marked this pull request as ready for review February 4, 2026 08:49
@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Feb 4, 2026

This kind of change should target 4.2.x

@GromNaN
Copy link
Copy Markdown
Member

GromNaN commented Feb 4, 2026

This kind of change should target 4.2.x

I agree.

This is an additional constraint added to this methods. Do all implementations in Doctrine projects return a list?

The list type must be also in Doctrine ODM EntityPersister::loadAll() that is called by EntityRepository::findBy()

This needs to be explored, it might be an indexed array in certain conditions?
https://github.com/doctrine/orm/blob/4262eb495b4d2a53b45de1ac58881e0091f2970f/src/Internal/Hydration/ObjectHydrator.php#L508

@stof
Copy link
Copy Markdown
Member

stof commented Feb 4, 2026

@GromNaN the repository methods don't allow asking for indexed results (the ObjectHydrator indeed does not always return a list, it depends on the result set mapping).

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Feb 5, 2026

Then I guess the ORM types should be fixed? If yes, targeting the patch branch of ORM would be the correct thing to do.

@stof
Copy link
Copy Markdown
Member

stof commented Feb 5, 2026

@greg0ire why ? The findAll and findBy methods of the repository always return a list (as they don't allow requesting indexed results). The ObjectHydrator is not documented as returning a list in the ORM.

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Feb 5, 2026

Sorry, I got confused, I thought you were implying the hydrator and the entity methods were connected indirectly, and did not check that. Or maybe they are connected, but the way one uses the other, a list is always returned?

@stchr
Copy link
Copy Markdown
Author

stchr commented Feb 5, 2026

Thanks for all the input, highly appreciated!
To summarize if I understand correctly: everything is fine, just need to target 4.2.x branch, right?

@stchr
Copy link
Copy Markdown
Author

stchr commented Feb 5, 2026

closed in favor of #499, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants