Merge pull request #4157 from kinglozzer/pulls/orm-null-limit

NEW: Allow 'null' limit for database queries (closes #3487)

_config/database.yml

@@ -7,10 +7,10 @@ Injector:
     properties:
       connector: %$PDOConnector
       schemaManager: %$MySQLSchemaManager
-      queryBuilder: %$DBQueryBuilder
+      queryBuilder: %$MySQLQueryBuilder
   MySQLDatabase:
     class: 'MySQLDatabase'
     properties:
       connector: %$MySQLiConnector
       schemaManager: %$MySQLSchemaManager
-      queryBuilder: %$DBQueryBuilder
+      queryBuilder: %$MySQLQueryBuilder

model/connect/MySQLQueryBuilder.php

@@ -0,0 +1,56 @@
+<?php
+
+/**
+ * Builds a SQL query string from a SQLExpression object
+ *
+ * @package framework
+ * @subpackage model
+ */
+class MySQLQueryBuilder extends DBQueryBuilder {
+
+	/**
+	 * Max number of rows allowed in MySQL
+	 * @var string
+	 */
+	const MAX_ROWS = '18446744073709551615';
+
+	/**
+	 * Return the LIMIT clause ready for inserting into a query.
+	 *
+	 * @param SQLSelect $query The expression object to build from
+	 * @param array $parameters Out parameter for the resulting query parameters
+	 * @return string The finalised limit SQL fragment
+	 */
+	public function buildLimitFragment(SQLSelect $query, array &$parameters) {
+		$nl = $this->getSeparator();
+
+		// Ensure limit is given
+		$limit = $query->getLimit();
+		if(empty($limit)) return '';
+
+		// For literal values return this as the limit SQL
+		if (!is_array($limit)) {
+			return "{$nl}LIMIT $limit";
+		}
+
+		// Assert that the array version provides the 'limit' key
+		if (!array_key_exists('limit', $limit) || ($limit['limit'] !== null && ! is_numeric($limit['limit']))) {
+			throw new InvalidArgumentException(
+				'MySQLQueryBuilder::buildLimitSQL(): Wrong format for $limit: '. var_export($limit, true)
+			);
+		}
+
+		if($limit['limit'] === null) {
+			$limit['limit'] = self::MAX_ROWS;
+		}
+
+		// Format the array limit, given an optional start key
+		$clause = "{$nl}LIMIT {$limit['limit']}";
+		if(isset($limit['start']) && is_numeric($limit['start']) && $limit['start'] !== 0) {
+			$clause .= " OFFSET {$limit['start']}";
+		}
+
+		return $clause;
+	}
+
+}

model/queries/SQLSelect.php

@@ -234,8 +234,7 @@ class SQLSelect extends SQLConditionalExpression {
 		} else if($limit && is_string($limit)) {
 			if(strpos($limit, ',') !== false) {
 				list($start, $innerLimit) = explode(',', $limit, 2);
-			}
+			} else {
-			else {
 				list($innerLimit, $start) = explode(' OFFSET ', strtoupper($limit), 2);
 			}
 
@@ -243,6 +242,11 @@ class SQLSelect extends SQLConditionalExpression {
 				'start' => trim($start),
 				'limit' => trim($innerLimit),
 			);
+		} else if($limit === null && $offset) {
+			$this->limit = array(
+				'start' => $offset,
+				'limit' => $limit
+			);
 		} else {
 			$this->limit = $limit;
 		}
@@ -590,7 +594,7 @@ class SQLSelect extends SQLConditionalExpression {
 		$count = $clone->execute()->value();
 		// If there's a limit set, then that limit is going to heavily affect the count
 		if($this->limit) {
-			if($count >= ($this->limit['start'] + $this->limit['limit'])) {
+			if($this->limit['limit'] !== null && $count >= ($this->limit['start'] + $this->limit['limit'])) {
 				return $this->limit['limit'];
 			} else {
 				return max(0, $count - $this->limit['start']);

tests/model/DataListTest.php

@@ -122,9 +122,12 @@ class DataListTest extends SapphireTest {
 		$check = $list->limit(null);
 		$this->assertEquals(3, $check->count());
 
-		// no limit with an offset is currently not supported by the orm
+		$check = $list->limit(null, 2);
-		// $check = $list->limit(null, 2);
+		$this->assertEquals(1, $check->count());
-		// $this->assertEquals(1, $check->count());
+
+		// count()/first()/last() methods may alter limit/offset, so run the query and manually check the count
+		$check = $list->limit(null, 1)->toArray();
+		$this->assertEquals(2, count($check));
 	}
 
 	public function testDistinct() {