From 65301365a0e80920cbc5fd8701840cb9dca7959a Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Thu, 17 Mar 2016 00:12:40 +0100 Subject: [PATCH 1/2] added failing test for #11088 --- tests/data/ar/Order.php | 4 ++- tests/data/ar/OrderItem.php | 4 ++- tests/framework/db/ActiveRecordTest.php | 35 +++++++++++++++++++++++++ tests/framework/db/DatabaseTestCase.php | 22 ++++++++++++++++ tests/framework/db/QueryBuilderTest.php | 22 ---------------- 5 files changed, 63 insertions(+), 24 deletions(-) diff --git a/tests/data/ar/Order.php b/tests/data/ar/Order.php index 17dfb5d6db7..20ea8e15a36 100644 --- a/tests/data/ar/Order.php +++ b/tests/data/ar/Order.php @@ -12,9 +12,11 @@ */ class Order extends ActiveRecord { + public static $tableName; + public static function tableName() { - return 'order'; + return static::$tableName ?: 'order'; } public function getCustomer() diff --git a/tests/data/ar/OrderItem.php b/tests/data/ar/OrderItem.php index 4c45f8839c8..ebb590cf9d8 100644 --- a/tests/data/ar/OrderItem.php +++ b/tests/data/ar/OrderItem.php @@ -12,9 +12,11 @@ */ class OrderItem extends ActiveRecord { + public static $tableName; + public static function tableName() { - return 'order_item'; + return static::$tableName ?: 'order_item'; } public function getOrder() diff --git a/tests/framework/db/ActiveRecordTest.php b/tests/framework/db/ActiveRecordTest.php index c6a226e2410..1d0ff38d7d4 100644 --- a/tests/framework/db/ActiveRecordTest.php +++ b/tests/framework/db/ActiveRecordTest.php @@ -815,6 +815,41 @@ public function testJoinWithSameTable() $this->assertEquals(0, count($orders[0]->itemsIndexed)); } + public function tableNameProvider() + { + return [ + ['order', 'order_item'], + ['order', '{{%order_item}}'], + ['{{%order}}', 'order_item'], + ['{{%order}}', '{{%order_item}}'], + ]; + } + + /** + * Test whether conditions are quoted correctly in conditions where joinWith is used. + * @see https://github.com/yiisoft/yii2/issues/11088 + * @dataProvider tableNameProvider + */ + public function testRelationWhereParams($orderTableName, $orderItemTableName) + { + Order::$tableName = $orderTableName; + OrderItem::$tableName = $orderItemTableName; + + /** @var $order Order */ + $order = Order::findOne(1); + $itemsSQL = $order->getOrderitems()->createCommand()->rawSql; + $expectedSQL = $this->replaceQuotes("SELECT * FROM [[order_item]] WHERE [[order_id]]=1"); + $this->assertEquals($expectedSQL, $itemsSQL); + + $order = Order::findOne(1); + $itemsSQL = $order->getOrderItems()->joinWith('item')->createCommand()->rawSql; + $expectedSQL = $this->replaceQuotes("SELECT [[order_item]].* FROM [[order_item]] LEFT JOIN [[item]] ON [[order_item]].[[item_id]] = [[item]].[[id]] WHERE [[order_item]].[[order_id]]=1"); + $this->assertEquals($expectedSQL, $itemsSQL); + + Order::$tableName = null; + OrderItem::$tableName = null; + } + public function testAlias() { $query = Order::find(); diff --git a/tests/framework/db/DatabaseTestCase.php b/tests/framework/db/DatabaseTestCase.php index e500e6c08a6..ec80f95cebd 100644 --- a/tests/framework/db/DatabaseTestCase.php +++ b/tests/framework/db/DatabaseTestCase.php @@ -89,4 +89,26 @@ public function prepareDatabase($config, $fixture, $open = true) } return $db; } + + /** + * adjust dbms specific escaping + * @param $sql + * @return mixed + */ + protected function replaceQuotes($sql) + { + switch ($this->driverName) { + case 'mysql': + case 'sqlite': + return str_replace(['[[', ']]'], '`', $sql); + case 'cubrid': + case 'pgsql': + case 'oci': + return str_replace(['[[', ']]'], '"', $sql); + case 'sqlsrv': + return str_replace(['[[', ']]'], ['[', ']'], $sql); + default: + return $sql; + } + } } diff --git a/tests/framework/db/QueryBuilderTest.php b/tests/framework/db/QueryBuilderTest.php index 42857058c84..a73a1eae74b 100644 --- a/tests/framework/db/QueryBuilderTest.php +++ b/tests/framework/db/QueryBuilderTest.php @@ -54,28 +54,6 @@ protected function getQueryBuilder() throw new \Exception('Test is not implemented for ' . $this->driverName); } - /** - * adjust dbms specific escaping - * @param $sql - * @return mixed - */ - protected function replaceQuotes($sql) - { - switch ($this->driverName) { - case 'mysql': - case 'sqlite': - return str_replace(['[[', ']]'], '`', $sql); - case 'cubrid': - case 'pgsql': - case 'oci': - return str_replace(['[[', ']]'], '"', $sql); - case 'sqlsrv': - return str_replace(['[[', ']]'], ['[', ']'], $sql); - default: - return $sql; - } - } - /** * this is not used as a dataprovider for testGetColumnType to speed up the test * when used as dataprovider every single line will cause a reconnect with the database which is not needed here From 85cb331414076feca52e6c50ba86046a7918b3b6 Mon Sep 17 00:00:00 2001 From: "p.chapl" Date: Wed, 23 Mar 2016 18:12:54 +0600 Subject: [PATCH 2/2] fixes #11088 quote column name in "where" condition relation data with joinWith --- framework/db/ActiveQuery.php | 6 +++--- framework/db/ActiveRelationTrait.php | 13 ++++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/framework/db/ActiveQuery.php b/framework/db/ActiveQuery.php index 49211a68fd9..86809531e81 100644 --- a/framework/db/ActiveQuery.php +++ b/framework/db/ActiveQuery.php @@ -169,7 +169,7 @@ public function prepare($builder) if ($this->via instanceof self) { // via junction table $viaModels = $this->via->findJunctionRows([$this->primaryModel]); - $this->filterByModels($viaModels); + $this->filterByModels($viaModels, $builder); } elseif (is_array($this->via)) { // via relation /* @var $viaQuery ActiveQuery */ @@ -182,9 +182,9 @@ public function prepare($builder) $this->primaryModel->populateRelation($viaName, $model); $viaModels = $model === null ? [] : [$model]; } - $this->filterByModels($viaModels); + $this->filterByModels($viaModels, $builder); } else { - $this->filterByModels([$this->primaryModel]); + $this->filterByModels([$this->primaryModel], $builder); } $query = Query::create($this); diff --git a/framework/db/ActiveRelationTrait.php b/framework/db/ActiveRelationTrait.php index 22f971f8ffe..ac447db0544 100644 --- a/framework/db/ActiveRelationTrait.php +++ b/framework/db/ActiveRelationTrait.php @@ -419,9 +419,10 @@ private function indexBuckets($buckets, $indexBy) /** * @param array $attributes the attributes to prefix + * @param QueryBuilder|null $builder * @return array */ - private function prefixKeyColumns($attributes) + private function prefixKeyColumns($attributes, $builder = null) { if ($this instanceof ActiveQuery && (!empty($this->join) || !empty($this->joinWith))) { if (empty($this->from)) { @@ -438,6 +439,11 @@ private function prefixKeyColumns($attributes) } if (isset($alias)) { foreach ($attributes as $i => $attribute) { + + if (!empty($builder)) { + $attribute = $builder->db->quoteColumnName($attribute); + } + $attributes[$i] = "$alias.$attribute"; } } @@ -447,12 +453,13 @@ private function prefixKeyColumns($attributes) /** * @param array $models + * @param QueryBuilder|null $builder */ - private function filterByModels($models) + private function filterByModels($models, $builder = null) { $attributes = array_keys($this->link); - $attributes = $this->prefixKeyColumns($attributes); + $attributes = $this->prefixKeyColumns($attributes, $builder); $values = []; if (count($attributes) === 1) {