Count broken if select columns contain parameters

[patrickcarlohickman]

Given the following query and parameter bindings:

select
    `users`.`id`,
    `users`.`email`,
    `users`.`active`,
    (select count(*) from `phones` where `phones`.`active` = ? and `phones`.`user_id` = `users`.`id`) as phones
from `users`
where `users`.`active` = ?

Bindings: [  true,  true ]

This generates the following count query:

select count(*) as aggregate
from (select '1' as row from `users` where `users`.`active` = ?) AS count_row_table

Bindings: [  true,  true ]

The issue is that the columns for the original select had a parameter, but the binding for that parameter was not removed when the select was replaced for the count. Therefore, there are still two values being bound to the new count statement which only has one parameter now. This causes the count statement to fail.

From the PHP docs here: "You cannot bind more values than specified; if more keys exist in input_parameters than in the SQL specified in the PDO::prepare(), then the statement will fail and an error is emitted."

Current solution working for me so far (I can make a pull request, if this looks reasonable):

protected function count($count  = 'count_all')
{
    // snip...

    $myQuery = clone $query;
    $myBindings = $myQuery->getBindings();
    // if its a normal query ( no union ) replace the slect with static text to improve performance
    if( !preg_match( '/UNION/i', $myQuery->toSql() ) ){
        // remove any bindings for parameters found in the select columns
        if (!empty($myBindings)) {
            $myFields = implode(',', $myQuery->columns);
            $numFieldParams = 0;
            // shortcut the regex if no ? at all in fields
            if (strpos($myFields, '?') !== false) {
                // count the number of unquoted parameters (?) in the field list
                $paramRegex = '#(?:(["\'])(?:\\\.|[^\1])*\1|\\\.|[^\?])+#';
                $numFieldParams = preg_match_all($paramRegex, $myFields) - 1;
            }
            // slice off the bindings related to the field parameters
            $myBindings = array_slice($myBindings, $numFieldParams);
        }

        // replace the select columns
        $myQuery->select( DB::raw("'1' as row") );

        // snip...
    }

    $this->$count = DB::connection($connection)
    ->table(DB::raw('('.$myQuery->toSql().') AS count_row_table'))
    ->setBindings($myBindings)->count();
}

[s0ckz]

Not sure if its the same problem, but I had some problems with duplicated binding values, so I solved simply changing the last 3 lines of the function:

        $bindings = $myQuery->getBindings();
        $this->$count = DB::connection($connection)
        ->table(DB::raw('('.$myQuery->toSql().') AS count_row_table'))
        ->setBindings($bindings)->count();

[s0ckz]

Actually this clone thing is messing up some things... I have a complex query with union and it's throwing invalid parameters number exception when I updated the packages from Laravel...

This solved:

        $myQuery = $query;
        if( !preg_match( '/UNION/i', $myQuery->toSql() ) ){
            $myQuery = clone $query;

I'm only cloning if its necessary... I don't think the default clone will work just fine, It was messing up the bindings.

[s0ckz]

Please, check this latest solution, @patrickcarlohickman ... I think the problem is that the parent object, $query (from Eloquent\Builder) is being cloned, but not the bindings array from this query... It explains why the 'true' value appears two times.

[s0ckz]

Actually, I think the clone won't work correctly because the Builder object is not overriding the __clone, and, since it's a complex object, the PHP superficial clone won't work correctly, leading to unexpected results like these...