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

Deprecate WITH in joined associations #10979

Closed
wants to merge 3 commits into from

Conversation

beberlei
Copy link
Member

@beberlei beberlei commented Oct 9, 2023

Deprecates JOIN path.association WITH type of expressions as they create persistent collections with a subset of related entities and look like they are fully loaded, causing all kinds of trouble.

A user could potentially work around this and move the condition to the WHERE clause. I checked the code and its not really possible in this case to detect it without adding tons of more code. So the WITH deprecation is what we can do about this.

Fixes #10978

Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/issues/10978',
'WITH join conditions are deprecated for joins on associations. Use either a sub-select with another occurance of this table or use Collection::matching(Criteria) in follow-up code.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'WITH join conditions are deprecated for joins on associations. Use either a sub-select with another occurance of this table or use Collection::matching(Criteria) in follow-up code.'
'WITH join conditions are deprecated for joins on associations. Use either a sub-select with another occurrence of this table or use Collection::matching(Criteria) in follow-up code.'

@mpdude
Copy link
Contributor

mpdude commented Oct 10, 2023

Looks good regarding the WITH keyword in JoinAssociation parts of the DQL.

In order to fix #10978, we additionally need to take care of WHERE expressions referring to fetch-join aliases.

@beberlei
Copy link
Member Author

@mpdude ah yes, I checked that - and its not possible, because you don't really know how its used.

@beberlei beberlei closed this Oct 10, 2023
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