ZF-4099: Zend_Db_Adapter_Pdo_Mssql - Order By in Limit Function
Description
In the Zend_Db_Adapter_Pdo_Mssql->limit() method there is a critical bug Order By Clause of the outer_tbl. Consider the following code:
$select = $db->select()
->from( 'TBL', array( 'COL1', 'COL2' ) )
->order( array( 'COL1 ASC', 'COL2 DESC' ) );
Will generate following query:
SELECT "TBL"."COL1", "TBL"."COL2"
FROM "TBL"
ORDER BY "COL1" ASC, "COL2" DESC
Now when limitPage is applied:
$select = $db->select()
->from( 'TBL', array( 'COL1', 'COL2' ) )
->order( array( 'COL1 ASC', 'COL2 DESC' ) )
->limitPage( 2, 50 );
will generate the following query:
SELECT *
FROM (
SELECT TOP 50 *
FROM (
SELECT TOP 100 "TBL"."COL1",
"TBL"."COL2"
FROM "TBL"
ORDER BY "COL1" ASC,
"COL2" DESC
) AS inner_tbl
ORDER BY "COL1" ,
"COL2" ASC
) AS outer_tbl
ORDER BY "COL1" ,
"COL2" desc
Now here is the problem. As you can see in the innermost query the order by section is correct, ASC on COL1 and DESC on COL2 the same should be applied on all the outer tables but in reverse order. By doing this, then only we will get the correct result for the next page.
So, the correct query should be:
SELECT *
FROM (
SELECT TOP 50 *
FROM (
SELECT TOP 100 "TBL"."COL1",
"TBL"."COL2"
FROM "TBL"
ORDER BY "COL1" ASC,
"COL2" DESC
) AS inner_tbl
ORDER BY "COL1" DESC,
"COL2" ASC
) AS outer_tbl
ORDER BY "COL1" asc,
"COL2" desc
To fix this issue, I have reviewed the limit function of the Zend_Db_Adapter_Pdo_Mssql class which is:
/**
* Adds an adapter-specific LIMIT clause to the SELECT statement.
*
* @link http://lists.bestpractical.com/pipermail/rt-devel/…
*
* @param string $sql
* @param integer $count
* @param integer $offset OPTIONAL
* @throws Zend_Db_Adapter_Exception
* @return string
*/
public function limit($sql, $count, $offset = 0)
{
$count = intval($count);
if ($count <= 0) {
/** @see Zend_Db_Adapter_Exception */
require_once 'Zend/Db/Adapter/Exception.php';
throw new Zend_Db_Adapter_Exception("LIMIT argument count=$count is not valid");
}
$offset = intval($offset);
if ($offset < 0) {
/** @see Zend_Db_Adapter_Exception */
require_once 'Zend/Db/Adapter/Exception.php';
throw new Zend_Db_Adapter_Exception("LIMIT argument offset=$offset is not valid");
}
$orderby = stristr($sql, 'ORDER BY');
if ($orderby !== false) {
$sort = (stripos($orderby, ' desc') !== false) ? 'desc' : 'asc';
$order = str_ireplace('ORDER BY', '', $orderby);
$orderInv = str_ireplace( 'asc', 'desc', $orderOrig );
$order = trim(preg_replace('/\bASC\b|\bDESC\b/i', '', $order));
}
$sql = preg_replace('/^SELECT\s/i', 'SELECT TOP ' . ($count+$offset) . ' ', $sql);
$sql = 'SELECT * FROM (SELECT TOP ' . $count . ' * FROM (' . $sql . ') AS inner_tbl';
if ($orderby !== false) {
$sql .= ' ORDER BY ' . $order . ' ';
$sql .= (stripos($sort, 'asc') !== false) ? 'DESC' : 'ASC';
}
$sql .= ') AS outer_tbl';
if ($orderby !== false) {
$sql .= ' ORDER BY ' . $order . ' ' . $sort;
}
return $sql;
}
// ------------------------------------------------------------------------------
And a fixed version of this function from my side:
/**
* Adds an adapter-specific LIMIT clause to the SELECT statement.
*
* @link http://lists.bestpractical.com/pipermail/rt-devel/…
*
* @param string $sql
* @param integer $count
* @param integer $offset OPTIONAL
* @throws Zend_Db_Adapter_Exception
* @return string
*/
public function limit($sql, $count, $offset = 0)
{
$count = intval($count);
if ($count <= 0) {
/** @see Zend_Db_Adapter_Exception */
require_once 'Zend/Db/Adapter/Exception.php';
throw new Zend_Db_Adapter_Exception("LIMIT argument count=$count is not valid");
}
$offset = intval($offset);
if ($offset < 0) {
/** @see Zend_Db_Adapter_Exception */
require_once 'Zend/Db/Adapter/Exception.php';
throw new Zend_Db_Adapter_Exception("LIMIT argument offset=$offset is not valid");
}
$orderOrig = '';
$orderInv = '';
$orderby = stristr($sql, 'ORDER BY');
if ($orderby !== false) {
$sort = (stripos($orderby, ' desc') !== false) ? 'desc' : 'asc';
$order = str_ireplace('ORDER BY', '', $orderby);
//$orderInv = str_ireplace( 'asc', 'desc', $orderOrig );
//$order = trim(preg_replace('/\bASC\b|\bDESC\b/i', '', $order));
$orderOrig = $order; // order by in original form
$orderInv = $orderOrig; // order by in inverse form (sort orders of each columns will be reversed)
$orderInv = str_ireplace( 'DESC', 'DESC1', $orderInv ); // change DESC to DESC1 so later we can change this to ASC
$orderInv = str_ireplace( 'ASC', 'DESC', $orderInv ); // change ASC to DESC
$orderInv = str_ireplace( 'DESC1', 'ASC', $orderInv ); // change DESC1 to ASC now
}
$sql = preg_replace('/^SELECT\s/i', 'SELECT TOP ' . ($count+$offset) . ' ', $sql);
$sql = 'SELECT * FROM (SELECT TOP ' . $count . ' * FROM (' . $sql . ') AS inner_tbl';
if ($orderby !== false) {
//$sql .= ' ORDER BY ' . $order . ' ';
//$sql .= (stripos($sort, 'asc') !== false) ? 'DESC' : 'ASC';
$sql .= ' ORDER BY ' . $orderInv . ' ';
}
$sql .= ') AS outer_tbl';
if ($orderby !== false) {
//$sql .= ' ORDER BY ' . $order . ' ' . $sort;
$sql .= ' ORDER BY ' . $orderOrig;
}
return $sql;
}
Hope this is resolved in upcoming version of Zend Framework
Thanks,
Hashaam
Comments
Posted by Thomas Weidner (thomas) on 2008-10-05T14:24:07.000+0000
Fixed component assignment
Posted by old of Satoru Yoshida (yoshida@zend.co.jp) on 2009-05-30T19:45:03.000+0000
I have one question.
If You would use 3 or more orders, your plan could runs good?
Posted by Ralph Schindler (ralph) on 2009-08-21T06:36:23.000+0000
A fix has been provided in r17706, please test.
Posted by Ralph Schindler (ralph) on 2009-08-24T10:50:00.000+0000
Fixed in release branch 1.9