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

Add/fix support for distinct selects when using pagination #4088

Merged
merged 4 commits into from
May 30, 2014

Conversation

uzegonemad
Copy link
Contributor

This adds/fixes the ability to use select()->distinct()->paginate(). Previously, the count would be off because the count query disregarded distinct().

Fixes #3191

@taylorotwell
Copy link
Member

I need better examples of what was fixed here. Post the incorrect SQL that was generated before and the Eloquent or query builder query used to generate it. Then post the correct SQL that is generated now.

// We have to check if the value is "null" so that the count function does
// not attempt to count an invalid string. Checking the value is better
// here because the count function already has an optional parameter.
if ( is_null($columns))
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the extra space: if (is_null($columns)).

@uzegonemad
Copy link
Contributor Author

@taylorotwell Good point - that is admittedly pretty vague.

Eloquent:

Header::select('query_serial_num', 'serial_num', 'motor_model_no')
      ->distinct()
      ->where('query_serial_num', 'like', $serial.'%')
      ->paginate(self::RESULTS_PER_PAGE);

Before fix:

# Count Query
select count(*) as aggregate from `header` where `query_serial_num` like ?

# Results Query
select distinct `query_serial_num`, `serial_num`, `motor_model_no` from `header` where `query_serial_num` like ? limit 50 offset 0

After fix:

# Count Query (changed)
select count(distinct `query_serial_num`, `serial_num`, `motor_model_no`) as aggregate from `header` where `query_serial_num` like ?

# Results Query (unchanged)
select distinct `query_serial_num`, `serial_num`, `motor_model_no` from `header` where `query_serial_num` like ? limit 50 offset 0

@swt83
Copy link

swt83 commented Apr 18, 2014

I have this issue also.

@myquote-dev
Copy link

We would also love to see this problem fixed! A brief test of uzegonemad's changes shows they do indeed seem to fix it.

With the current version of laravel (4.1) we are having to work around this problem by resorting to using DB::raw() queries for accurate pagination results when distinct() is used.

Hopefully a change can make it into 4.2!

@uzegonemad
Copy link
Contributor Author

@taylorotwell I understand you're busy but do please let me know if there's any other information you need from me. Thanks!

@taylorotwell taylorotwell merged commit cf47292 into laravel:4.1 May 30, 2014
@taylorotwell
Copy link
Member

This broke the following situation and had to be reverted.

http://laravel.io/bin/rWexv

@uzegonemad
Copy link
Contributor Author

Thanks for the heads up, maybe you have a better solution to the problem?

@Lambik
Copy link

Lambik commented Mar 31, 2016

I know this is very old, but I still seem to have the same issue in laravel 5.1.
Will there ever be a valid fix? The pastebin link is broken so I don't know what broke and caused the revert.

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.

6 participants