count(*) problem with postgres. about will_paginate HOT 12 OPEN

Thanks for detail analysis for this issue. I knew about it before, but I needed someone to dissect it for me. Also, I never really had the motivation to fix it inside will_paginate since it's clearly an ActiveRecord issue. Putting too much assumptions about SQL inside will_paginate is not where I want to go.

Can you check how ActiveRecord 3.0 handles these cases? Because it's built on Arel now, so it must understand relational logic better. I'm interested if it's possible to build a scope with "distinct" (like here) and then call count and perform a limited find all without any special parameters or workarounds.

from will_paginate.

I can comment on this and potentially offer a patch (or the start of a patch).

I had this same issue with Microsoft's SQL Server (yea, yea, I know, but I have to for work). As junaid pointed out, will_paginate re-writes the query for MySQL so that it becomes DISTINCT(table_name.id). It doesn't do that for other databases and I think I found the issue.

MySQL uses the backtick () to denote table names and such. SQL Server, on the other hand, uses "[" and "]" to enclose things and it looks (from junaid's post) that PostgreSQL uses standard double-quotes. In lib/will_paginate/finder.rb (line 201 in version 2.3.12), there's a gsub that removes the backticks from the select part of the sql statement and (if a substitution was performed), it goes on to change the select part of it to be DISTINCT #{klass.table_name}.#{klass.primary_key}`. So, if it doesn't detect any backticks in the original select, then it doesn't replace the select part with a count of the primary key.

This was easily fixed (for SQL Server) by changing the gsub to a regexp including "[" and "]" as well as the backtick (so it's now if options[:select].gsub(/[\[\]\]/, '') =~ /\w+.*/`). I might have to repost that last part since I'm not sure how the escaping will work. Anyway, that line could easily be altered to include the double-quote that PostgreSQL is using to enclose table names and therefore make will_paginate work on PostgreSQL as well.

EDIT: please ignore the backslash before the backtick in the gsub in the last code snippet. If I remove the backslash, it ends code mode at that backtick, but the backslash should be thought of as an escape character there. I'm not completely sure what's going on, but the square brackets need a backslash before them in the regexp, but the backtick shouldn't have one before it.

from will_paginate.

I'm really dying here of need for a proper, tested patch … You guys did great investigative work, that could maybe continue in form of a pull request? ;) Maybe?

from will_paginate.

I think 470dae1 fixes this one too

from will_paginate.

thanks kjg, i will check this and update this ticket.

from will_paginate.

this issue looks quite old, but seems to still exist
I am using will_paginate 3.0.2 with postgres and rails 3.1, it seems to ignore DISTINCT

Table1.joins("LEFT OUTER JOIN table2 ON ...").paginate(:page => @params[:page])
pagination: SELECT COUNT(*) FROM "table1" LEFT OUTER ...
real query: SELECT DISTINCT table1.*, table2.* FROM "table1" LEFT OUTER JOIN table2...

so the pagination is coming up with more records than my DISTINCT query does, the fix from kjg looked like it should do what I want, but the code base no longer has a finder.rb file, I guess I'll take a look at lib/will_paginate/active_record.rb

from will_paginate.

just came up with a dirty workaround, might help someone else

query = Foo.joins("LEFT OUTER JOIN bars ON bars.id = foos.id").where(etc....).select("DISTINCT foos.*, bars.*")
Foo.paginate_by_sql(query.to_sql, :page => @params[:page])

the problem does not exist in paginate_by_sql

from will_paginate.

Maybe something like this will help?: "select count(*) from (#{collection.to_sql}) count_sql"

from will_paginate.

house9 -- many thanks, this solved it (at least until a real fix is in place -- I suspect this is going to get pokey on large datasets).

from will_paginate.

@mislav would you accept a pull request for this?

https://github.com/mislav/will_paginate/blob/master/lib/will_paginate/active_record.rb#L67

rel = rel.apply_finder_options(@wp_count_options) if defined? @wp_count_options
rel.count

i believe #apply_finder_options overwrites any :select option sent to it when rel.count runs, so perhaps wp_count_options should accept :distinct => true and have that passed along such that rel.count :distinct => true

from will_paginate.

Bump

from will_paginate.

@mislav Can you please update the document how to use this new option? It seem that @clifton's patch is merged to the lasted release, but I don't know how to use it.
Thanks.

from will_paginate.