From ce2fc6c83dca564ee7a46235d5c1fbb3c785617e Mon Sep 17 00:00:00 2001 From: Majna Date: Sun, 27 May 2012 23:15:20 +0200 Subject: [PATCH 1/3] Improve belongsTo and hasOne for unneeded queries when recursive > 1. Reuse already joined data for 'belongsTo' and 'hasOne' associations instead of running unneeded queries for each record. Fixes #47 --- lib/Cake/Model/Datasource/DboSource.php | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index 193bf2d75..ecccf0dc3 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -1080,6 +1080,7 @@ class DboSource extends DataSource { $query = trim($this->generateAssociationQuery($model, null, null, null, null, $queryData, false, $null)); $resultSet = $this->fetchAll($query, $model->cacheQueries); + if ($resultSet === false) { $model->onError(); return false; @@ -1092,6 +1093,10 @@ class DboSource extends DataSource { } if ($model->recursive > -1) { + $joined = array(); + if (isset($queryData['joins'][0]['alias'])) { + $joined[$model->alias] = (array)Hash::extract($queryData['joins'], '{n}.alias'); + } foreach ($_associations as $type) { foreach ($model->{$type} as $assoc => $assocData) { $linkModel = $model->{$assoc}; @@ -1108,6 +1113,7 @@ class DboSource extends DataSource { if (isset($db) && method_exists($db, 'queryAssociation')) { $stack = array($assoc); + $stack['joined'] = $joined; $db->queryAssociation($model, $linkModel, $type, $assoc, $assocData, $array, true, $resultSet, $model->recursive - 1, $stack); unset($db); @@ -1176,6 +1182,11 @@ class DboSource extends DataSource { * @throws CakeException when results cannot be created. */ public function queryAssociation(Model $model, &$linkModel, $type, $association, $assocData, &$queryData, $external, &$resultSet, $recursive, $stack) { + if (isset($stack['joined'])) { + $joined = $stack['joined']; + unset($stack['joined']); + } + if ($query = $this->generateAssociationQuery($model, $linkModel, $type, $association, $assocData, $queryData, $external, $resultSet)) { if (!is_array($resultSet)) { throw new CakeException(__d('cake_dev', 'Error in Model %s', get_class($model))); @@ -1251,10 +1262,17 @@ class DboSource extends DataSource { foreach ($resultSet as &$row) { if ($type !== 'hasAndBelongsToMany') { $q = $this->insertQueryData($query, $row, $association, $assocData, $model, $linkModel, $stack); + $fetch = null; if ($q !== false) { - $fetch = $this->fetchAll($q, $model->cacheQueries); - } else { - $fetch = null; + $joinedData = array(); + if (($type === 'belongsTo' || $type === 'hasOne') && isset($row[$linkModel->alias], $joined[$model->alias]) && in_array($linkModel->alias, $joined[$model->alias])) { + $joinedData = Hash::filter($row[$linkModel->alias]); + if (!empty($joinedData)) { + $fetch[0] = array($linkModel->alias => $row[$linkModel->alias]); + } + } else { + $fetch = $this->fetchAll($q, $model->cacheQueries); + } } } $selfJoin = $linkModel->name === $model->name; From 364c293e16c0f5286eee45b286e58d2e9ffa5396 Mon Sep 17 00:00:00 2001 From: Majna Date: Sun, 27 May 2012 23:16:20 +0200 Subject: [PATCH 2/3] Add test for belongsTo and hasOne optimization. --- .../Case/Model/Datasource/DboSourceTest.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php index c74f037df..ad83ca57e 100644 --- a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php +++ b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php @@ -767,6 +767,62 @@ class DboSourceTest extends CakeTestCase { $this->assertTrue(is_array($result)); } +/** + * test that queryAssociation() reuse already joined data for 'belongsTo' and 'hasOne' associations + * instead of running unneeded queries for each record + * + * @return void + */ + public function testQueryAssociationUnneededQueries() { + $this->loadFixtures('Article', 'User', 'Comment', 'Attachment', 'Tag', 'ArticlesTag'); + $Comment = new Comment; + + $fullDebug = $this->db->fullDebug; + $this->db->fullDebug = true; + + $Comment->find('all', array('recursive' => 2)); // ensure Model descriptions are saved + $this->db->getLog(); + + // case: Comment belongsTo User and Article + $Comment->unbindModel(array( + 'hasOne' => array('Attachment') + )); + $Comment->Article->unbindModel(array( + 'belongsTo' => array('User'), + 'hasMany' => array('Comment'), + 'hasAndBelongsToMany' => array('Tag') + )); + $Comment->find('all', array('recursive' => 2)); + $log = $this->db->getLog(); + $this->assertEquals(1, count($log['log'])); + + // case: Comment belongsTo Article, Article belongsTo User + $Comment->unbindModel(array( + 'belongsTo' => array('User'), + 'hasOne' => array('Attachment') + )); + $Comment->Article->unbindModel(array( + 'hasMany' => array('Comment'), + 'hasAndBelongsToMany' => array('Tag'), + )); + $Comment->find('all', array('recursive' => 2)); + $log = $this->db->getLog(); + $this->assertEquals(7, count($log['log'])); + + // case: Comment hasOne Attachment + $Comment->unbindModel(array( + 'belongsTo' => array('Article', 'User'), + )); + $Comment->Attachment->unbindModel(array( + 'belongsTo' => array('Comment'), + )); + $Comment->find('all', array('recursive' => 2)); + $log = $this->db->getLog(); + $this->assertEquals(1, count($log['log'])); + + $this->db->fullDebug = $fullDebug; + } + /** * test that fields() is using methodCache() * From 115b2c14958df8c9b7a7552fcd5c2c85b45f4ef9 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sun, 27 May 2012 20:43:15 -0400 Subject: [PATCH 3/3] Rename joined to _joined to hopefully prevent issues. --- lib/Cake/Model/Datasource/DboSource.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index ecccf0dc3..da4a7f734 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -1113,7 +1113,7 @@ class DboSource extends DataSource { if (isset($db) && method_exists($db, 'queryAssociation')) { $stack = array($assoc); - $stack['joined'] = $joined; + $stack['_joined'] = $joined; $db->queryAssociation($model, $linkModel, $type, $assoc, $assocData, $array, true, $resultSet, $model->recursive - 1, $stack); unset($db); @@ -1182,9 +1182,9 @@ class DboSource extends DataSource { * @throws CakeException when results cannot be created. */ public function queryAssociation(Model $model, &$linkModel, $type, $association, $assocData, &$queryData, $external, &$resultSet, $recursive, $stack) { - if (isset($stack['joined'])) { - $joined = $stack['joined']; - unset($stack['joined']); + if (isset($stack['_joined'])) { + $joined = $stack['_joined']; + unset($stack['_joined']); } if ($query = $this->generateAssociationQuery($model, $linkModel, $type, $association, $assocData, $queryData, $external, $resultSet)) {