From 23fc498c275db92ba182be3bf13e672b20866ba3 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 3 Oct 2014 09:33:18 +0100 Subject: [PATCH] NEW: Allow 'null' limit for database queries (closes #3487) --- _config/database.yml | 4 +-- model/connect/MySQLQueryBuilder.php | 56 +++++++++++++++++++++++++++++ model/queries/SQLSelect.php | 10 ++++-- tests/model/DataListTest.php | 9 +++-- 4 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 model/connect/MySQLQueryBuilder.php diff --git a/_config/database.yml b/_config/database.yml index d4a257d45..916dd2c57 100644 --- a/_config/database.yml +++ b/_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 diff --git a/model/connect/MySQLQueryBuilder.php b/model/connect/MySQLQueryBuilder.php new file mode 100644 index 000000000..a11e368b3 --- /dev/null +++ b/model/connect/MySQLQueryBuilder.php @@ -0,0 +1,56 @@ +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; + } + +} diff --git a/model/queries/SQLSelect.php b/model/queries/SQLSelect.php index 17c09fcdd..784c78306 100644 --- a/model/queries/SQLSelect.php +++ b/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']); diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index 3ddd195b6..1132c3be6 100755 --- a/tests/model/DataListTest.php +++ b/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); - // $this->assertEquals(1, $check->count()); + $check = $list->limit(null, 2); + $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() {