Issues

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

Fixed component assignment

I have one question.

If You would use 3 or more orders, your plan could runs good?

A fix has been provided in r17706, please test.

Fixed in release branch 1.9