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

Use count(:all) for AR #36

Closed
espen opened this issue May 31, 2018 · 4 comments
Closed

Use count(:all) for AR #36

espen opened this issue May 31, 2018 · 4 comments

Comments

@espen
Copy link
Contributor

espen commented May 31, 2018

When specifying which columns to retrieve using .select in Active Record it causes an issue when using .count as it will use all the columns from the select. Instead '*' should be used by setting :all.

Code example: https://github.com/espen/pagy_count_bug/blob/master/app/controllers/users_controller.rb

Failing test: https://github.com/espen/pagy_count_bug/blob/master/test/controllers/users_controller_test.rb

Error:
UsersControllerTest#test_shows_users:
ActiveRecord::StatementInvalid: PG::UndefinedFunction: ERROR:  function count(bigint, character varying) does not exist
LINE 1: SELECT COUNT(id, name) FROM "users"
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
: SELECT COUNT(id, name) FROM "users"
    app/controllers/users_controller.rb:6:in `index'
    test/controllers/users_controller_test.rb:5:in `block in <class:UsersControllerTest>'

Suggestion, use .count(:all) for AR in https://github.com/ddnexus/pagy/blob/master/lib/pagy/backend.rb#L21

@ddnexus
Copy link
Owner

ddnexus commented May 31, 2018

PostgreSQL?

@espen
Copy link
Contributor Author

espen commented May 31, 2018

Yes, I haven't tried MySQL so not sure if it differs. It works with SQLite but haven't checked what is going on there.

@ddnexus
Copy link
Owner

ddnexus commented May 31, 2018

MySQL works... it's a known issue of PG, however it looks like the change wouldn't break anything.

@ddnexus
Copy link
Owner

ddnexus commented Jun 1, 2018

pushed to rubygem (v0.8.2)

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

No branches or pull requests

2 participants