Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

invoke execute() twice passing ParameterContainer, second execution uses old parameters #135

Open
GeeH opened this issue Jun 28, 2016 · 2 comments

Comments

@GeeH
Copy link
Contributor

GeeH commented Jun 28, 2016

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7280
User: @lomo74
Created On: 2015-02-28T15:30:00Z
Updated At: 2015-11-06T20:52:18Z
Body

$stmt = $adapter->query("SELECT CAST(:num AS INT) AS num");
//first invocation
$params = new ParameterContainer();
$params->offsetSet('num', 1);
echo $stmt->execute($params)->current()['num'] . PHP_EOL;
//second invocation
$params = new ParameterContainer();
$params->offsetSet('num', 2);
echo $stmt->execute($params)->current()['num'] . PHP_EOL;

output:
1
1

Looking at zend-db/Zend/Db/Adapter/Statement.php line 216 and subsequent:

/** START Standard ParameterContainer Merging Block */
if (!$this->parameterContainer instanceof ParameterContainer) {
if ($parameters instanceof ParameterContainer) {
$this->parameterContainer = $parameters;
$parameters = null;
} else {
$this->parameterContainer = new ParameterContainer();
}
}

    if (is_array($parameters)) {
        $this->parameterContainer->setFromArray($parameters);
    }

    if ($this->parameterContainer->count() > 0) {
        $this->bindParametersFromContainer();
    }
    /** END Standard ParameterContainer Merging Block */

$this->parameterContainer is not overwritten, unless either
(a) it is not a ParameterContainer instance
(b) the $parameters passed to the function is an array

During the second invocation, neither of the above is true, so the new parameters are simply ignored and the old ones are used.

The above should look like this IMHO:

/** START Standard ParameterContainer Merging Block */
if ($parameters instanceof ParameterContainer) {
$this->parameterContainer = $parameters;
$parameters = null;
} else if (!$this->parameterContainer instanceof ParameterContainer) {
$this->parameterContainer = new ParameterContainer();
}

    if (is_array($parameters)) {
        $this->parameterContainer->setFromArray($parameters);
    }

    if ($this->parameterContainer->count() > 0) {
        $this->bindParametersFromContainer();
    }
    /** END Standard ParameterContainer Merging Block */

@mcouillard
Copy link

Ouch, this is still an active bug? I just ran into it today using the latest 2.x version of zend-db coupled with SQL Server, PDO and ODBC - combined they actually care about parameter data types (at least on Linux, not Windows), so ParameterContainer was required.

Quick workaround is this because execute() doesn't properly handle being given a container:
if ($params instanceof ParameterContainer) {
$stmt->setParameterContainer($params);
}
$result = $stmt->execute($params);

I'll try to get a PR up for execute() handling around /** START Standard ParameterContainer Merging Block */...

@michalbundyra
Copy link
Member

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at laminas/laminas-db#113.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants