Permalink
Browse files

BUG Fixing SQLQuery::aggregate() adding ORDER BY when no limit.

DataQuery::initialiseQuery() will add a default sort to a query,
and when calling up an aggregate it will make a query like this
which doesn't make sense:

SELECT MAX("LastEdited") FROM "Member" ORDER BY "ID"

In this case there is no need to add the ORDER BY, and it will
break databases like MSSQL in cases such as
GenericTemplateGlobalProvider
which provides a default List() function for adding aggregates
into SSViewer template cacheblocks.

If we add a limit, however, then it does make sense:

SELECT MAX("LastEdited") FROM "Member" ORDER BY "ID" LIMIT 10

This fixes SQLQuery::aggregate() to NOT add an ORDER BY to an
aggregate call if there is no limit.
  • Loading branch information...
1 parent e43ca93 commit 95bb799e6fe9c4497a3e5fd1a12d4c3bde7f073c @halkyon halkyon committed Sep 6, 2013
Showing with 38 additions and 4 deletions.
  1. +13 −4 model/SQLQuery.php
  2. +25 −0 tests/model/SQLQueryTest.php
View
@@ -1066,18 +1066,27 @@ public function count( $column = null) {
* @param $alias An optional alias for the aggregate column.
*/
public function aggregate($column, $alias = null) {
-
$clone = clone $this;
- $clone->setLimit($this->limit);
- $clone->setOrderBy($this->orderby);
+
+ // don't set an ORDER BY clause if no limit has been set. It doesn't make
+ // sense to add an ORDER BY if there is no limit, and it will break
+ // queries to databases like MSSQL if you do so. Note that the reason
+ // this came up is because DataQuery::initialiseQuery() introduces
+ // a default sort.
+ if($this->limit) {
+ $clone->setLimit($this->limit);
+ $clone->setOrderBy($this->orderby);
+ } else {
+ $clone->setOrderBy(array());
+ }
+
$clone->setGroupBy($this->groupby);
if($alias) {
$clone->setSelect(array());
$clone->selectField($column, $alias);
} else {
$clone->setSelect($column);
}
-
return $clone;
}
@@ -415,6 +415,31 @@ public function testAggregate() {
}
/**
+ * Tests that an ORDER BY is only added if a LIMIT is set.
+ */
+ public function testAggregateNoOrderByIfNoLimit() {
+ $query = new SQLQuery();
+ $query->setFrom('"SQLQueryTest_DO"');
+ $query->setOrderBy('Common');
+ $query->setLimit(array());
+
+ $aggregate = $query->aggregate('MAX("ID")');
+ $limit = $aggregate->getLimit();
+ $this->assertEquals(array(), $aggregate->getOrderBy());
+ $this->assertEquals(array(), $limit);
+
+ $query = new SQLQuery();
+ $query->setFrom('"SQLQueryTest_DO"');
+ $query->setOrderBy('Common');
+ $query->setLimit(2);
+
+ $aggregate = $query->aggregate('MAX("ID")');
+ $limit = $aggregate->getLimit();
+ $this->assertEquals(array('Common' => 'ASC'), $aggregate->getOrderBy());
+ $this->assertEquals(array('start' => 0, 'limit' => 2), $limit);
+ }
+
+ /**
* Test that "_SortColumn0" is added for an aggregate in the ORDER BY
* clause, in combination with a LIMIT and GROUP BY clause.
* For some databases, like MSSQL, this is a complicated scenario

0 comments on commit 95bb799

Please sign in to comment.