calculate-all icon indicating copy to clipboard operation
calculate-all copied to clipboard

Add ids as a function alias

Open StefSchenkelaars opened this issue 8 years ago • 4 comments

Hi there, I found myself fetching the associated ids during a call during my project multiple times. I performed this by calling

Order.group(:currency).calculate_all(ids: 'ARRAY_AGG(id)')

I've now added the :ids as a function alias but the ARRAY_AGG function only works on Postgres.

On MySQL sort of similar behaviour can be achieved by using the group-concat function but it returns a string instead of an array. This can later be parsed but I did not put too much effort in this since I don't use MySQL. In addition you would need a check in the helpers module to see what database service you are using. Maybe someone has an idea and we can have a discussion about this.

StefSchenkelaars avatar Jan 28 '17 16:01 StefSchenkelaars

Um, do we really need it?

I mean, compare with other "function aliases": ids isn't working for any column other than id, it works only on postgres, and it isn't actually a "calculating" aggregate, calculate_all name kinda stop making sense.

I've added function aliases (and calculate_all gem itself) for cases when you going to fetch a lot of aggregates in one pass, like calculate_all(:amount_max, :amount_min, :amount_average, :amount_sum). I can't imagine a scenario, when you would want to fetch more aggregates together with calculate_all('array_agg(id)') or calculate_all(ids: 'array_agg(id)'), and it's short enough already with one argument, should work as a replacement for grouping #ids on scopes.

I actually considered it a year ago, but didn't add it because of reasons above. There're a lot of nice functions in https://www.postgresql.org/docs/9.6/static/functions-aggregate.html (I love percentiles, for example), but they are all made possible to be used by this gem already.

I would consider adding postgres only array_agg alias, though, if it'd be more generic. Something enabling using different columns, calling array_agg(disctint column1), or even array_agg(column1 order by column2 desc), though. Maybe, :column1_array or :column1_distinct_array, or :column1s. But it's a bit too magic already. What do you think?

codesnik avatar Jan 29 '17 11:01 codesnik

Also, I never considered #calculate_all a good method to be called from your views or controllers. It's a plumbing, to be called from model class methods or some query object. And if you extracted some logic there, 'array_agg()' sql fragment doesn't look so scary anymore.

codesnik avatar Jan 29 '17 11:01 codesnik

Well I of course don't use it with only fetching the ids. I use it in an API call which returns multiple averages and ids grouped by some attribute. So I do use it now in a combination with other aggregates.

But I agree that it is not really generic and if you start on adding this alias, you should probably include more postgres aggregate functions. Maybe a good idea to leave this PR open for now and not use it until I / we have found a nice generic way of combining this stuff.

On the other hand, things like :column1_array or :column1s doesn't sound that magic for me and now you just get an error if the function alias is not found so I think more aliases is not bad. You can always use custom stuff by entering a string.

By the way, did you also wonder why this is not included in ActiveRecord? Especially that the calculate functions like average do not accept arrays and only work on one attribute.

StefSchenkelaars avatar Jan 29 '17 11:01 StefSchenkelaars

I've thought about making a patch for active_record, as an additional form to call #calculate or as another method. This gem was my try to refine and strip down functionality before trying to incorporate it into activerecord. That's one of the reasons I've been reluctant to add postgres only features.

But I feel it doesn't really fit, at least, right now.

ActiveRecord isn't really made with using raw sql fragments in mind, and aggregating stuff is so basic, even with this gem it isn't advanced much into the relation realm. Looks like anyone who needs something more complex than CRUD has to juggle with arrays of hashes of arrays returned by a private activerecord API, or jump straight to sequel or alternatives.

That gem in particular was extracted from code in which I tried to use more advanced stuff, see http://stackoverflow.com/questions/35210823/why-could-postgresql-9-5s-cube-rollup-and-grouping-sets-be-slower-than-equival/35544953

I just noticed that first particular step could be implemented with public API #pluck, and wanted to share this hack with others, it's nowhere near obvious. And still after initial grouping I had to juggle with arrays and hashes, like it isn't an ActiveRecord at all.

It's not included or written by someone else because not a lot of people need it, I think. Do you know any alternatives for this gem? And you are probably the only active user of this one.

codesnik avatar Jan 29 '17 12:01 codesnik