Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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

Open
bizley wants to merge 3 commits into api-platform:main
base: main
Choose a base branch
Loading
from bizley:main

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

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 reacted with thumbs up emoji

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?

Copy link
Member

soyuka commented Mar 25, 2025

bizley reacted with thumbs up emoji

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.

Copy link
Member

soyuka commented Apr 2, 2025

first guess: FilterEagerLoadingExtension

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

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.

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.

Copy link
Member

soyuka commented Apr 10, 2025

status? should we merge this?

Copy link
Author

bizley commented Apr 10, 2025

Give me few more days till the end of week ;)

soyuka reacted with heart emoji

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

Reviewers

No reviews

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unnecessary joins made by the eager loading extension

2 participants

AltStyle によって変換されたページ (->オリジナル) /