tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-09-28 20:29:15 +02:00
Code Issues Projects Releases Wiki Activity

BUGFIX: Fixed errors caused by complex raw SQL sort() calls. (#7236)

Browse Source
This commit is contained in:
Sam Minnee 2012-05-01 17:09:57 +12:00
parent 5abf8cf0f3
commit a8e8a6060a
5 changed files with 69 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
model/DataList.php
View File

@ -189,7 +189,13 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
$this->dataQuery->sort(null, null); // wipe the sort
foreach(func_get_arg(0) as $col => $dir) {
$this->dataQuery->sort($this->getRelationName($col), $dir, false);
// Convert column expressions to SQL fragment, while still allowing the passing of raw SQL fragments.
try {
$relCol = $this->getRelationName($col);
} catch(InvalidArgumentException $e) {
$relCol = $col;
}
$this->dataQuery->sort($relCol, $dir, false);
}
}
@ -250,12 +256,16 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Translates a Object relation name to a Database name and apply the relation join to
* the query
* the query. Throws an InvalidArgumentException if the $field doesn't correspond to a relation
*
* @param string $field
* @return string
*/
public function getRelationName($field) {
if(!preg_match('/^[A-Z0-9._]+$/i', $field)) {
throw new InvalidArgumentException("Bad field expression $field");
}
if(strpos($field,'.') === false) {
return '"'.$field.'"';
}

16
model/DataQuery.php
View File

@ -229,7 +229,7 @@ class DataQuery {
* @param SQLQuery $query
* @return null
*/
protected function ensureSelectContainsOrderbyColumns($query) {
protected function ensureSelectContainsOrderbyColumns($query, $originalSelect = array()) {
$tableClasses = ClassInfo::dataClassesFor($this->dataClass);
$baseClass = array_shift($tableClasses);
@ -239,8 +239,13 @@ class DataQuery {
foreach($orderby as $k => $dir) {
// don't touch functions in the ORDER BY or function calls
// selected as fields
if(strpos($k, '(') !== false || preg_match('/_SortColumn/', $k))
if(strpos($k, '(') !== false) continue;
// Pull through SortColumn references from the originalSelect variables
if(preg_match('/_SortColumn/', $k)) {
if(isset($originalSelect[$k])) $query->selectField($originalSelect[$k], $k);
continue;
}
$col = str_replace('"', '', trim($k));
$parts = explode('.', $col);
@ -616,8 +621,11 @@ class DataQuery {
*/
public function column($field = 'ID') {
$query = $this->getFinalisedQuery(array($field));
$query->select($this->expressionForField($field, $query));
$this->ensureSelectContainsOrderbyColumns($query);
$originalSelect = $query->itemisedSelect();
$fieldExpression = $this->expressionForField($field, $query);
$query->clearSelect();
$query->selectField($fieldExpression);
$this->ensureSelectContainsOrderbyColumns($query, $originalSelect);
return $query->execute()->column($field);
}

36
model/SQLQuery.php
View File

@ -343,13 +343,12 @@ class SQLQuery {
else {
$sort = explode(",", $clauses);
}
$clauses = array();
foreach($sort as $clause) {
$clause = explode(" ", trim($clause));
$dir = (isset($clause[1])) ? $clause[1] : $direction;
$clauses[$clause[0]] = $dir;
list($column, $direction) = $this->getDirectionFromString($clause, $direction);
$clauses[$column] = $direction;
}
}
@ -357,16 +356,13 @@ class SQLQuery {
foreach($clauses as $key => $value) {
if(!is_numeric($key)) {
$column = trim($key);
$direction = strtoupper(trim($value));
$columnDir = strtoupper(trim($value));
}
else {
$clause = explode(" ", trim($value));
$column = trim($clause[0]);
$direction = (isset($clause[1])) ? strtoupper(trim($clause[1])) : "";
list($column, $columnDir) = $this->getDirectionFromString($value);
}
$this->orderby[$column] = $direction;
$this->orderby[$column] = $columnDir;
}
}
else {
@ -380,9 +376,9 @@ class SQLQuery {
// directly in the ORDER BY
if($this->orderby) {
$i = 0;
foreach($this->orderby as $clause => $dir) {
if(strpos($clause, '(') !== false) {
// Function calls and multi-word columns like "CASE WHEN ..."
if(strpos($clause, '(') !== false || strpos($clause, " ") !== false ) {
// remove the old orderby
unset($this->orderby[$clause]);
@ -399,6 +395,22 @@ class SQLQuery {
return $this;
}
/**
* Extract the direction part of a single-column order by clause.
*
* Return a 2 element array: array($column, $direction)
*/
private function getDirectionFromString($value, $defaultDirection = null) {
if(preg_match('/^(.*)(asc|desc)$/i', $value, $matches)) {
$column = trim($matches[1]);
$direction = strtoupper($matches[2]);
} else {
$column = $value;
$direction = $defaultDirection ? $defaultDirection : "ASC";
}
return array($column, $direction);
}
/**
* Returns the current order by as array if not already. To handle legacy
* statements which are stored as strings. Without clauses and directions,

14
tests/model/DataListTest.php
View File

@ -515,4 +515,18 @@ class DataListTest extends SapphireTest {
$this->assertEquals('Bob', $list->last()->Name, 'Last comment should be from Bob');
$this->assertEquals('Phil', $list->first()->Name, 'First comment should be from Phil');
}
public function testSortByComplexExpression() {
// Test an expression with both spaces and commas
// This test also tests that column() can be called with a complex sort expression, so keep using column() below
$list = DataObjectTest_Team::get()->sort('CASE WHEN "DataObjectTest_Team"."ClassName" = \'DataObjectTest_SubTeam\' THEN 0 ELSE 1 END, "Title" DESC');
$this->assertEquals(array(
'Subteam 3',
'Subteam 2',
'Subteam 1',
'Team 3',
'Team 2',
'Team 1',
), $list->column("Title"));
}
}

14
tests/model/SQLQueryTest.php
View File

@ -102,7 +102,7 @@ class SQLQueryTest extends SapphireTest {
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby('MyName');
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName', $query->sql());
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName ASC', $query->sql());
$query = new SQLQuery();
$query->from[] = "MyTable";
@ -117,7 +117,7 @@ class SQLQueryTest extends SapphireTest {
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby('MyName ASC, Color');
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName ASC, Color', $query->sql());
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName ASC, Color ASC', $query->sql());
$query = new SQLQuery();
$query->from[] = "MyTable";
@ -127,23 +127,23 @@ class SQLQueryTest extends SapphireTest {
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby(array('MyName' => 'desc', 'Color'));
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName DESC, Color', $query->sql());
$this->assertEquals('SELECT * FROM MyTable ORDER BY MyName DESC, Color ASC', $query->sql());
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby('implode("MyName","Color")');
$this->assertEquals('SELECT implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0', $query->sql());
$this->assertEquals('SELECT *, implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 ASC', $query->sql());
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby('implode("MyName","Color") DESC');
$this->assertEquals('SELECT implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 DESC', $query->sql());
$this->assertEquals('SELECT *, implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 DESC', $query->sql());
$query = new SQLQuery();
$query->from[] = "MyTable";
$query->orderby('RAND()');
$this->assertEquals('SELECT RAND() AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0', $query->sql());
$this->assertEquals('SELECT *, RAND() AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 ASC', $query->sql());
}
public function testReverseOrderBy() {
@ -174,7 +174,7 @@ class SQLQueryTest extends SapphireTest {
$query->orderby('implode("MyName","Color") DESC');
$query->reverseOrderBy();
$this->assertEquals('SELECT implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 ASC',$query->sql());
$this->assertEquals('SELECT *, implode("MyName","Color") AS "_SortColumn0" FROM MyTable ORDER BY _SortColumn0 ASC',$query->sql());
}
function testFiltersOnID() {