Skip to content
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.

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

feat(doctrine): optimize EagerLoadingExtension to skip unnecessary joins #7039

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bizley
Copy link

@bizley bizley commented Mar 21, 2025

Q A
Branch? main for features / current stable version branch for bug fixes
Tickets Closes #7036 (WIP)
License MIT
Doc PR api-platform/docs#... (not yet)

This is a quick check if anything breaks after this optimization. Tests TBD

@soyuka
Copy link
Member

soyuka commented Mar 21, 2025

interetsing, there's only one failure at features/doctrine/eager_loading.feature:64 I guess it's because of the property filter (it adds attributes to the serializer groups), maybe that the code needs a small adjustment?

@bizley
Copy link
Author

bizley commented Mar 22, 2025

Ok, looks like I made it green. Now to make tests proving it generates fewer joins. Should I proceed? Any tips?

@soyuka
Copy link
Member

soyuka commented Mar 25, 2025

really nice, the easiest is to add a feature inside:

https://github.com/api-platform/core/blob/main/features/doctrine/eager_loading.feature

Fixtures are located at https://github.com/api-platform/core/tree/main/tests/Fixtures/TestBundle/Entity

Thanks!

@bizley
Copy link
Author

bizley commented Apr 2, 2025

Just for an update on this - we encountered heavy db problems in our app for some custom filters execution so I'm checking if this is just a problem on our side or this change would do that for everyone, which of course we really don't want to happen.

@soyuka
Copy link
Member

soyuka commented Apr 2, 2025

first guess: FilterEagerLoadingExtension

did you analyse the queries? do you need many joins?

@bizley
Copy link
Author

bizley commented Apr 3, 2025

Probably, yes, this one. We have changed prio of EagerLoadingExtension to -19 and FilterEagerLoadingExtension to -18. Generated query had two nested SELECTs so terrible in general. Analyzing.

@soyuka
Copy link
Member

soyuka commented Apr 3, 2025

yes its also why we recommend to define your own queries as we're quite limited in how to we can generate automatic queries, you can also use the LinksHandler for these kind of use cases.

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.

Unnecessary joins made by the eager loading extension
2 participants