8000 Fix drivers' `exec()` method to not execute queries via prepared statements by deeky666 · Pull Request #2640 · doctrine/dbal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix drivers' exec() method to not execute queries via prepared statements #2640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

deeky666
Copy link
Member
@deeky666 deeky666 commented Feb 6, 2017

Even though the driver connection interface is not quite clear about this, the exec() method is obviously intended for DML/DDL statements to be directly executed without preparing the statement beforehand. It is not possible to bind parameters here and it will only ever return affected rows.
Now while nevertheless using prepared statements internally might not seem like such a big deal, there are some cons about this approach:

  • SQL Server requires some DDL statements like creating/dropping temporary tables to be executed directly without prepared statements because of database internals (See: [Linux] Prepared statements changing session scope? microsoft/msphpsql#276)
  • Using prepared statements if not necessary creates unwanted overhead
  • The behaviour is not consistent with the PDO implementations, which do not use prepared statement either if the driver in question is capable of doing so

I also updated the testsuite where invalid executeQuery() calls for DML/DDL statement where done.

This bugfix is required for the SQL Server testsuite to succeed. It makes 16 errors succeed on this branch with sqlsrv.

I did not add any additonal tests because it is not possible to mock whether prepared statements are used or not and also all of the various test cases are already covered by functional tests.

@Ocramius
Copy link
Member
Ocramius commented Feb 7, 2017

After going back and forth a few times, especially considering the test case changes, I'm going to merge this only into 2.6

@Ocramius Ocramius merged commit f92d3f4 into doctrine:master Feb 7, 2017
@Ocramius
Copy link
Member
Ocramius commented Feb 7, 2017

@deeky666 👍

@Ocramius Ocramius changed the title Fix drivers' exec() method to not execute via prepared statements Fix drivers' exec() method to not execute queries via prepared statements Jul 22, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0