Browse Source

Fixing pagination with complex queries having expressions in order by
Closes #7872

Jose Lorenzo Rodriguez 10 years ago
parent
commit
18b0ab6ca1

+ 16 - 0
src/Database/Expression/QueryExpression.php

@@ -512,6 +512,22 @@ class QueryExpression implements ExpressionInterface, Countable
     }
 
     /**
+     * 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.
      * String conditions are stored directly in the conditions, while any other

+ 6 - 0
src/ORM/Query.php

@@ -723,6 +723,7 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
             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 @@ class Query extends DatabaseQuery implements JsonSerializable, QueryInterface
             }
         }
 
+        if (!$complex && $this->_valueBinder !== null) {
+            $order = $this->clause('order');
+            $complex = $order === null ? false : $order->hasNestedExpression();
+        }
+
         $count = ['count' => $query->func()->count('*')];
 
         if (!$complex) {

+ 21 - 0
tests/TestCase/Database/Expression/QueryExpressionTest.php

@@ -112,4 +112,25 @@ class QueryExpressionTest extends TestCase
             $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());
+    }
 }

+ 24 - 0
tests/TestCase/ORM/QueryRegressionTest.php

@@ -1206,4 +1206,28 @@ class QueryRegressionTest extends TestCase
         $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());
+    }
 }