From 63a2bea7ec0171595f2a96639827e9d477fc6878 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Tue, 17 Dec 2013 00:44:35 +0100 Subject: [PATCH 1/2] Remove OC_DB_StatementWrapper::numRows(). Using this method will result in an unneccesary extra SQL query (which also may return an incorrect result because the underlying table changed in the meantime). In general: If you are performing an UPDATE, DELETE or equivalent query, OC_DB_StatementWrapper::execute() will already give you the number of "affected rows" via \Doctrine\DBAL\Driver\Statement::rowCount(). This will not work for SELECT queries, however. If you want to know whether a table contains any rows matching your condition, use "SELECT id FROM ... WHERE ... LIMIT 1". If you want to know whether a table contains any rows matching your condition and you also need the data, use "SELECT ... FROM ... WHERE ...", then use one of the fetch() methods. If you want to count the number of rows matching your condition, use use "SELECT COUNT(...) AS number_of_rows FROM ... WHERE ...", then use one of the fetch() methods. --- lib/private/db/statementwrapper.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/lib/private/db/statementwrapper.php b/lib/private/db/statementwrapper.php index b8da1afc0e..5e89261d93 100644 --- a/lib/private/db/statementwrapper.php +++ b/lib/private/db/statementwrapper.php @@ -29,25 +29,6 @@ class OC_DB_StatementWrapper { return call_user_func_array(array($this->statement,$name), $arguments); } - /** - * provide numRows - */ - public function numRows() { - $type = OC_Config::getValue( "dbtype", "sqlite" ); - if ($type == 'oci') { - // OCI doesn't have a queryString, just do a rowCount for now - return $this->statement->rowCount(); - } - $regex = '/^SELECT\s+(?:ALL\s+|DISTINCT\s+)?(?:.*?)\s+FROM\s+(.*)$/i'; - $queryString = $this->statement->getWrappedStatement()->queryString; - if (preg_match($regex, $queryString, $output) > 0) { - $query = OC_DB::prepare("SELECT COUNT(*) FROM {$output[1]}"); - return $query->execute($this->lastArguments)->fetchColumn(); - }else{ - return $this->statement->rowCount(); - } - } - /** * make execute return the result instead of a bool */ From 4bc2a22317dd65f142cf697cd10ed6920d930544 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Tue, 17 Dec 2013 01:28:05 +0100 Subject: [PATCH 2/2] Remove all uses of numRows(). --- tests/lib/db.php | 17 ++++++++--------- tests/lib/dbschema.php | 2 +- tests/lib/preferences.php | 8 ++++---- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/tests/lib/db.php b/tests/lib/db.php index 96d5f873b5..f0b271a36f 100644 --- a/tests/lib/db.php +++ b/tests/lib/db.php @@ -137,10 +137,10 @@ class Test_DB extends PHPUnit_Framework_TestCase { $query = OC_DB::prepare('SELECT `fullname`, `uri`, `carddata` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array($uri)); $this->assertTrue((bool)$result); - $row = $result->fetchRow(); - $this->assertArrayHasKey('carddata', $row); - $this->assertEquals($carddata, $row['carddata']); - $this->assertEquals(1, $result->numRows()); + $rowset = $result->fetchAll(); + $this->assertEquals(1, count($rowset)); + $this->assertArrayHasKey('carddata', $rowset[0]); + $this->assertEquals($carddata, $rowset[0]['carddata']); // Try to insert a new row $result = OC_DB::insertIfNotExist('*PREFIX*'.$this->table2, @@ -153,13 +153,12 @@ class Test_DB extends PHPUnit_Framework_TestCase { $query = OC_DB::prepare('SELECT `fullname`, `uri`, `carddata` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array($uri)); $this->assertTrue((bool)$result); - $row = $result->fetchRow(); - $this->assertArrayHasKey('carddata', $row); // Test that previously inserted data isn't overwritten - $this->assertEquals($carddata, $row['carddata']); // And that a new row hasn't been inserted. - $this->assertEquals(1, $result->numRows()); - + $rowset = $result->fetchAll(); + $this->assertEquals(1, count($rowset)); + $this->assertArrayHasKey('carddata', $rowset[0]); + $this->assertEquals($carddata, $rowset[0]['carddata']); } public function testUtf8Data() { diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 7de90c047c..4a7b7f7aac 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -103,7 +103,7 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { break; } - $name = $result->fetchOne(); //FIXME checking with '$result->numRows() === 1' does not seem to work? + $name = $result->fetchOne(); if ($name === $table) { return true; } else { diff --git a/tests/lib/preferences.php b/tests/lib/preferences.php index 68b794e9ea..a8236909de 100644 --- a/tests/lib/preferences.php +++ b/tests/lib/preferences.php @@ -101,28 +101,28 @@ class Test_Preferences extends PHPUnit_Framework_TestCase { $this->assertTrue(\OC_Preferences::deleteKey('Deleteuser', 'deleteapp', 'deletekey')); $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); $result = $query->execute(array('Deleteuser', 'deleteapp', 'deletekey')); - $this->assertEquals(0, $result->numRows()); + $this->assertEquals(0, count($result->fetchAll())); } public function testDeleteApp() { $this->assertTrue(\OC_Preferences::deleteApp('Deleteuser', 'deleteapp')); $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ?'); $result = $query->execute(array('Deleteuser', 'deleteapp')); - $this->assertEquals(0, $result->numRows()); + $this->assertEquals(0, count($result->fetchAll())); } public function testDeleteUser() { $this->assertTrue(\OC_Preferences::deleteUser('Deleteuser')); $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'); $result = $query->execute(array('Deleteuser')); - $this->assertEquals(0, $result->numRows()); + $this->assertEquals(0, count($result->fetchAll())); } public function testDeleteAppFromAllUsers() { $this->assertTrue(\OC_Preferences::deleteAppFromAllUsers('someapp')); $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `appid` = ?'); $result = $query->execute(array('someapp')); - $this->assertEquals(0, $result->numRows()); + $this->assertEquals(0, count($result->fetchAll())); } }