Count doesn't work with having

[qiangxue]

What steps will reproduce the problem?

1. Create a table
2. Create a CDbcondition with group by and having
3. Run CActiveRecord->count

What is the expected output? What do you see instead?
Is expected to retrive the number of record, but sometime the result is
mistaken because the having condition is not taken in account (it can be
seen by logging the query)

What version of the product are you using? On what operating system?
rev 2150

Please provide any additional information below.
I send you a small test. There is a sql for create a table and a contoller
that shows a query in wich the error is seen.

Migrated from http://code.google.com/p/yii/issues/detail?id=1244

---

earlier comments

matteo.falsitta said, at 2010-05-21T06:40:22.000Z:

A possible solution is to implement the count function with a subquery. like

Count(*)
FROM ($sqlStatement) st

where $sqlStatement is the same statement that is generated for findAll except
order_by and limit clause

matteo.falsitta said, at 2010-05-26T12:47:44.000Z:

I resolved my changing my copy of CDbCommandBuilder at line 91 like that:

/**
 * Creates a COUNT(*) command for a single table.
 * @param mixed the table schema ({@link CDbTableSchema}) or the table name (string).
 * @param CDbCriteria the query criteria
 * @param string the alias name of the primary table. Defaults to 't'.
 * @return CDbCommand query command.
 */
public function createCountCommand($table,$criteria,$alias='t')
{

    $this->ensureTable($table);
    $select=is_array($criteria->select) ? implode(', ',$criteria->select) :

$criteria->select;
if($criteria->alias!='')
$alias=$criteria->alias;
$alias=$this->_schema->quoteTableName($alias);
$sql=($criteria->distinct ? 'SELECT DISTINCT':'SELECT')." {$select} FROM
{$table->rawName} $alias";
$sql=$this->applyJoin($sql,$criteria->join);
$sql=$this->applyCondition($sql,$criteria->condition);
$sql=$this->applyGroup($sql,$criteria->group);
$sql=$this->applyHaving($sql,$criteria->having);

    $sql="SELECT COUNT(*) FROM ($sql) sq";
    $command=$this->_connection->createCommand($sql);
    $this->bindValues($command,$criteria->params);
    return $command;
}

Can you check this solution and update the framework?

matteo.falsitta said, at 2010-06-09T13:44:39.000Z:

Has anyone checked this issue? Since revision 2175 the bug still exists

alexander.makarow said, at 2010-06-15T21:03:29.000Z:

Related topic: http://www.yiiframework.com/forum/index.php?/topic/9492-cactiverecord-count-dont-works-with-having/

thaddeusmt said, at 2010-07-16T16:28:25.000Z:

I can confirm this bug in rev 2264.

I noticed it when using a HAVING query with a CActiveDataProvider and CListView.

The data displayed is correct, but the count shown at the top is incorrect. Looking at the query log it's easy to see that the reason is that when it does the SELECT COUNT(DISTINCT t.id) it is dropping the HAVING clause.

I see it's marked for the 1.1.4 release so I will look for it then. Thanks and cheers!

hmsegura said, at 2010-08-18T14:20:52.000Z:

Related topic: http://code.google.com/p/yii/issues/detail?id=675

qiang.xue said, at 2010-09-01T15:00:46.000Z:

This issue was closed by revision r2404.

ejohnson%bridgeworkscreative.com@gtempaccount.com said, at 2010-09-07T17:01:20.000Z:

So, apparently this was "fixed" in 1.1.4, but actually it seems broken to me now. My expected output has chagned from 1.1.3. When I run a COUNT on my AR query which returns 7 rows it says "1 row" now. I think it's related to the GROUP BY problem...

Here is my query:
$criteria->join = "JOIN
category_template categories_categories ON
(t.id=categories_categories.theme_id)
JOIN category categories ON
(categories.id=categories_categories.category_id)";
$criteria->addInCondition('categories.slug',$currentTags);
$criteria->group = 't.id';
$criteria->having = "count(DISTINCT categories.name) >= ".count($currentTags);

And here is the SQL that COUNT is turning out:

SELECT COUNT(DISTINCT t.id) FROM theme t
JOIN category_template categories_categories ON
(t.id=categories_categories.theme_id)
JOIN category categories ON
(categories.id=categories_categories.category_id)
WHERE (categories.slug='schedulicity') AND (t.status = 'active')
GROUP BY t.id
HAVING count(DISTINCT categories.name) >= 1

When you run this, instead of count(7) I get 7 result rows of count(1). Yii returns the first, which means instead of 7 it says 1.

I don't know... maybe it's working like it should and this has to do with hte group by thing, but my expected output broke between 1.1.3 and 1.1.4. Thanks

qiang.xue said, at 2010-09-07T17:05:30.000Z:

Yes, this may break some existing code if they used group/having but was ignored by Yii. The fix is to remove group/having before doing count(). Sorry.

matteo.falsitta said, at 2010-09-20T06:30:09.000Z:

I can confirm this bug in revision 2485.

Now it throws an exception instead of returning a wrong result.

My suggestion is to use the same code for execute the count and the select. My fix, by the way, is still working correctly. Can you please take in account?

I am also attaching the fix I am using, so if anyone (like hmsegura) has the same problem can workaround it by incuding it while waiting for an official working fix.

You should place this file in component and include in config/main.php as follows:

'db'=>array(
'class'=>'ZDbConnection',
'connectionString'=>'...',

qiang.xue said, at 2010-09-20T12:02:04.000Z:

What is the issue now? And could you highlight what is your fix?

matteo.falsitta said, at 2010-09-20T12:16:29.000Z:

The issue is that count has problem in when there is a group by and having condition.

My fix is to change CDbCommandBuilder like that:

I resolved my changing my copy of CDbCommandBuilder at line 91 like that:

/**
 * Creates a COUNT(*) command for a single table.
 * @param mixed the table schema ({@link CDbTableSchema}) or the table name (string).
 * @param CDbCriteria the query criteria
 * @param string the alias name of the primary table. Defaults to 't'.
 * @return CDbCommand query command.
 */
public function createCountCommand($table,$criteria,$alias='t')
{

    $this->ensureTable($table);
    $select=is_array($criteria->select) ? implode(', ',$criteria->select) :

$criteria->select;
if($criteria->alias!='')
$alias=$criteria->alias;
$alias=$this->_schema->quoteTableName($alias);
$sql=($criteria->distinct ? 'SELECT DISTINCT':'SELECT')." {$select} FROM
{$table->rawName} $alias";
$sql=$this->applyJoin($sql,$criteria->join);
$sql=$this->applyCondition($sql,$criteria->condition);
$sql=$this->applyGroup($sql,$criteria->group);
$sql=$this->applyHaving($sql,$criteria->having);

    $sql="SELECT COUNT(*) FROM ($sql) sq";
    $command=$this->_connection->createCommand($sql);
    $this->bindValues($command,$criteria->params);
    return $command;
}

This uses the same query generated for select like a subquery for count the items. It returns always the right number (at least in all my tests).

With the actual revision, this kind of criteria:

$criteria->select='page, count(*) as request';
$criteria->group= 'page';
$criteria->having= 'request>='.(int) $this->request;

Makes the count to fail, because the select is not considered. With my proposal it works good.

What do you think about, Qiang?

qiang.xue said, at 2010-09-20T12:21:52.000Z:

I see. I think we only need to use sub-queries when there is no group or having. Please correct me if I'm wrong.

matteo.falsitta said, at 2010-09-20T12:30:44.000Z:

As soon as I can imagine, there is no need of subqueries if there are no group or having.

Anyway I think that semplicity is also a value, and as CDbCommandBuilder::createCountCommand is already very complicated, I suggest to change to a simpier implementation instead of adding another one if to the many you already have.

Simi4a said, at 2010-10-06T20:14:12.000Z:

Hey, nice fix matteo.falsitta, thank you very much.

jamesgillmore said, at 2010-10-30T12:53:06.000Z:

works like a charm. Nice work Matteo!

Everybody, dont forget to download the extended extension (which itself extends CDbConnection) required for Matteo's patch:
http://www.yiiframework.com/extension/phppdo/

also, Matteo, your patch seems to work fine for me if I just extend CDbConnection. What's the requirement for needing that PDO extension? Do I actually need it if i don't have PDO blocked by my hosting provider?

matteo.falsitta said, at 2010-10-30T15:59:20.000Z:

It works fine if you use CDbConnection. In this project I had also the problem with a hosting that didn't support pdo, so I had to use even this extension. When I posted the workaround I forgot to extend CDbConnection, that's all!!

qiang.xue said, at 2010-11-02T02:31:06.000Z:

Issue 1602 has been merged into this issue.

qiang.xue said, at 2010-11-02T15:09:05.000Z:

This issue was closed by revision r2601.

qiang.xue said, at 2010-11-02T15:09:56.000Z:

Could you help test it? Thanks!

matteo.falsitta said, at 2010-11-02T15:33:58.000Z:

I tested right now with the revision 2602 and still there is the problem.

Tomorrow I will give a more deep glance and I will tell you something of more detailed.

nanda.sunu said, at 2011-01-04T19:16:50.000Z:

I am experiencing this problem with r2654, any word on when a fix will be out?

qiang.xue said, at 2011-01-04T19:24:57.000Z:

Please report what the problem is.

alexander.makarow said, at 2011-01-14T13:35:16.000Z:

http://www.yiiframework.com/forum/index.php?/topic/9492-cactiverecord-count-dont-works-with-having/

matteo.falsitta said, at 2011-03-16T15:27:52.000Z:

In revision 3092 (or maybe previous) it is fixed.

Thank you very much for your work!

haertl.mike said, at 2011-04-04T08:21:11.000Z:

The fix above only applies to single table queries. I found that this bug still exists for relational queries. While i'm not sure where/how the fix has to be applied, i found this line in CActiveFinder:756 which clears all HAVING + GROUP BY clauses from a query:

$query->orders=$query->groups=$query->havings=array();

Maybe someone can comment wether a similar fix can be used in CJoinElement?

haertl.mike said, at 2011-04-05T10:48:28.000Z:

I've implemented a fix that works for me. Unfortunately issue 2294 was blocking it, so i had to comment out some lines to make the following fix work (see issue 2294).

The fix adds a new condition to CJoinElement::count():

public function count($criteria=null)
{
    $query=new CJoinQuery($this,$criteria);
    // ensure only one big join statement is used
    $this->_finder->baseLimited=false;
    $this->_finder->joinAll=true;
    $this->buildQuery($query);

    // <<< START >>>
    if (!empty($criteria->group) && !empty($criteria->having))
    {
        $query->orders=array();
        $query->limit=$query->offset=-1;
        $command=$query->createCommand($this->_builder,true);
        return $command->queryScalar();
    }
    // <<< END >>>

and changes createCommand to:

public function createCommand($builder,$countSubQuery=false)
{
    /* ... */

    // <<< START >>>
    if($countSubQuery)
        $sql="SELECT COUNT(*) FROM ($sql) sq";
    // <<< END >>>

    $command=$builder->getDbConnection()->createCommand($sql);
    $builder->bindValues($command,$this->params);
    return $command;
}

Now GROUP BY ... HAVING ... also works for ARR queries for me. I'm not sure if it matches all edge cases. But it should at least improve the situation.

haertl.mike said, at 2011-04-11T07:15:20.000Z:

The fix above is compatible with the fix for issue 2294.

Drawback of the suggested fix:

The Sub-SELECT still selects all columns. It should be sufficient to only select the main table Pk column + custom columns (the latter could get used in query conditions):

$criteria->select=array('t.*','COUNT(othertable.id) AS someCount');
$criteria->with=array('othertable');
$criteria->group='t.id';
$criteria->having='someCount<4';

haertl.mike said, at 2011-04-19T12:23:14.000Z:

Is there a chance to get a fix for this included in the next release?

I'm willing to help out where i can. The fix i suggested seems to work, it only could need some more tuning, see last comment.

I can also commit it, if it helps.

tonvandenheuvel said, at 2011-05-12T09:00:05.000Z:

Chiming in that I would also like to see haertl.mike's fix being committed so that it can be tested before it is included in a release.

maximcherny said, at 2011-08-15T01:26:36.000Z:

Guys, have you ever considered "SELECT SQL_CALC_FOUND_ROWS..."?

alexander.makarow said, at 2011-08-15T13:21:37.000Z:

No:

1. MySQL specific.
2. Sometimes is slower than 2 separate queries.

   qiang.xue said, at 2012-01-01T03:36:53.000Z:

set for 1.1.10 milestone

qiang.xue said, at 2012-01-01T03:37:09.000Z:

set for 1.1.10 milestone

qiang.xue said, at 2012-01-01T03:37:35.000Z:

set for 1.1.10 milestone

[Rupert-RR]

I'm running into this problem too - any word on when a fix will be forthcoming (I see that no-one is assigned to it at the moment)?

[tudorilisoi]

I can confirm this bug as well.
CDbCommandBuilder does count right for tables without relations (see createCountCommand() method)
CActiveFinder strips 'group' and 'having', inspite of support for those clauses in AR relations().(see count() method)
My fix was to wrap the count in a subselect just like CdbCommandBuilder, which is not by far optimal, because it fetches all records and columns in order to count them.
I can't think of a quick memory-unexpensive solution. Maybe some sort of criteria parameter such as 'count_select'(1) would allow to count properly in more complicated scenarios, without fetching all the records and columns.
For example, you want to display posts and filter by rating which is an aggregate field:

$criteria->select = '*, AVG(foreign_table.rating) AS rating';

$model = Posts::model()->with('rating_relation')->with('author')->with('comments')->together();

This will JOIN a lot of tables if you need to fetch eagerly, so that you'll be able to sort a grid or something similar.
But, when it comes to counting records, you could do (1):

$model->getDbCriteria()->count_select='t.id, AVG(foreign_table.rating) AS rating'

When the CActiveFinder will need to count all records, it will do

$query=new CJoinQuery($this,$criteria);
// ensure only one big join statement is used
$this->_finder->baseLimited=false;
$this->_finder->joinAll=true;
$this->buildQuery($query);

//override select columns list with the count_select
$query->selects=array($criteria->count_select);

$command=$query->createCommand($this->_builder);
$sql = ($command->text);
//wrap query in a subselect
$sql="SELECT COUNT(*) FROM ($sql) sq";
$command->text = $sql;
return $command->queryScalar();

This is the most flexible way I can think of for circumventing these issues.

[tudorilisoi]

Hi!
I did an "early" implementation of $count_select in both in CActiveRecord relations and CDbCriteria.
So far this is tested with only 1 JOIN and it does work without ingnoring the HAVING clause.

<?
public function relations()
{
return array(
            'model_stems' => array(
                CActiveRecord::HAS_MANY,
                'XStemModelWordsModel',
                'model_id',
                'group' => "$alias.model_uid",
                'joinType' => "LEFT JOIN",
                'on' => "$alias.model_class='" . get_class($owner) . "'",
                'select' => "
                            COUNT(DISTINCT $alias.stem_id) AS stem_count, 
                            SUM(weight)*COUNT(DISTINCT $alias.stem_id) as search_rank,
                            SUM(weight) AS item_weight",
// this is the new relation property
                'count_select' => "COUNT(DISTINCT $alias.stem_id) AS stem_count",

            );
}

similarily, you can use a CDbCriteria object like this:

<?
        $criteria = $model->getDbCriteria();
        $model->with('model_stems')->together();
        $criteria->having = "stem_count >=" . $word_count;
        $criteria->count_select="t.id as parent_id";

the 'count_select's are merged both from the relations and criteria, only when counting an eagerly loaded model with relations.
Here's the branch with the fix:
https://github.com/tudorilisoi/yii/tree/issue_82
and here's a list of changes which I introduced:
https://github.com/tudorilisoi/yii/compare/issue_82
please pull the latest revision if interested

[andr2k]

Met the bug too. Would be nice to get rid of it.

[samdark]

Should be fixed now with #2167