ZF-10598: Zend Select odd behavior

Issue Type:

Bug

Created:

2010-10-25T16:13:30.000+0000

Last Updated:

2011-04-23T07:46:42.000+0000

Status:

Resolved

Fix version(s):

Reporter:

The Lone Coder (loconut)

Assignee:

Ramon Henrique Ornelas (ramon)

Tags:

Zend_Db_Select

Related issues:

ZF-8944

Attachments:

Description

Here is some code that was designed to randomly choose a row from the databse. I discovered that doing $select->__toString() would always show the correct SQL and when put in MySQL directly would work, but when used with fetchRow($select) the end result seemed to ignore the limit clause. If I switched to fetchAll($select) it worked.


    public static function ChooseRandomPoolFile()
    {
        $retval = false;

        $tbl = new TableAdapters_PoolFiles();
        $db = $tbl->getAdapter();
        $select = $db->select()
            ->from('PoolFiles', array('Sum' => new Zend_Db_Expr('COUNT(*)')))
            ->where('FileSize > 0')
            ->order(array('DateAdded ASC', 'BlockID ASC'));
        $count = $db->fetchOne($select);
        unset($select);

        if (is_numeric($count) && ($count > 0)) {
            $offset = mt_rand(0, ($count - 1));
            $select = $tbl->select()
                ->where('FileSize > 0')
                ->order(array('DateAdded ASC', 'BlockID ASC'))
                ->limit(1, $offset);
            $result = $tbl->fetchRow($select);
            if ($result instanceof Zend_Db_Table_Row) {
                $retval = new self($result, false);
            }
        }

        return $retval;
    }

the above would always return the same row despite getting a varying / valid offset. Changing the above to:


    public static function ChooseRandomPoolFile()
    {
        $retval = false;

        $tbl = new TableAdapters_PoolFiles();
        $db = $tbl->getAdapter();
        $select = $db->select()
            ->from('PoolFiles', array('Sum' => new Zend_Db_Expr('COUNT(*)')))
            ->where('FileSize > 0')
            ->order(array('DateAdded ASC', 'BlockID ASC'));
        $count = $db->fetchOne($select);
        unset($select);

        if (is_numeric($count) && ($count > 0)) {
            $offset = mt_rand(0, ($count - 1));
            $select = $tbl->select()
                ->where('FileSize > 0')
                ->order(array('DateAdded ASC', 'BlockID ASC'))
                ->limit(1, $offset);
            $result = $tbl->fetchAll($select);
            if (($result instanceof Zend_Db_Table_Rowset) && ($result->count() == 1)) {
                $retval = new self($result->current(), false);
            }
        }

        return $retval;
    }

would work fine.

Long story short it appears fetchRow ignores the limit clause assuming it can, but forgetting users may want to select 1 but after some offset?

Comments

Posted by The Lone Coder (loconut) on 2010-10-25T17:04:26.000+0000

Sorry on the above I made a typo- I took my final version and re-edited it while I was typing my post to make my original, and typed fetchOne instead of fetchRow. The one that doesn't work should be:

public static function ChooseRandomPoolFile() { $retval = false;

$tbl = new TableAdapters_PoolFiles(); $db = $tbl->getAdapter(); $select = $db->select() ->from('PoolFiles', array('Sum' => new Zend_Db_Expr('COUNT'))) ->where('FileSize > 0') ->order(array('DateAdded ASC', 'BlockID ASC')); $count = $db->fetchOne($select); unset($select);

if (is_numeric($count) && ($count > 0)) { $offset = mt_rand(0, ($count - 1)); $select = $tbl->select() ->where('FileSize > 0') ->order(array('DateAdded ASC', 'BlockID ASC')) ->limit(1, $offset); $result = $tbl->fetchRow($select); if ($result instanceof Zend_Db_Table_Row) { $retval = new self($result, false); } }

return $retval; }

Posted by The Lone Coder (loconut) on 2010-10-25T17:11:06.000+0000

Upon inspection of Zend/Db/Table/Abstract.php around line 1361 it appears that the limit is overridden.

Ideally it should preserve any limit offset that was passed in.

change the limits to something like

$this->limit(1, $select->getPart('limitoffset'));

Posted by The Lone Coder (loconut) on 2010-10-25T17:27:29.000+0000

proposed patch?

Index: Zend/Db/Table/Abstract.php

--- Zend/Db/Table/Abstract.php (revision 1791) +++ Zend/Db/Table/Abstract.php (working copy) @@ -1358,10 +1358,10 @@ $this->_order($select, $order); }

$select->limit(1);

     $select->limit(1, $select->getPart(Zend_Db_Select::LIMIT_OFFSET));

 } else {

$select = $where->limit(1);

     $select = $where->limit(1, $select->getPart(Zend_Db_Select::LIMIT_OFFSET));
 }

 $rows = $this->_fetch($select);

Posted by The Lone Coder (loconut) on 2010-10-25T17:31:23.000+0000

JIRA stinks for posting code... yeesh.

patch placed at: http://webtrotter.com/zf1.10.8.patch

Posted by The Lone Coder (loconut) on 2010-10-25T17:33:46.000+0000

I believe my patch to Zend_Db_Table_Abstract is a better solution though.

Posted by Ralph Schindler (ralph) on 2010-11-19T07:31:37.000+0000

All you need to do is put the ``` tag around your code.

Posted by Ralph Schindler (ralph) on 2010-11-19T07:36:12.000+0000

If you are using MySQL, why not instead do something like this: $select->order(new Zend_Db_Expr('RAND()')); as opposed to finding the count and crafting your own random algorithm?

Posted by The Lone Coder (loconut) on 2010-11-19T08:47:06.000+0000

Well, regardless of a better implementation of the above code, the bug is real. Fact is, if you give a limit with a specific row offset and fetchOne, it obliterates the offset. Simply passing in the offset as in my patch lets one specify an arbitrary offset and still be able to fetchOne.

Posted by The Lone Coder (loconut) on 2010-11-19T08:50:24.000+0000

Have a look at the lines referenced in the patch.

In Zend/Db/Table/Abstract.php:

There are two instances where it has $select->limit(1) instead of $select->limit(1, $select->getPart(Zend_Db_Select::LIMIT_OFFSET))

Which would fix the problem. It seems pretty obvious looking at the source what the problem is and the solution is only a half step away.

Posted by The Lone Coder (loconut) on 2011-02-06T12:16:20.000+0000

I've updated the parent task as this is still not resolved. I'm commenting here to reawaken anyone watching.

Posted by Kim Blomqvist (kblomqvist) on 2011-04-23T07:46:42.000+0000

Patch provided in ZF-11253.