Issues

ZF-2017: Illogic syntax in method Zend_Db_Select->query() while using binding parameters

Description

Zend_Db_Select->query() should behave exactly like Zend_Db_Adapter->select(). You may not actually pass binding parameters to Zend_Db_Select->query(), while you may throught Zend_Db_Adapter->query(), whitch is a strange behavior.

Actual Zend_Db_Select code :


    public function query($fetchMode = null)
    {
        $stmt = $this->_adapter->query($this);
        if ($fetchMode == null) {
            $fetchMode = $this->_adapter->getFetchMode();
        }
        $stmt->setFetchMode($fetchMode);
        return $stmt;
    }

Improvement to consider binding parameters :


    public function query($fetchMode = null, $bind = array() )
    {
        $stmt = $this->_adapter->query($this, $bind);
        if ($fetchMode == null) {
            $fetchMode = $this->_adapter->getFetchMode();
        }
        $stmt->setFetchMode($fetchMode);
        return $stmt;
    }

Like this, both Select and Adapter's query() methods have the same behavior. Use case :


$select = $db->select();
$select->from('test', array('id','champ'))
      ->where('id > :id');
$statement = $select->query(Zend_Db::FETCH_OBJ,array(':id'=> 2));
/* behaves exactly as this :
*$db->setFetchMode(Zend_Db::FETCH_OBJ);
*$statement = $db->query($select,array(':id'=> 2));
*/

We could as well improve Zend_Db_Select->query() by specifing a fetch mode directly :


    public function query($fetchMode = null, $bind = array())
    {
        if ($fetchMode != null) {
           $this->_adapter->setFetchMode($fetchMode);
        }        
        return $this->_adapter->query($this,$bind);
    }

Comments

Assigned to Bill

I like the idea, although the latter 'fetchMode' example may need to be changed slightly. With the example shown, it will change the state of the fetchmode for the entire adapter, whereas currently the fetchmode is only applied on a per-statement basis. But yes, the $bind param is a great idea.

Resetting 'fix version priority' and 'fix version' to be re-evaluated for next release.

I think that this is a great idea, as it is much better to use binding at the database level, rather than quoting at the Zend_Db_Adapter level.

If we do allow Zend_Db_Select->query() to include a bind, we should also include a bind() method to allow the bind to be set up in advance.

Using Zend_Paginator is a use case:


        $table = new Bugs();
        $select = $table->select()->where("bug_status = :bug_status")
                                  ->bind(array(':bug_status' => $this->_getParam('status')));
        $paginator = Zend_Paginator::factory($select, 'DbTableSelect')
        $paginator->setCurrentPageNumber($this->_getParam('page')); 
        $this->view->paginator = $paginator;

Without the bind() function there is no easy way to set up the bind variables before passing the Zend_Db_Table_Select to the Zend_Paginator, and consequently you can't use all that Zend_Paginator, View_Renderer, Front_Controller, etc. magic to just fetch the correct rows.

I think this was planned at one time: see [http://framework.zend.com/issues/browse/ZF-105]

My additions to Zend_Db_Select to support this:

    /**
     * Variables to bind when executing the query
     *
     * @var array
     */
    protected $_bind = array();

    /**
     * Set the bind variables for future use
     *
     * @param array    $bind
     * @return Zend_Db_Select This Zend_Db_Select object.
     */
    public function bind($bind)
    {
        $this->_bind = $bind;
        return $this;
    }

    /**
     * Get the bind variables currently stored for future use
     *
     * @return array    $bind
     */
    public function getBind()
    {        
        return $this->_bind;
    }

And then an updated query, slightly different to the one proposed in the Description:

    /**
     * Execute the Select statement and return the results
     * 
     * @param integer $fetchMode OPTIONAL
     * @param array   $bind      OPTIONAL
     * @return PDO_Statement|Zend_Db_Statement
     */
    public function query($fetchMode = null, $bind = array())
    {
        if (!empty($bind)) {
            $this->bind($bind);
        }
        $stmt = $this->_adapter->query($this, $this->getBind());
        if ($fetchMode == null) {
            $fetchMode = $this->_adapter->getFetchMode();
        }
        $stmt->setFetchMode($fetchMode);
        return $stmt;
    }

The getBind() function is required because I think a change to Zend_Db_Table_Abstract is also required. The _fetch() method selects rows using a Zend_Db_Table_Select (which inherits from Zend_Db_Select). This method needs to be aware of the stored bind variables and use them. Hence, the original code:

    protected function _fetch(Zend_Db_Table_Select $select)
    {
        $stmt = $this->_db->query($select);
        $data = $stmt->fetchAll(Zend_Db::FETCH_ASSOC);
        return $data;
    }

would be replaced by:

    protected function _fetch(Zend_Db_Table_Select $select)
    {
        $stmt = $this->_db->query($select, $select->getBind());
        $data = $stmt->fetchAll(Zend_Db::FETCH_ASSOC);
        return $data;
    }

In particular this is required because Zend_Paginator_Adapter_DbSelect executes $this->_select->getTable()->fetchAll($this->_select) to get the rows, and therefore fetchAll (and hence _fetch) need to know how to access the bind variables it they have been set.

This issue has gone unaddressed for too long. I'm re-assigning to Ralph for re-evaluation and categorization.

Will evaluate within 2 weeks

Attached patch with most suggestions in this thread applied. Will see if I can write some unit tests later on. Ralph, do you have the time to look at this?

Added an updated patch file, complete with unit tests.

Resolved in r. 14528

Still does not work properly as of 1.8.4.

Db\Select.php

line 674 should be:

$stmt = $this->_adapter->query($this, $this->getBind());