Skip to content

Commit

Permalink
Merge pull request #107 from moufmouf/bugfix/sort_on_external_col
Browse files Browse the repository at this point in the history
Adds an order by clause analyzer to be able to include external order by columns or expressions in order by columns.
  • Loading branch information
moufmouf committed Sep 23, 2016
2 parents 0565a2a + 8e1ed18 commit a83f910
Show file tree
Hide file tree
Showing 8 changed files with 293 additions and 12 deletions.
3 changes: 2 additions & 1 deletion composer.json
Expand Up @@ -32,7 +32,8 @@
"beberlei/porpaginas": "~1.0",
"mouf/classname-mapper": "~1.0",
"mouf/utils.common.doctrine-cache-wrapper": "~1.0",
"logger/essentials" : "^0.1.9"
"logger/essentials" : "^0.1.9",
"greenlion/php-sql-parser": "^4.0"
},
"require-dev" : {
"phpunit/phpunit": "~5.5",
Expand Down
2 changes: 1 addition & 1 deletion src/Mouf/Database/TDBM/AbstractTDBMObject.php
Expand Up @@ -496,7 +496,7 @@ private function removeManyToOneRelationship(string $tableName, string $foreignK
* @param string $foreignKeyName
* @param string $searchTableName
* @param array $searchFilter
* @param string $orderString The ORDER BY part of the query. All columns must be prefixed by the table name (in the form: table.column). WARNING : This parameter is not kept when there is an additionnal or removal object !
* @param string $orderString The ORDER BY part of the query. All columns must be prefixed by the table name (in the form: table.column). WARNING : This parameter is not kept when there is an additionnal or removal object !
*
* @return AlterableResultIterator
*/
Expand Down
6 changes: 6 additions & 0 deletions src/Mouf/Database/TDBM/InnerResultIterator.php
Expand Up @@ -136,6 +136,12 @@ public function next()
$beansData = [];
foreach ($row as $i => $value) {
$columnDescriptor = $this->columnDescriptors[$i];

if ($columnDescriptor['tableGroup'] === null) {
// A column can have no tableGroup (if it comes from an ORDER BY expression)
continue;
}

// Let's cast the value according to its type
$value = $columnDescriptor['type']->convertToPHPValue($value, $this->databasePlatform);

Expand Down
133 changes: 133 additions & 0 deletions src/Mouf/Database/TDBM/OrderByAnalyzer.php
@@ -0,0 +1,133 @@
<?php

namespace Mouf\Database\TDBM;

use Doctrine\Common\Cache\Cache;
use PHPSQLParser\PHPSQLParser;

/**
* Class in charge of analyzing order by clauses.
*
* Analyzing those clauses are important because an order by clause must be pushed into the select clause too.
*/
class OrderByAnalyzer
{
/**
* The content of the cache variable.
*
* @var Cache
*/
private $cache;

/**
* @var string
*/
private $cachePrefix;

/**
* OrderByAnalyzer constructor.
*
* @param Cache $cache
* @param string|null $cachePrefix
*/
public function __construct(Cache $cache, $cachePrefix = null)
{
$this->cache = $cache;
$this->cachePrefix = $cachePrefix;
}

/**
* Returns an array for each sorted "column" in the form:.
*
* [
* [
* 'type' => 'colref',
* 'table' => null,
* 'column' => 'a',
* 'direction' => 'ASC'
* ],
* [
* 'type' => 'expr',
* 'expr' => 'RAND()',
* 'direction' => 'DESC'
* ]
* ]
*
* @param string $orderBy
*
* @return array
*/
public function analyzeOrderBy(string $orderBy) : array
{
$key = $this->cachePrefix.'_order_by_analysis_'.$orderBy;
$results = $this->cache->fetch($key);
if ($results !== false) {
return $results;
}
$results = $this->analyzeOrderByNoCache($orderBy);
$this->cache->save($key, $results);

return $results;
}

private function analyzeOrderByNoCache(string $orderBy) : array
{
$sqlParser = new PHPSQLParser();
$sql = 'SELECT 1 FROM a ORDER BY '.$orderBy;
$parsed = $sqlParser->parse($sql, true);

$results = [];

for ($i = 0, $count = count($parsed['ORDER']); $i < $count; ++$i) {
$orderItem = $parsed['ORDER'][$i];
if ($orderItem['expr_type'] === 'colref') {
$parts = $orderItem['no_quotes']['parts'];
$columnName = array_pop($parts);
if (!empty($parts)) {
$tableName = array_pop($parts);
} else {
$tableName = null;
}

$results[] = [
'type' => 'colref',
'table' => $tableName,
'column' => $columnName,
'direction' => $orderItem['direction'],
];
} else {
$position = $orderItem['position'];
if ($i + 1 < $count) {
$nextPosition = $parsed['ORDER'][$i + 1]['position'];
$str = substr($sql, $position, $nextPosition - $position);
} else {
$str = substr($sql, $position);
}

$str = trim($str, " \t\r\n,");

$results[] = [
'type' => 'expr',
'expr' => $this->trimDirection($str),
'direction' => $orderItem['direction'],
];
}
}

return $results;
}

/**
* Trims the ASC/DESC direction at the end of the string.
*
* @param string $sql
*
* @return string
*/
private function trimDirection(string $sql) : string
{
preg_match('/^(.*)(\s+(DESC|ASC|))*$/Ui', $sql, $matches);

return $matches[1];
}
}
50 changes: 40 additions & 10 deletions src/Mouf/Database/TDBM/TDBMService.php
Expand Up @@ -115,9 +115,9 @@ class TDBMService
private $toSaveObjects;

/**
* The content of the cache variable.
* A cache service to be used.
*
* @var array<string, mixed>
* @var Cache|null
*/
private $cache;

Expand All @@ -143,6 +143,11 @@ class TDBMService
*/
private $logger;

/**
* @var OrderByAnalyzer
*/
private $orderByAnalyzer;

/**
* @param Connection $connection The DBAL DB connection to use
* @param Cache|null $cache A cache service to be used
Expand Down Expand Up @@ -181,6 +186,7 @@ public function __construct(Connection $connection, Cache $cache = null, SchemaA
$this->rootLogger = $logger;
$this->setLogLevel(LogLevel::WARNING);
}
$this->orderByAnalyzer = new OrderByAnalyzer($this->cache, $this->cachePrefix);
}

/**
Expand Down Expand Up @@ -1315,7 +1321,7 @@ public function findObjects($mainTable, $filter = null, array $parameters = arra

$parameters = array_merge($parameters, $additionalParameters);

list($columnDescList, $columnsList) = $this->getColumnsList($mainTable, $additionalTablesFetch);
list($columnDescList, $columnsList) = $this->getColumnsList($mainTable, $additionalTablesFetch, $orderString);

$sql = 'SELECT DISTINCT '.implode(', ', $columnsList).' FROM MAGICJOIN('.$mainTable.')';

Expand All @@ -1334,7 +1340,6 @@ public function findObjects($mainTable, $filter = null, array $parameters = arra

if (!empty($orderString)) {
$sql .= ' ORDER BY '.$orderString;
$countSql .= ' ORDER BY '.$orderString;
}

if ($mode !== null && $mode !== self::MODE_CURSOR && $mode !== self::MODE_ARRAY) {
Expand Down Expand Up @@ -1394,7 +1399,7 @@ public function findObjectsFromSql(string $mainTable, string $from, $filter = nu
}, $columnDescList)).' FROM '.$from;

if (count($allFetchedTables) > 1) {
list($columnDescList, $columnsList) = $this->getColumnsList($mainTable, []);
list($columnDescList, $columnsList) = $this->getColumnsList($mainTable, [], $orderString);
}

// Let's compute the COUNT.
Expand Down Expand Up @@ -1458,14 +1463,17 @@ public function findObjectsFromSql(string $mainTable, string $from, $filter = nu
/**
* Returns the column list that must be fetched for the SQL request.
*
* @param string $mainTable
* @param array $additionalTablesFetch
* Note: MySQL dictates that ORDER BYed columns should appear in the SELECT clause.
*
* @param string $mainTable
* @param array $additionalTablesFetch
* @param string|null $orderBy
*
* @return array
*
* @throws \Doctrine\DBAL\Schema\SchemaException
*/
private function getColumnsList(string $mainTable, array $additionalTablesFetch = array())
private function getColumnsList(string $mainTable, array $additionalTablesFetch = array(), string $orderBy = null)
{
// From the table name and the additional tables we want to fetch, let's build a list of all tables
// that must be part of the select columns.
Expand All @@ -1477,6 +1485,30 @@ private function getColumnsList(string $mainTable, array $additionalTablesFetch
$tableGroups[$table] = $tableGroupName;
}

$columnsList = [];
$columnDescList = [];
$sortColumn = 0;

// Now, let's deal with "order by columns"
if ($orderBy !== null) {
$orderByColumns = $this->orderByAnalyzer->analyzeOrderBy($orderBy);

// If we sort by a column, there is a high chance we will fetch the bean containing this column.
// Hence, we should add the table to the $additionalTablesFetch
foreach ($orderByColumns as $orderByColumn) {
if ($orderByColumn['type'] === 'colref' && $orderByColumn['table'] !== null) {
$additionalTablesFetch[] = $orderByColumn['table'];
} elseif ($orderByColumn['type'] === 'expr') {
$sortColumnName = 'sort_column_'.$sortColumn;
$columnsList[] = $orderByColumn['expr'].' as '.$sortColumnName;
$columnDescList[] = [
'tableGroup' => null,
];
++$sortColumn;
}
}
}

foreach ($additionalTablesFetch as $additionalTable) {
$relatedTables = $this->_getRelatedTablesByInheritance($additionalTable);
$tableGroupName = $this->getTableGroupName($relatedTables);
Expand All @@ -1489,8 +1521,6 @@ private function getColumnsList(string $mainTable, array $additionalTablesFetch
// Let's remove any duplicate
$allFetchedTables = array_flip(array_flip($allFetchedTables));

$columnsList = [];
$columnDescList = [];
$schema = $this->tdbmSchemaAnalyzer->getSchema();

// Now, let's build the column list
Expand Down
20 changes: 20 additions & 0 deletions tests/Mouf/Database/TDBM/Dao/TestUserDao.php
Expand Up @@ -59,4 +59,24 @@ public function getUsersWrongTableName()
{
return $this->find('contacts.manager_id = 1');
}

/**
* Returns a list of users, sorted by a table on an external column.
*
* @return \Mouf\Database\TDBM\ResultIterator|\Mouf\Database\TDBM\Test\Dao\Bean\UserBean[]|\Mouf\Database\TDBM\Test\Dao\Generated\ResultArray
*/
public function getUsersByCountryName()
{
return $this->find(null, [], 'country.label DESC');
}

/**
* A test to sort by function.
*
* @return \Mouf\Database\TDBM\ResultIterator|\Mouf\Database\TDBM\Test\Dao\Bean\UserBean[]|\Mouf\Database\TDBM\Test\Dao\Generated\ResultArray
*/
public function getUsersByReversedCountryName()
{
return $this->find(null, [], 'REVERSE(country.label) ASC');
}
}
67 changes: 67 additions & 0 deletions tests/Mouf/Database/TDBM/OrderByAnalyzerTest.php
@@ -0,0 +1,67 @@
<?php

namespace Mouf\Database\TDBM;

use Doctrine\Common\Cache\ArrayCache;
use Doctrine\Common\Cache\VoidCache;

class OrderByAnalyzerTest extends \PHPUnit_Framework_TestCase
{
public function testAnalyzeOrderBy()
{
$analyzer = new OrderByAnalyzer(new VoidCache(), '');
$results = $analyzer->analyzeOrderBy('`a`, b desc, rand() DESC, masc, mytable.mycol');

$this->assertCount(5, $results);
$this->assertEquals([
'type' => 'colref',
'table' => null,
'column' => 'a',
'direction' => 'ASC',
], $results[0]);
$this->assertEquals([
'type' => 'colref',
'table' => null,
'column' => 'b',
'direction' => 'DESC',
], $results[1]);
$this->assertEquals([
'type' => 'expr',
'expr' => 'rand()',
'direction' => 'DESC',
], $results[2]);
$this->assertEquals([
'type' => 'colref',
'table' => null,
'column' => 'masc',
'direction' => 'ASC',
], $results[3]);
$this->assertEquals([
'type' => 'colref',
'table' => 'mytable',
'column' => 'mycol',
'direction' => 'ASC',
], $results[4]);
}

public function testExprWithAsc()
{
$analyzer = new OrderByAnalyzer(new VoidCache(), '');
$results = $analyzer->analyzeOrderBy('foodesc + barasc');

$this->assertCount(1, $results);
$this->assertEquals([
'type' => 'expr',
'expr' => 'foodesc + barasc',
'direction' => 'ASC',
], $results[0]);
}

public function testCache()
{
$analyzer = new OrderByAnalyzer(new ArrayCache(), '');
$results = $analyzer->analyzeOrderBy('foo');
$results2 = $analyzer->analyzeOrderBy('foo');
// For code coverage purpose
}
}

0 comments on commit a83f910

Please sign in to comment.