Issues

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

Issue Type: Improvement Created: 2007-09-30T07:54:26.000+0000 Last Updated: 2009-06-29T09:18:07.000+0000 Status: Resolved Fix version(s): - 1.8.0 (30/Apr/09)

Reporter: julien PAULI (doctorrock83) Assignee: Jurrien Stutterheim (norm2782) Tags: - Zend_Db_Select

Related issues: - ZF-3220

Attachments: - zf-2017.patch

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 :

<pre class="highlight">
    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 :

<pre class="highlight">
    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 :

<pre class="highlight">
$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 :

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

Comments

Posted by Thomas Weidner (thomas) on 2007-10-01T12:24:50.000+0000

Assigned to Bill

Posted by Simon Mundy (peptolab) on 2008-03-13T22:43:45.000+0000

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.

Posted by Wil Sinclair (wil) on 2008-03-25T22:06:44.000+0000

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

Posted by Roger Hunwicks (rhunwicks) on 2008-10-22T08:28:53.000+0000

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:

<pre class="highlight">
        $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:

<pre class="highlight">    /**
     * 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:

<pre class="highlight">    /**
     * 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:

<pre class="highlight">    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:

<pre class="highlight">    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.

Posted by Wil Sinclair (wil) on 2009-01-06T14:30:25.000+0000

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

Posted by Ralph Schindler (ralph) on 2009-01-10T09:49:46.000+0000

Will evaluate within 2 weeks

Posted by Jurrien Stutterheim (norm2782) on 2009-03-27T18:16:21.000+0000

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?

Posted by Jurrien Stutterheim (norm2782) on 2009-03-28T16:55:15.000+0000

Added an updated patch file, complete with unit tests.

Posted by Jurrien Stutterheim (norm2782) on 2009-03-29T01:56:07.000+0000

Resolved in r. 14528

Posted by Sergey Kolesov (hedgehog) on 2009-06-29T09:18:05.000+0000

Still does not work properly as of 1.8.4.

Db\Select.php

line 674 should be:

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

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts