khiav223577 / pluck_all

A more efficient way to get data from database. Like #pluck method but return array of hashes instead.
MIT License
102 stars 10 forks source link

Behaviour of pluck_all with select is broken in version 2.0.4 #46

Closed bblimke closed 4 years ago

bblimke commented 4 years ago

Here is how pluck_all behaves in 2.0.3 and how our system expected it to behave

User.limit(1).pluck(:id, :first_name)
SELECT  "users"."id", "users"."first_name" FROM "users" LIMIT 1
=> => [[1, "Tom"]]

User.limit(1).select(:id, :first_name).pluck_all
SELECT  "users"."id", "users"."first_name" FROM "users" LIMIT 1
=> [{"id"=>1, "first_name"=>"Tom"}]

User.limit(1).select("last_name, count(*) as total").group(:last_name).pluck_all
SELECT  last_name, count(*) as total FROM "users" GROUP BY "users"."last_name" LIMIT 1
=> [{"last_name"=>"Smith", "total"=>1}]

This is how version 2.0.4 behaves which breaks things for us

# that still works
User.limit(1).pluck(:id, :first_name)
=> => [[1, "Tom"]]

# this one now returns hashes with all attributes instead of just id and first_name
User.limit(1).select(:id, :first_name).pluck_all
SELECT  "users".* FROM "users" LIMIT 1
=> [{"id"=>1, "first_name"=>"Tom", "last_name"=>"Smith", ...

# this one now breaks completely
User.limit(1).select("last_name, count(*) as total").group(:last_name).pluck_all
SELECT  "users".* FROM "users" GROUP BY "users"."last_name" LIMIT 1
ActiveRecord::StatementInvalid: PG::GroupingError: ERROR:  column "users.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT  "users".* FROM "users" GROUP BY "users"."last_name" ...

@khiav223577 I guess this PR https://github.com/khiav223577/pluck_all/pull/40 caused it?

khiav223577 commented 4 years ago

@bblimke I tested what pluck method behaves in this case, and got the same error, too.

User.limit(1).group(:name).select('last_name, count(*) as total').pluck
ActiveRecord::StatementInvalid: PG::GroupingError: ERROR:  column "users.id" must appear in the GROUP BY clause or be used in an aggregate function

It also returns all attributes instead of just id and first_name when calling pluck with no argument.

User.limit(1).select(:id, :first_name).pluck
# => [1, 'first_name', 'test.email@example.com', nil, ...]

I would like to keep the behavior of pluck_all being the same as pluck. And the return value's format is the only difference between them. Sorry for the unnotified behavior changes.

You could try this:

User.limit(1).group(:last_name).pluck_all("last_name, count(*) as total")

or

users = User.limit(1).select(:id, :first_name)
users.pluck_all(*users.select_values)
bblimke commented 4 years ago

Thank you for the quick reply @khiav223577

It make sense to keep pluck_all compatible with pluck. Not critical, but I wonder if there is a way to improve pluck to be more intuitive when used in combination with select. If pluck_all is to follow pluck, then it's a pluck issue though.

khiav223577 commented 4 years ago

but I wonder if there is a way to improve pluck to be more intuitive when used in combination with select.

@bblimke I find an issue in rails that may relate to your question. See: https://github.com/rails/rails/issues/12447

You could also submit another issue in rails repository to discuss the behaviors of pluck. :)