From c3240525dfcc9de543886287aefdd4be9f7a39af Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Wed, 27 Jan 2016 15:33:01 +0100 Subject: [PATCH 1/4] updated docs about Expression class fixes #10647 --- framework/db/Expression.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/framework/db/Expression.php b/framework/db/Expression.php index ea7586203a..c40c4f97b0 100644 --- a/framework/db/Expression.php +++ b/framework/db/Expression.php @@ -9,15 +9,20 @@ namespace yii\db; /** * Expression represents a DB expression that does not need escaping or quoting. + * * When an Expression object is embedded within a SQL statement or fragment, * it will be replaced with the [[expression]] property value without any * DB escaping or quoting. For example, * * ```php * $expression = new Expression('NOW()'); - * $sql = 'SELECT ' . $expression; // SELECT NOW() + * $now = (new \yii\db\Query)->select($expression)->scalar(); // SELECT NOW(); + * echo $now; // prints the current date * ``` * + * Expression objects are mainly created for passing raw SQL expressions to methods of + * [[Query]], [[ActiveQuery]], and related classes. + * * An expression can also be bound with parameters specified via [[params]]. * * @author Qiang Xue From 5a462dc5d2a33119ea6cfea91c6e17b99cd8f225 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Wed, 27 Jan 2016 16:26:43 +0100 Subject: [PATCH 2/4] 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); } } From f0a62cdbd399930c4afb37286f722fde76d5c208 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Wed, 27 Jan 2016 16:39:34 +0100 Subject: [PATCH 3/4] allow expressions in GROUP BY --- framework/db/QueryBuilder.php | 20 +++++++++++++++++++- tests/framework/db/QueryBuilderTest.php | 8 ++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/framework/db/QueryBuilder.php b/framework/db/QueryBuilder.php index 0709634529..3aeb57fee3 100644 --- a/framework/db/QueryBuilder.php +++ b/framework/db/QueryBuilder.php @@ -751,7 +751,21 @@ class QueryBuilder extends \yii\base\Object */ public function buildGroupBy($columns) { - return empty($columns) ? '' : 'GROUP BY ' . $this->buildColumns($columns); + if (empty($columns)) { + return ''; + } + foreach ($columns as $i => $column) { + if ($column instanceof Expression) { + $columns[$i] = $column->expression; + // TODO +// foreach ($direction->params as $n => $v) { +// $params[$n] = $v; +// } + } elseif (strpos($column, '(') === false) { + $columns[$i] = $this->db->quoteColumnName($column); + } + } + return 'GROUP BY ' . implode(', ', $columns); } /** @@ -800,6 +814,10 @@ class QueryBuilder extends \yii\base\Object foreach ($columns as $name => $direction) { if ($direction instanceof Expression) { $orders[] = $direction->expression; + // TODO +// foreach ($direction->params as $n => $v) { +// $params[$n] = $v; +// } } else { $orders[] = $this->db->quoteColumnName($name) . ($direction === SORT_DESC ? ' DESC' : ''); } diff --git a/tests/framework/db/QueryBuilderTest.php b/tests/framework/db/QueryBuilderTest.php index 4478c44091..4ae1943c6e 100644 --- a/tests/framework/db/QueryBuilderTest.php +++ b/tests/framework/db/QueryBuilderTest.php @@ -601,7 +601,7 @@ class QueryBuilderTest extends DatabaseTestCase ->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'); + $expected = $this->replaceQuotes('SELECT * FROM [[operations]] ORDER BY [[name]], [[date]] DESC'); $this->assertEquals($expected, $sql); $this->assertEmpty($params); @@ -611,7 +611,7 @@ class QueryBuilderTest extends DatabaseTestCase ->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'); + $expected = $this->replaceQuotes('SELECT * FROM [[operations]] ORDER BY [[name]], [[date]] DESC'); $this->assertEquals($expected, $sql); $this->assertEmpty($params); @@ -645,7 +645,7 @@ class QueryBuilderTest extends DatabaseTestCase ->from('operations') ->groupBy('name, date'); list ($sql, $params) = $this->getQueryBuilder()->build($query); - $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY `name`, `date`'); + $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY [[name]], [[date]]'); $this->assertEquals($expected, $sql); $this->assertEmpty($params); @@ -655,7 +655,7 @@ class QueryBuilderTest extends DatabaseTestCase ->from('operations') ->groupBy(['name', 'date']); list ($sql, $params) = $this->getQueryBuilder()->build($query); - $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY `name`, `date`'); + $expected = $this->replaceQuotes('SELECT * FROM [[operations]] GROUP BY [[name]], [[date]]'); $this->assertEquals($expected, $sql); $this->assertEmpty($params); From 8680f0f1d8fd8b6b48c91e3352c20c6c598a43f1 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Wed, 27 Jan 2016 17:11:23 +0100 Subject: [PATCH 4/4] BC way of merging expression params of orderBy and groupBy this logic should be moved to the sub methods in 2.1 --- framework/db/QueryBuilder.php | 23 +++++++++++++++-------- framework/db/sqlite/QueryBuilder.php | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/framework/db/QueryBuilder.php b/framework/db/QueryBuilder.php index 3aeb57fee3..6bc3a9ce33 100644 --- a/framework/db/QueryBuilder.php +++ b/framework/db/QueryBuilder.php @@ -101,6 +101,21 @@ class QueryBuilder extends \yii\base\Object $sql = implode($this->separator, array_filter($clauses)); $sql = $this->buildOrderByAndLimit($sql, $query->orderBy, $query->limit, $query->offset); + if (!empty($query->orderBy)) { + foreach ($query->orderBy as $expression) { + if ($expression instanceof Expression) { + $params = array_merge($params, $expression->params); + } + } + } + if (!empty($query->groupBy)) { + foreach ($query->groupBy as $expression) { + if ($expression instanceof Expression) { + $params = array_merge($params, $expression->params); + } + } + } + $union = $this->buildUnion($query->union, $params); if ($union !== '') { $sql = "($sql){$this->separator}$union"; @@ -757,10 +772,6 @@ class QueryBuilder extends \yii\base\Object foreach ($columns as $i => $column) { if ($column instanceof Expression) { $columns[$i] = $column->expression; - // TODO -// foreach ($direction->params as $n => $v) { -// $params[$n] = $v; -// } } elseif (strpos($column, '(') === false) { $columns[$i] = $this->db->quoteColumnName($column); } @@ -814,10 +825,6 @@ class QueryBuilder extends \yii\base\Object foreach ($columns as $name => $direction) { if ($direction instanceof Expression) { $orders[] = $direction->expression; - // TODO -// foreach ($direction->params as $n => $v) { -// $params[$n] = $v; -// } } else { $orders[] = $this->db->quoteColumnName($name) . ($direction === SORT_DESC ? ' DESC' : ''); } diff --git a/framework/db/sqlite/QueryBuilder.php b/framework/db/sqlite/QueryBuilder.php index 3f00e21731..ec8664f4ae 100644 --- a/framework/db/sqlite/QueryBuilder.php +++ b/framework/db/sqlite/QueryBuilder.php @@ -11,6 +11,7 @@ use yii\db\Connection; use yii\db\Exception; use yii\base\InvalidParamException; use yii\base\NotSupportedException; +use yii\db\Expression; use yii\db\Query; /** @@ -379,6 +380,21 @@ class QueryBuilder extends \yii\db\QueryBuilder $sql = implode($this->separator, array_filter($clauses)); $sql = $this->buildOrderByAndLimit($sql, $query->orderBy, $query->limit, $query->offset); + if (!empty($query->orderBy)) { + foreach ($query->orderBy as $expression) { + if ($expression instanceof Expression) { + $params = array_merge($params, $expression->params); + } + } + } + if (!empty($query->groupBy)) { + foreach ($query->groupBy as $expression) { + if ($expression instanceof Expression) { + $params = array_merge($params, $expression->params); + } + } + } + $union = $this->buildUnion($query->union, $params); if ($union !== '') { $sql = "$sql{$this->separator}$union";