From 5a462dc5d2a33119ea6cfea91c6e17b99cd8f225 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Wed, 27 Jan 2016 16:26:43 +0100 Subject: [PATCH] added tests and documentation for #8824 --- framework/CHANGELOG.md | 3 +- framework/db/Query.php | 28 ++++++-- framework/db/QueryTrait.php | 7 ++ tests/framework/db/CommandTest.php | 27 ++++++++ tests/framework/db/QueryBuilderTest.php | 85 ++++++++++++++++++++++++- 5 files changed, 142 insertions(+), 8 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 40fdafb940..6f324e6bfb 100644 --- a/framework/CHANGELOG.md +++ b/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) diff --git a/framework/db/Query.php b/framework/db/Query.php index aa8d8fa720..02e043c3ac 100644 --- a/framework/db/Query.php +++ b/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 diff --git a/framework/db/QueryTrait.php b/framework/db/QueryTrait.php index b6ccee26cb..85d0108b5f 100644 --- a/framework/db/QueryTrait.php +++ b/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() diff --git a/tests/framework/db/CommandTest.php b/tests/framework/db/CommandTest.php index 37af409e47..764c59ae6e 100644 --- a/tests/framework/db/CommandTest.php +++ b/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(); diff --git a/tests/framework/db/QueryBuilderTest.php b/tests/framework/db/QueryBuilderTest.php index 5945fb6edd..4478c44091 100644 --- a/tests/framework/db/QueryBuilderTest.php +++ b/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); } }