In function query in views_plugin_query_default.inc
$this->has_aggregate = FALSE;
$non_aggregates = array();
list($non_aggregates) = $this->compile_fields($fields_array, $query);
if (count($this->having)) {
$this->has_aggregate = TRUE;
}
if ($this->has_aggregate && (!empty($this->groupby) || !empty($non_aggregates))) {
$groupby = array_unique(array_merge($this->groupby, $non_aggregates));
foreach ($groupby as $field) {
$query->groupBy($field);
}
if (!empty($this->having) && $condition = $this->build_condition('having')) {
$query->havingCondition($condition);
}
}
I'm trying to use add_groupby but couldn't get it to work until I added a having, to make it through this code. Why shouldn't I be allowed to have a SQL "GROUP BY" withoug a "HAVING"?
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | views-has_aggregate-1512488-11.patch | 673 bytes | liam mcdermott |
| #10 | views-has_aggregate-1512488-10.patch | 616 bytes | ronino |
| #8 | views-has_aggregate-1512488-8.patch | 598 bytes | ronino |
Comments
Comment #1
dawehnerI'm not really sure what's your point ... there is a !empty($this->having), so only if you have having it gets added.
Your problem is probably that you have configured your conditions to met a count criteria?
Comment #2
jody lynnBut $this->has_aggregate will always be FALSE unless you have something in $this->having. So my group by does not work unless I add a having.
Comment #3
merlinofchaos commentedUnless there's some other way to set has_aggregate, Jody Lynn is totally right. And I don't know the answer to her question. This seems to be an error?
Comment #4
dawehnerOh i see...
Well true, this seems to be an error, do you want to provide a patch? I think just moving out the condition would be fine here.
Comment #5
jody lynnYeah, I'm not clear on the intention of $this->has_aggregate, what it's for and where else it is used. In this section of code it seems useless though.
Comment #6
VivienLetang commentedI confirm the bug. A simple way to resolve it is to remove the test on $this->has_aggregate
I will build a well formatted patch tommorow morning
Comment #7
rgristroph commentedI can confirm that I am also not able to do a add_groupby() without also doing a add_having(). Also, when I do the add_groupby() on one field I get in the SQL a "GROUP BY" on all the fields, but I think that is a separate bug.
Comment #8
ronino commented$this->has_aggregate is also used and set in views_plugin_query_default::compile_fields(), but I don't really understand how to trigger that. I helped myself by adding $this->query->add_having_expression(0, '1') in my handler which finally adds the required GROUP BY.
In case the test
if ($this->has_aggregate && (!empty($this->groupby) || !empty($non_aggregates))) {is indeed a bug, I added a patch to remove the has_aggregate condition.
Comment #10
ronino commentedUpdated patch to comment #8.
Comment #11
liam mcdermott commentedWe've just been doing some work on User Stats and have a View that's using GROUP BY, this had us banging our heads against a brick wall!
Anyway, here's a re-rolled patch, I've tested and it works as expected.
Comment #12
dawehnerStuff like that should really really have a test, i'm sorry but these are critical code paths, which should be backed up :(
Comment #13
liam mcdermott commentedSorry, I don't follow. This needs tests written, or I should mark the status 'needs review' so the patch will be automatically tested?
Comment #14
dawehnerIf it's a bug, you can write a simpletest, in order to be sure that your change fixes the issue.
Set to needs work, in order to write a test for it. Sorry I think these changes in the central pieces should be avoided as much as possible.
Comment #15
alcroito commentedDuplicate of #2159347: Aggregation not working simple test case.