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.
This commit is contained in:
Sean Harvey 2013-09-06 18:01:52 +12:00
parent e43ca931d6
commit 95bb799e6f
2 changed files with 38 additions and 4 deletions

View File

@ -1066,10 +1066,20 @@ class SQLQuery {
* @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());
@ -1077,7 +1087,6 @@ class SQLQuery {
} else {
$clone->setSelect($column);
}
return $clone;
}

View File

@ -414,6 +414,31 @@ class SQLQueryTest extends SapphireTest {
$this->assertEquals(array(2), $result->column('cnt'));
}
/**
* 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.