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

Corrected distinct option #126

Merged
merged 1 commit into from
Dec 6, 2016
Merged

Corrected distinct option #126

merged 1 commit into from
Dec 6, 2016

Conversation

jbgomond
Copy link
Contributor

@jbgomond jbgomond commented Apr 8, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes

Fix of the DISTINCT option.

Since 1.3.0 (this commit 861c2a0), the distinct option doesn't have the same effect.

Before 1.3.0

distinct: false
image

distinct: true
image


1.3.3 without correction

distinct: false
image

distinct: true
image


1.3.3 with correction

distinct: false
capture11

distinct: true
image

See doctrine documentation : http://doctrine-orm.readthedocs.org/en/latest/tutorials/pagination.html

@jbgomond jbgomond changed the title Corrected distinct option. Corrected distinct option Apr 8, 2015
@salemgolemugoo
Copy link

I have the same issue. @pilot @stloyd pls check and merge

@afranioce
Copy link

+1

@jbgomond
Copy link
Contributor Author

Waiting for me ?

@dextervip
Copy link

Any updates?

@l3pp4rd
Copy link
Collaborator

l3pp4rd commented Aug 10, 2016

can you rebase?

@W0rma
Copy link
Contributor

W0rma commented Aug 31, 2016

@MJBGO Can you please rebase in order to get this PR merged?

@jbgomond
Copy link
Contributor Author

Yes sorry I'll do it this week

@W0rma
Copy link
Contributor

W0rma commented Nov 25, 2016

Any updates @MJBGO ?

@jbgomond
Copy link
Contributor Author

Yes, very sorry. I'll do it this week for sure :)

@jbgomond jbgomond reopened this Nov 28, 2016
@jbgomond
Copy link
Contributor Author

jbgomond commented Nov 28, 2016

Fresh commit done !

I've corrected the problem like explained in the initial message (I've updated with detailed queries).
Doctrine uses $fetchJoinCollection in order to know if the query is a collection or not (true by default) and modifies the number of query generated (3 if false, 4 if true).

The patch that came with 1.3.0 doesn't use the distinct option entirely as seen on the screenshots.
If you think there's a better solution, tell me :)

@W0rma
Copy link
Contributor

W0rma commented Dec 6, 2016

@l3pp4rd Could you please review?

@l3pp4rd l3pp4rd merged commit 6cedcd4 into KnpLabs:master Dec 6, 2016
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.

7 participants