Zend\Db\Adapter\Sqlsrv\Sqlsrv never calls Statement\initialize() (fix within) #2678

Closed
BenjaminNolan opened this Issue Oct 4, 2012 · 6 comments

Comments

Projects
None yet
5 participants
Contributor

BenjaminNolan commented Oct 4, 2012

Hi,

Zend\Db\Adapter\Sqlsrv\Sqlsrv currently contains this in its createStatement function, which differs somewhat in structure from the other drivers and never calls $statement->initialize if $sqlOrResource is null.

    /**
     * @param string|resource $sqlOrResource
     * @return Statement
     */
    public function createStatement($sqlOrResource = null)
    {
        $statement = clone $this->statementPrototype;
        if (is_string($sqlOrResource)) {
            $statement->setSql($sqlOrResource);
            if (!$this->connection->isConnected()) {
                $this->connection->connect();
            }
            $statement->initialize($this->connection->getResource());
        } elseif (is_resource($sqlOrResource)) {
            $statement->setResource($sqlOrResource);
        }
        return $statement;
    }

As a result, you get the following error when you try to actually access anything through it:

Warning: sqlsrv_prepare() expects parameter 1 to be resource, null given

Taking the if-statement from the createStatement method in Zend\Db\Adapter\Mysqli\Mysqli and modifying it to check for instanceof Zend\Db\Adapter\Sqlsrv\Statement instead fixes the problem, and the adapter works as expected.

    /**
     * @param string|resource $sqlOrResource
     * @return Statement
     */
    public function createStatement($sqlOrResource = null)
    {
        $statement = clone $this->statementPrototype;
        if ($sqlOrResource instanceof Statement) {
            $statement->setResource($sqlOrResource);
        } else {
            if (is_string($sqlOrResource)) {
                $statement->setSql($sqlOrResource);
            }
            if (!$this->connection->isConnected()) {
                $this->connection->connect();
            }
            $statement->initialize($this->connection->getResource());
        }
        return $statement;
    }

I would attach a patch file to this, but I can't actually figure out how to do it. :/ D'oh.

@ghost ghost assigned ralphschindler Oct 4, 2012

Owner

weierophinney commented Oct 4, 2012

@Syniq By switching to github issues, we're encouraging developers to create a pull request with their patch instead of simply attaching one. You can read how to do this in the CONTRIBUTING.md inside the repository.

Contributor

BenjaminNolan commented Oct 5, 2012

Aha! That would make sense. I'll sort that out when I get back to my desk, if someone hasn't done it already.

Chup commented Oct 11, 2012

Thank you! Was just what i needed.

Member

ralphschindler commented Oct 12, 2012

After looking at the code, I can see that there is an issue in the initialize() method of the Statement. I can fix that. The only issue I see in the main driver class is that it should, instead of calling setResource() it too shoudl call initialize().

In your use case, you should not be providing an instance of Sqlsrv\Statement to Sqlsrv\Sqlsrv::createStatement(). createStatement() is intended to be a factory for Sqlsrv\Statement objects.

Contributor

dkidd commented Nov 2, 2012

I think ralphschindler's fix may have led to issue #2885.

BenjaminNolan added a commit to BenjaminNolan/zf2 that referenced this issue Dec 4, 2012

Contributor

BenjaminNolan commented Dec 4, 2012

I've just been talking to Ralph about this bug in IRC as it's still there (and I've finally figured out how to do a pull request). Fix has been dispatched forthwith. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment