Skip to content

Don't use fetchJoinCollection on Doctrine Paginator when not needed#2639

Merged
teohhanhui merged 1 commit into
api-platform:2.4from
teohhanhui:fix/doctrine-paginator-fetch-join-collection
Mar 29, 2019
Merged

Don't use fetchJoinCollection on Doctrine Paginator when not needed#2639
teohhanhui merged 1 commit into
api-platform:2.4from
teohhanhui:fix/doctrine-paginator-fetch-join-collection

Conversation

@teohhanhui

@teohhanhui teohhanhui commented Mar 22, 2019

Copy link
Copy Markdown
Contributor
Q A
Bug fix? yes?
New feature? no?
BC breaks? no?
Deprecations? yes
Tests pass? yes
Fixed tickets #2611 (comment), #2611 (comment)
License MIT
Doc PR api-platform/docs#771

Refactor QueryChecker etc.

I've removed the flawed handling of "join by class", as we simply have not enough information to do the right thing there. The main caveat (as was before) is that our checking covers the "80%":

  • join by association
  • no complex expression in SELECT / ORDER BY clauses

but will be confused in the remaining "20%".

For example:

🗹 Supported:

SELECT d, a_1
FROM Dummy d
LEFT JOIN d.relatedDummies a_1
ORDER BY a_1.name

🗷 Not supported:

SELECT d, a_1
FROM Dummy d
LEFT JOIN RelatedDummy a_1 WITH 1 = 1
ORDER BY a_1.name

🗷 Not supported:

SELECT d, LENGTH(a_1.name) AS HIDDEN score
FROM Dummy d
LEFT JOIN d.relatedDummies a_1
ORDER BY score

@teohhanhui
teohhanhui force-pushed the fix/doctrine-paginator-fetch-join-collection branch 7 times, most recently from b5c8553 to b0d02c7 Compare March 22, 2019 22:41
@teohhanhui
teohhanhui force-pushed the fix/doctrine-paginator-fetch-join-collection branch 2 times, most recently from 0bd5756 to e54656c Compare March 25, 2019 15:02
@teohhanhui

teohhanhui commented Mar 25, 2019

Copy link
Copy Markdown
Contributor Author

Since we have no way to cover all use cases, and "we don't know when we don't know" (so we can't even fall back to safe defaults, unless we remove the whole guessing altogether), it's important that we let the user configure these in the resource / operation attributes (done in this PR).

I'd argue that our guessing is worth it, because it'd give the best performance for most users out of the box. I think it's consistent with what I perceive as the philosophy of API Platform, which is to make things effortless for the most common use cases. When the user starts doing more advanced stuff, they are then required to take over control.

@teohhanhui
teohhanhui force-pushed the fix/doctrine-paginator-fetch-join-collection branch from aa6e2c7 to 20291cc Compare March 27, 2019 13:19
@teohhanhui
teohhanhui merged commit cb76832 into api-platform:2.4 Mar 29, 2019
@teohhanhui
teohhanhui deleted the fix/doctrine-paginator-fetch-join-collection branch March 29, 2019 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants