added tests and documentation for #8824

framework/CHANGELOG.md

@@ -69,6 +69,7 @@ Yii Framework 2 Change Log
 - Enh #8613: `yii\widgets\FragmentCache` will not store empty content anymore which fixes some problems related to `yii\filters\PageCache` (kidol)
 - Enh #8649: Added total applied migrations to final report (vernik91)
 - Enh #8687: Added support for non-gregorian calendars, e.g. persian, taiwan, islamic to `yii\i18n\Formatter` (cebe, z-avanes, hooman-pro)
+- Enh #8824: Allow passing a `yii\db\Expression` to `Query::groupBy()` (cebe)
 - Enh #8995: `yii\validators\FileValidator::maxFiles` can be set to `0` to allow unlimited count of files (PowerGamer1, silverfire)
 - Enh #9282: Improved JSON error handling to support PHP 5.5 error codes (freezy-sk)
 - Enh #9337: Added `yii\db\ColumnSchemaBuilder::defaultExpression()` to support DB Expression as default value (kotchuprik)
@@ -103,7 +104,7 @@ Yii Framework 2 Change Log
 - Enh #10319: `yii\helpers\VarDumper::dump()` now respects PHP magic method `__debugInfo()` (klimov-paul)
 - Enh #10359: Support wildcard category name in `yii/console/controllers/MessageController` (rmrevin)
 - Enh #10390: Added ability to disable outer tag for `\yii\helpers\BaseHtml::radiolist()`, `::checkboxList()` (TianJinRong, githubjeka, silverfire)
-- Enh #10535: Allow passing a `yii\db\Expression` to `Query::orderBy()` (andrewnester, cebe)
+- Enh #10535: Allow passing a `yii\db\Expression` to `Query::orderBy()` and `Query::groupBy()` (andrewnester, cebe)
 - Enh: Added last resort measure for `FileHelper::removeDirectory()` fail to unlink symlinks under Windows (samdark)
 - Chg #9369: `Yii::$app->user->can()` now returns `false` instead of erroring in case `authManager` component is not configured (creocoder)
 - Chg #9411: `DetailView` now automatically sets container tag ID in case it's not specified (samdark)

framework/db/Query.php

@@ -677,16 +677,24 @@ class Query extends Component implements QueryInterface
 
     /**
      * Sets the GROUP BY part of the query.
-     * @param string|array $columns the columns to be grouped by.
+     * @param string|array|Expression $columns the columns to be grouped by.
      * Columns can be specified in either a string (e.g. "id, name") or an array (e.g. ['id', 'name']).
      * The method will automatically quote the column names unless a column contains some parenthesis
      * (which means the column contains a DB expression).
+     *
+     * Note that if your group-by is an expression containing commas, you should always use an array
+     * to represent the group-by information. Otherwise, the method will not be able to correctly determine
+     * the group-by columns.
+     *
+     * Since version 2.0.7, an [[Expression]] object can be passed to specify the GROUP BY part explicitly in plain SQL.
      * @return $this the query object itself
      * @see addGroupBy()
      */
     public function groupBy($columns)
     {
-        if (!is_array($columns)) {
+        if ($columns instanceof Expression) {
+            $columns = [$columns];
+        } elseif (!is_array($columns)) {
             $columns = preg_split('/\s*,\s*/', trim($columns), -1, PREG_SPLIT_NO_EMPTY);
         }
         $this->groupBy = $columns;
@@ -699,12 +707,20 @@ class Query extends Component implements QueryInterface
      * Columns can be specified in either a string (e.g. "id, name") or an array (e.g. ['id', 'name']).
      * The method will automatically quote the column names unless a column contains some parenthesis
      * (which means the column contains a DB expression).
+     *
+     * Note that if your group-by is an expression containing commas, you should always use an array
+     * to represent the group-by information. Otherwise, the method will not be able to correctly determine
+     * the group-by columns.
+     *
+     * Since version 2.0.7, an [[Expression]] object can be passed to specify the GROUP BY part explicitly in plain SQL.
      * @return $this the query object itself
      * @see groupBy()
      */
     public function addGroupBy($columns)
     {
-        if (!is_array($columns)) {
+        if ($columns instanceof Expression) {
+            $columns = [$columns];
+        } elseif (!is_array($columns)) {
             $columns = preg_split('/\s*,\s*/', trim($columns), -1, PREG_SPLIT_NO_EMPTY);
         }
         if ($this->groupBy === null) {
@@ -717,7 +733,7 @@ class Query extends Component implements QueryInterface
 
     /**
      * Sets the HAVING part of the query.
-     * @param string|array $condition the conditions to be put after HAVING.
+     * @param string|array|Expression $condition the conditions to be put after HAVING.
      * Please refer to [[where()]] on how to specify this parameter.
      * @param array $params the parameters (name => value) to be bound to the query.
      * @return $this the query object itself
@@ -734,7 +750,7 @@ class Query extends Component implements QueryInterface
     /**
      * Adds an additional HAVING condition to the existing one.
      * The new condition and the existing one will be joined using the 'AND' operator.
-     * @param string|array $condition the new HAVING condition. Please refer to [[where()]]
+     * @param string|array|Expression $condition the new HAVING condition. Please refer to [[where()]]
      * on how to specify this parameter.
      * @param array $params the parameters (name => value) to be bound to the query.
      * @return $this the query object itself
@@ -755,7 +771,7 @@ class Query extends Component implements QueryInterface
     /**
      * Adds an additional HAVING condition to the existing one.
      * The new condition and the existing one will be joined using the 'OR' operator.
-     * @param string|array $condition the new HAVING condition. Please refer to [[where()]]
+     * @param string|array|Expression $condition the new HAVING condition. Please refer to [[where()]]
      * on how to specify this parameter.
      * @param array $params the parameters (name => value) to be bound to the query.
      * @return $this the query object itself

framework/db/QueryTrait.php

@@ -295,8 +295,10 @@ trait QueryTrait
      * @param string|array|Expression $columns the columns (and the directions) to be ordered by.
      * Columns can be specified in either a string (e.g. `"id ASC, name DESC"`) or an array
      * (e.g. `['id' => SORT_ASC, 'name' => SORT_DESC]`).
+     *
      * The method will automatically quote the column names unless a column contains some parenthesis
      * (which means the column contains a DB expression).
+     *
      * Note that if your order-by is an expression containing commas, you should always use an array
      * to represent the order-by information. Otherwise, the method will not be able to correctly determine
      * the order-by columns.
@@ -316,9 +318,14 @@ trait QueryTrait
      * @param string|array|Expression $columns the columns (and the directions) to be ordered by.
      * Columns can be specified in either a string (e.g. "id ASC, name DESC") or an array
      * (e.g. `['id' => SORT_ASC, 'name' => SORT_DESC]`).
+     *
      * The method will automatically quote the column names unless a column contains some parenthesis
      * (which means the column contains a DB expression).
      *
+     * Note that if your order-by is an expression containing commas, you should always use an array
+     * to represent the order-by information. Otherwise, the method will not be able to correctly determine
+     * the order-by columns.
+     *
      * Since version 2.0.7, an [[Expression]] object can be passed to specify the ORDER BY part explicitly in plain SQL.
      * @return $this the query object itself
      * @see orderBy()

tests/framework/db/CommandTest.php

@@ -222,6 +222,33 @@ SQL;
         $this->assertEquals('user5@example.com', $command->queryScalar());
     }
 
+    public function paramsNonWhereProvider()
+    {
+        return[
+            ['SELECT SUBSTR(name, :len) FROM {{customer}} WHERE [[email]] = :email GROUP BY SUBSTR(name, :len)'],
+            ['SELECT SUBSTR(name, :len) FROM {{customer}} WHERE [[email]] = :email ORDER BY SUBSTR(name, :len)'],
+            ['SELECT SUBSTR(name, :len) FROM {{customer}} WHERE [[email]] = :email'],
+        ];
+    }
+
+    /**
+     * Test whether param binding works in other places than WHERE
+     * @dataProvider paramsNonWhereProvider
+     */
+    public function testBindParamsNonWhere($sql)
+    {
+        $db = $this->getConnection();
+
+        $db->createCommand()->insert('customer', ['name' => 'testParams', 'email' => 'testParams@example.com', 'address' => '1'])->execute();
+
+        $params = [
+            ':email' => 'testParams@example.com',
+            ':len' => 5,
+        ];
+        $command = $db->createCommand($sql, $params);
+        $this->assertEquals('Params', $command->queryScalar());
+    }
+
     public function testFetchMode()
     {
         $db = $this->getConnection();

tests/framework/db/QueryBuilderTest.php

@@ -536,6 +536,15 @@ class QueryBuilderTest extends DatabaseTestCase
         $expected = $this->replaceQuotes("SELECT 1 AS ab, 2 AS cd, 3 AS [[ef]] FROM [[tablename]]");
         $this->assertEquals($expected, $sql);
         $this->assertEmpty($params);
+
+        $query = (new Query())
+            ->select(new Expression("SUBSTR(name, 0, :len)", [':len' => 4]))
+            ->from('tablename');
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes("SELECT SUBSTR(name, 0, :len) FROM [[tablename]]");
+        $this->assertEquals($expected, $sql);
+        $this->assertEquals([':len' => 4], $params);
+
     }
 
     public function testCompositeInCondition()
@@ -586,15 +595,89 @@ class QueryBuilderTest extends DatabaseTestCase
 
     public function testOrderBy()
     {
+        // simple string
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->orderBy('name ASC, date DESC');
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] ORDER BY `name`, `date` DESC');
+        $this->assertEquals($expected, $sql);
+        $this->assertEmpty($params);
+
+        // array syntax
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->orderBy(['name' => SORT_ASC, 'date' => SORT_DESC]);
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] ORDER BY `name`, `date` DESC');
+        $this->assertEquals($expected, $sql);
+        $this->assertEmpty($params);
+
+        // expression
         $query = (new Query())
             ->select('*')
             ->from('operations')
             ->where('account_id = accounts.id')
             ->orderBy(new Expression('SUBSTR(name, 3, 4) DESC, x ASC'));
-
         list ($sql, $params) = $this->getQueryBuilder()->build($query);
         $expected = $this->replaceQuotes('SELECT * FROM [[operations]] WHERE account_id = accounts.id ORDER BY SUBSTR(name, 3, 4) DESC, x ASC');
         $this->assertEquals($expected, $sql);
         $this->assertEmpty($params);
+
+        // expression with params
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->orderBy(new Expression('SUBSTR(name, 3, :to) DESC, x ASC', [':to' => 4]));
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] ORDER BY SUBSTR(name, 3, :to) DESC, x ASC');
+        $this->assertEquals($expected, $sql);
+        $this->assertEquals([':to' => 4], $params);
+    }
+
+    public function testGroupBy()
+    {
+        // simple string
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->groupBy('name, date');
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY `name`, `date`');
+        $this->assertEquals($expected, $sql);
+        $this->assertEmpty($params);
+
+        // array syntax
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->groupBy(['name', 'date']);
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY `name`, `date`');
+        $this->assertEquals($expected, $sql);
+        $this->assertEmpty($params);
+
+        // expression
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->where('account_id = accounts.id')
+            ->groupBy(new Expression('SUBSTR(name, 0, 1), x'));
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] WHERE account_id = accounts.id GROUP BY SUBSTR(name, 0, 1), x');
+        $this->assertEquals($expected, $sql);
+        $this->assertEmpty($params);
+
+        // expression with params
+        $query = (new Query())
+            ->select('*')
+            ->from('operations')
+            ->groupBy(new Expression('SUBSTR(name, 0, :to), x', [':to' => 4]));
+        list ($sql, $params) = $this->getQueryBuilder()->build($query);
+        $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY SUBSTR(name, 0, :to), x');
+        $this->assertEquals($expected, $sql);
+        $this->assertEquals([':to' => 4], $params);
     }
 }