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 group clause in scope cause error #1123

Closed
janckerchen opened this issue Mar 11, 2012 · 15 comments
Closed

Use group clause in scope cause error #1123

janckerchen opened this issue Mar 11, 2012 · 15 comments

Comments

@janckerchen
Copy link

when I add group clause to scope like this

(I change the example code to make it clear after latortuga commented)

get count for each category.

class MyClass < ActiveRecord::Base
scope :category, select("count(id) as count, category").group(:category)
end

ActiveAdmin.register MyClass do
scope :category, :default => true
end

I get this error on rails page:

Showing /usr/local/lib/ruby/gems/1.9.1/gems/activeadmin-0.4.3/app/views/active_admin/resource/index.html.arb where line #1 raised:
ActiveSupport::OrderedHash can't be coerced into Fixnum

I notice that, in rails console, count attribute of scope with group is not Fixnum but Hash.

I am not sure the issue belongs to rails or activeadmin, somebody help me find the way~~

@latortuga
Copy link
Contributor

It doesn't make sense to have use scope that's performing an aggregate function as your default. You should use scopes to order or filter the data presented to you. Performing SELECT MAX(age) which will return a single value, the max of the age column. The reason ordered hash is returned is because it is also returning the nickname of the same row as the max age.

@janckerchen
Copy link
Author

latortuga, I change the example code to make it clear, how can I fill with this requirement by scope in activeadmin?

@latortuga
Copy link
Contributor

What is it that you're actually trying to accomplish? Everything I said in my other comment still applies - if you are using an aggregate function in your query and then grouping, it doesn't make sense to use that scope for the index page.

@janckerchen
Copy link
Author

en... I should find another way.

@joevandyk
Copy link
Contributor

@latortuga i'm confused why you say it wouldn't make sense.

Say you have:

class User < AR::Base
  has_many :scores
  scope :plays_a_lot, 
       joins('left join scores on scores.user_id = users.id').
       having('count(scores.*) > 18').
       group('users.id')
end

So you could do User.plays_a_lot to find all users that have more than 18 scores.

@latortuga
Copy link
Contributor

@joevandyk Your scope works fine and I probably painted with too broad a brush. You can probably get away with using it with AA by suppressing the scope count calculation. I meant mostly that it doesn't make sense to use an aggregate on the table you're trying to display for an index method. SELECT max(users.age) FROM users doesn't make a very good query for an index page, it returns just the single result.

@joevandyk
Copy link
Contributor

The problem is that it doesn't actually work fine. :) calling User.plays_a_lot.count returns an ordered hash, not an integer, and activeadmin blows up when it tries to paginate over the :plays_a_lot scope.

@latortuga
Copy link
Contributor

Your group clause doesn't make sense - you're grouping on a primary key (meaning it's unique, it won't group anything).

What you're describing is not an active admin issue, it's a SQL issue. To get the number of results returned from a query with a group, you have to put the group into a sub-query and count on the temporary table that is generated.

@joevandyk
Copy link
Contributor

To do the aggregate count, you have to do a group on the user_id.

@latortuga
Copy link
Contributor

You can't perform an aggregate count on a grouped SQL query. Look at the SQL:

SELECT scores.user_id, COUNT(*)
FROM scores
GROUP BY scores.user_id
HAVING COUNT(*) > 18

Where do you propose we put the "aggregate count" in that query? The answer, of course, is that you have to either perform two queries or use a sub-select.

@rraub
Copy link
Contributor

rraub commented Jun 29, 2012

I'm running into a similar problem, and I noticed that other applications (like phpmyadmin) use:
SELECT SQL_CALC_FOUND_ROWS * ...; SELECT FOUND_ROWS();
to do their counting. Of course this is really mysql specific..

@joevandyk
Copy link
Contributor

In this case, I'm selecting users, not scores.

create table users (id integer primary key);
create table scores (user_id integer references users);

insert into users select generate_series(1, 10);
insert into scores select generate_series(1, 10);
insert into scores select generate_series(1, 5);
insert into scores select generate_series(1, 3);

-- The SQL AR generates for User.plays_a_lot
select id from users
left join scores on scores.user_id = users.id
group by users.id
having count(scores.*) > 2;

Outputs:
  1
  2
  3

-- The ideal count query
-- Unfortunately, AR doesn't generate this sql
-- with User.plays_a_lot.count.
select count(*) from (
select id from users
left join scores on scores.user_id = users.id
group by users.id
having count(scores.*) > 2) f;

Outputs:
1

@rraub
Copy link
Contributor

rraub commented Jun 29, 2012

Ok, I've figured out a decent solution for you using an inner select to grab the user id's.

scope :plays_a_lot, 
    where(' users.id in (select count(*) as ROWS from scores WHERE ROWS > 18) ')

performance wise it seems on par with the group by query at least from my testing.

@latortuga
Copy link
Contributor

Unfortunately, AR doesn't generate this sql

Now you're asking for magic. Like I said, this is not an Active Admin issue, but rather a function of how GROUP BY changes the semantics of count. There's no possible way to infer from your joins call that a sub-select will be required.

This whole things stinks of a non-problem. If you're going to be querying this type of thing often, use a counter cache. If not, just run it from the command line and get your info that way. When there's actual running code that is causing problems we can continue this thread but trying to work out a solution for made up code is a waste of time.

@joevandyk
Copy link
Contributor

I ran into this problem two days ago. Yes, I could add counter caches for each of these things, but I'd rather not slow down inserts and add a bunch of columns for each particular thing that I might want to count by.

What would solve the problem for me is for activeadmin to not paginate records with that particular scope.

i.e.

scope :plays_a_lot, :paginate => false

There is an option to enable/disable pagination, but it doesn't seem to work across scopes.

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

4 participants