From 18b0ab6ca16c3aba94e243f6d00668de69f2d75d Mon Sep 17 00:00:00 2001 From: Jose Lorenzo Rodriguez Date: Sat, 19 Dec 2015 22:22:55 +0100 Subject: [PATCH] Fixing pagination with complex queries having expressions in order by Closes #7872 --- src/Database/Expression/QueryExpression.php | 16 +++++++++++++ src/ORM/Query.php | 6 +++++ .../Expression/QueryExpressionTest.php | 21 ++++++++++++++++ tests/TestCase/ORM/QueryRegressionTest.php | 24 +++++++++++++++++++ 4 files changed, 67 insertions(+) diff --git a/src/Database/Expression/QueryExpression.php b/src/Database/Expression/QueryExpression.php index a0d71adeed0..780876878a5 100644 --- a/src/Database/Expression/QueryExpression.php +++ b/src/Database/Expression/QueryExpression.php @@ -511,6 +511,22 @@ public function isCallable($c) return is_array($c) && isset($c[0]) && is_object($c[0]) && is_callable($c); } + /** + * Returns true if this expression contains any other nested + * ExpressionInterface objects + * + * @return boolean + */ + public function hasNestedExpression() + { + foreach ($this->_conditions as $c) { + if ($c instanceof ExpressionInterface) { + return true; + } + } + return false; + } + /** * Auxiliary function used for decomposing a nested array of conditions and build * a tree structure inside this object to represent the full SQL expression. diff --git a/src/ORM/Query.php b/src/ORM/Query.php index cceba5db49c..eb5f1966fcb 100644 --- a/src/ORM/Query.php +++ b/src/ORM/Query.php @@ -723,6 +723,7 @@ protected function _performCount() count($query->clause('union')) || $query->clause('having') ); + if (!$complex) { // Expression fields could have bound parameters. foreach ($query->clause('select') as $field) { @@ -733,6 +734,11 @@ protected function _performCount() } } + if (!$complex && $this->_valueBinder !== null) { + $order = $this->clause('order'); + $complex = $order === null ? false : $order->hasNestedExpression(); + } + $count = ['count' => $query->func()->count('*')]; if (!$complex) { diff --git a/tests/TestCase/Database/Expression/QueryExpressionTest.php b/tests/TestCase/Database/Expression/QueryExpressionTest.php index 77837850a89..b3578dd2980 100644 --- a/tests/TestCase/Database/Expression/QueryExpressionTest.php +++ b/tests/TestCase/Database/Expression/QueryExpressionTest.php @@ -112,4 +112,25 @@ public function testDeepCloning() $this->assertNotSame($originalParts[$i], $part); }); } + + /** + * Tests the hasNestedExpression() function + * + * @return void + */ + public function testHasNestedExpression() + { + $expr = new QueryExpression(); + $this->assertFalse($expr->hasNestedExpression()); + + $expr->add(['a' => 'b']); + $this->assertTrue($expr->hasNestedExpression()); + + $expr = new QueryExpression(); + $expr->add('a = b'); + $this->assertFalse($expr->hasNestedExpression()); + + $expr->add(new QueryExpression('1 = 1')); + $this->assertTrue($expr->hasNestedExpression()); + } } diff --git a/tests/TestCase/ORM/QueryRegressionTest.php b/tests/TestCase/ORM/QueryRegressionTest.php index cbaa3f712df..e98ca58d49b 100644 --- a/tests/TestCase/ORM/QueryRegressionTest.php +++ b/tests/TestCase/ORM/QueryRegressionTest.php @@ -1206,4 +1206,28 @@ public function testFormatResultsMemory() $endMemory = memory_get_usage() / 1024 / 1024; $this->assertWithinRange($endMemory, $memory, 1.25, 'Memory leak in ResultSet'); } + + /** + * Tests that having bound placeholders in the order clause does not result + * in an error when trying to count a query. + * + * @return void + */ + public function testCountWithComplexOrderBy() + { + $table = TableRegistry::get('Articles'); + $query = $table->find(); + $query->orderDesc($query->newExpr()->add(['id' => 3])); + $query->order(['title' => 'desc']); + // Executing the normal query before getting the count + $query->all(); + $this->assertEquals(3, $query->count()); + + $table = TableRegistry::get('Articles'); + $query = $table->find(); + $query->orderDesc($query->newExpr()->add(['id' => 3])); + $query->order(['title' => 'desc']); + // Not executing the query first, just getting the count + $this->assertEquals(3, $query->count()); + } }