Execute method does not contain return statement in SQLSrvStatement

[ekarajbic-mydays]

Shouldn't there be a return statement in execute method, since documentation states that method returns boolen?

[Ocramius]

Could you link the specific bit of code please?

[ekarajbic-mydays]

In class https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php

method execute on line 249.

The documentation section is: https://www.doctrine-project.org/api/dbal/2.7/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.html#method_execute

[Ocramius]

Ref:

dbal/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php

Line 249 in d475ade

public function execute($params = null)

[morozov]

As part of #3154, I'm trying to change the return type of Statement::execute() from bool to void. You cannot reliably use the return value because the implementations may throw all sorts of exceptions. Besides the documentation mismatch, is there a practical reason for changing the code?

[ekarajbic-mydays]

Nobody is relying only on return, of course try-catch blocks must be there.

But, it was nice to know if it was executed or not with true/false as return.

All other Statements still have it except SQLSrvStatement, so in these terms it should be taken as a BUG.

In #3154 , is it guaranteed then when void was returned, the statement was executed successfully if no exception has been thrown ?

If that is the case, then no other reason beside mismatch in documentation is problem.

[morozov]

But, it was nice to know if it was executed or not with true/false as return.

Currently, the fact that the method doesn't throw an exception is the indicator of success:

dbal/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php

Lines 263 to 265 in d475ade

	if (!sqlsrv_execute($this->stmt)) {
	throw SQLSrvException::fromSqlSrvErrors();
	}

In #3154, is it guaranteed then when void was returned, the statement was executed successfully if no exception has been thrown?

Yes, this is the goal.

[morozov]

I believe this is irrelevant as of #4045.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.