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

distinct() updates QueryBuilder state correctly #10789

Merged

Conversation

macroparts
Copy link

Previously calling distinct() when the QueryBuilder was in clean state would cause subsequent getDQL() calls to ignore the distinct queryPart

Fixes #10784

@greg0ire greg0ire added the Bug label Jun 23, 2023
@greg0ire greg0ire self-requested a review June 23, 2023 09:50
@greg0ire greg0ire changed the base branch from 2.14.x to 2.15.x June 23, 2023 09:50
@greg0ire
Copy link
Member

2.14.x is no longer maintained. I retargeted on 2.15.x, and now there is a conflict.

@macroparts macroparts force-pushed the distinct-updates-state-correctly branch from eec49b5 to 73237c2 Compare June 23, 2023 13:05
@macroparts
Copy link
Author

Conflicts are resolved now.

@greg0ire
Copy link
Member

The build failure seems unrelated to your changes 🤔

@greg0ire
Copy link
Member

slevomat/coding-standard published a minor release 4 hours ago, that's why.

@macroparts
Copy link
Author

Should I fix those in my PR or are these fixed separately?

@greg0ire
Copy link
Member

greg0ire commented Jun 23, 2023

I don't think you can fix them: slevomat/coding-standard#1585
The best course of action might be to add a version constraint preventing 8.13.0 in a separate PR.

@macroparts
Copy link
Author

Their maintainers might provide a fix for the broken comment parsing in their latest release soon. I'll leave it for now and check back on Monday.

@greg0ire
Copy link
Member

greg0ire commented Jun 23, 2023

They are usually quite swift, so yeah, maybe. Also, it might be possible to work around some of the issues (use @param instead of @psalm-param, avoid @\, etc.

@greg0ire
Copy link
Member

Addressed in #10790

Previously calling distinct() when the QueryBuilder was in clean state would cause subsequent getDQL() calls to ignore the distinct queryPart

Fixes doctrine#10784
@greg0ire greg0ire force-pushed the distinct-updates-state-correctly branch from 73237c2 to efb50b9 Compare June 25, 2023 15:54
@derrabus derrabus added this to the 2.15.4 milestone Jun 25, 2023
@greg0ire greg0ire merged commit 4e13890 into doctrine:2.15.x Jun 25, 2023
@greg0ire
Copy link
Member

Thanks @macroparts !

@macroparts
Copy link
Author

Thank you @greg0ire !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling distinct() on QueryBuilder does not update state correctly
4 participants