Skip to content

avoid returning the same id multiple times in filter query of smart content #4067

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

Merged
merged 4 commits into from
Jul 18, 2018

Conversation

danrot
Copy link
Contributor

@danrot danrot commented Jul 17, 2018

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #issuenum
Related issues/PRs #issuenum
License MIT
Documentation PR sulu/sulu-docs#prnum

What's in this PR?

This PR avoids returning the same ID multiple times in the ID query of the smart content.

Why?

If a media has assigned Category A and B and a SmartContent using the media data provider has an or filter on Category A and B, then the ID query from the DataProviderRepositoryTrait returns the same media ID twice. That only matters if there is also a limit (either in the smart content overlay or using the max_per_page param in the template XML), because then less items will be returned in the final query.

@danrot danrot added this to the Release 1.6 milestone Jul 17, 2018
@danrot danrot force-pushed the hotfix/smart-content-limit branch from a4f92e3 to cf77f87 Compare July 17, 2018 16:30
@@ -60,6 +60,7 @@ private function findByFiltersIds($filters, $page, $pageSize, $limit, $locale, $

$queryBuilder = $this->createQueryBuilder('c')
->select('c.id')
->distinct()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chirimoya This was the easiest solution we could find. Another solution would be groupBy('id'). I have read that this might be faster in some situations, but I've used an EXPLAIN statement to check the execution plan, and they were identical for both solutions. So I decided to go with distinct, because it explains better what is actually happening here.

@danrot danrot changed the title avoid returning the same id multiple times in filter query of smart c… avoid returning the same id multiple times in filter query of smart content Jul 17, 2018
@danrot danrot force-pushed the hotfix/smart-content-limit branch from aa0820e to 608d6bd Compare July 18, 2018 06:44
@danrot danrot force-pushed the hotfix/smart-content-limit branch from 7b1f4a0 to 0402d89 Compare July 18, 2018 07:22
@danrot danrot force-pushed the hotfix/smart-content-limit branch from 0402d89 to cc58b55 Compare July 18, 2018 08:15
@chirimoya chirimoya merged commit fdecbf3 into sulu:release/1.5 Jul 18, 2018
@danrot danrot deleted the hotfix/smart-content-limit branch July 18, 2018 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants