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

SET options are not effective over multiple queries in the same connection #385

Closed
mathieuk opened this Issue May 6, 2017 · 6 comments

Comments

4 participants
@mathieuk

mathieuk commented May 6, 2017

When you issue SET queries like SET IDENTITY_INSERT table ON OR SET QUOTED_IDENTIFIER OFF I expect those settings to be of effect for the entire duration of that specific connection but they aren't.

Details

  • Driver Version: 4.1.7-preview (manually compiled off master)
  • OS Version and Distro: Linux/CentOS 7
  • Can you connect to SQL Server via sqlcmd: yes
  • Environment details: PHP 7.1 64bit NTS with pdo_sqlsrv

Steps to Reproduce:

  1. Allow values for the identity column on mytable to be inserted: SET IDENTITY_INSERT mytable ON
  2. Issue query setting a value for an identity column: INSERT INTO mytable (id) VALUES (1)

Expected result:
mytable now has a row with IDENTITY column id with value 1.

Actual result:
Error is thrown that inserting into IDENTITY column is only allowed with IDENTITY_INSERT set to ON.

Steps to Reproduce II:

  1. Disable quoted identifiers SET QUOTED_IDENTIFIERS OFF
  2. Issue query with a quoted identifier: SELECT * FROM "mytable"

Expected result:
Database error reporting a syntax error due to a quoted identifier while we've disabled it on the connection.

Actual result:
No error.

Repro Script

<?php
/* 
CREATE TABLE mytable (
	id int IDENTITY(1,1)
);
*/ 

// Should work, but errors out: 
$connection = new PDO('sqlsrv:DSN here', 'user', 'password'); 
$connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$connection->query("SET IDENTITY_INSERT mytable ON");
$connection->query("INSERT INTO mytable(id) VALUES(1)");

// PHP Fatal error:  Uncaught PDOException: SQLSTATE[23000]: [Microsoft][ODBC Driver 13 for SQL Server][SQL Server]Cannot insert explicit value for identity column in table 'mytable' when IDENTITY_INSERT is set to OFF.

// This actually does work: 
$connection->query("SET IDENTITY_INSERT mytable ON; INSERT INTO mytable(id) VALUES(1)");

?>

Repro Script 2

<?php

$connection = new PDO('sqlsrv:DSN here', 'user', 'password'); 
$connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

// Should throw exception, but works:
$connection->query('SET QUOTED_IDENTIFIER OFF');
$connection->query('SELECT * FROM "mytable"');

// Actually throws expected exception: 
$connection->query('SET QUOTED_IDENTIFIER OFF; SELECT * FROM "mytable"');

// PHP Fatal error:  Uncaught PDOException: SQLSTATE[42000]: [Microsoft][ODBC Driver 13 for SQL Server][SQL Server]Incorrect syntax near 'mytable'.
?>
@ulvii

This comment has been minimized.

Show comment
Hide comment
@ulvii

ulvii May 9, 2017

Member

Hi @mathieuk ,

Thank you for the repro scripts. I will investigate and get back to you soon.

Member

ulvii commented May 9, 2017

Hi @mathieuk ,

Thank you for the repro scripts. I will investigate and get back to you soon.

@ulvii ulvii self-assigned this May 10, 2017

@ulvii

This comment has been minimized.

Show comment
Hide comment
@ulvii

ulvii May 11, 2017

Member

Hi @mathieuk ,

After some investigation, I determined that, this is not a problem in the driver.

When you issue SET queries like SET IDENTITY_INSERT table ON OR SET QUOTED_IDENTIFIER OFF I expect those settings to be of effect for the entire duration of that specific connection but they aren't.

This is not always true. SET IDENTITY_INSERT is session specific and there can be multiple sessions on a single connection.

Note that, you are preparing and executing( default behavior ) the statement by using $connection->query. https://docs.microsoft.com/en-us/sql/connect/php/pdo-query

Prepared statements result in RPCs ( Remote Procedure Calls ) in SQL Server, which are run in different processes. If you execute the following lines directly on SQL Server, you will get the same exception.

declare @p1 int;
exec sp_prepexec @p1 output,NULL,N'SET IDENTITY_INSERT mytable ON';
exec sp_prepexec @p1 output,NULL,N'INSERT INTO mytable(id) VALUES(1)'

You can read more about RPCs here: https://technet.microsoft.com/en-us/library/cc738291(v=ws.10).aspx

As you mentioned above, you can include SET IDENTITY_INSERT ON/OFF as part of the prepared statement and your script will work fine.

You can also use PDO::SQLSRV_ATTR_DIRECT_QUERY attribute to specify direct statement execution instead of the default, which is prepared statement execution. So you would add the following line to your script:
$connection->setAttribute(constant('PDO::SQLSRV_ATTR_DIRECT_QUERY'), true);

Read more here: https://docs.microsoft.com/en-us/sql/connect/php/direct-statement-execution-prepared-statement-execution-pdo-sqlsrv-driver

I will keep the issue open for a while more, in case if you need any further clarifications.

Member

ulvii commented May 11, 2017

Hi @mathieuk ,

After some investigation, I determined that, this is not a problem in the driver.

When you issue SET queries like SET IDENTITY_INSERT table ON OR SET QUOTED_IDENTIFIER OFF I expect those settings to be of effect for the entire duration of that specific connection but they aren't.

This is not always true. SET IDENTITY_INSERT is session specific and there can be multiple sessions on a single connection.

Note that, you are preparing and executing( default behavior ) the statement by using $connection->query. https://docs.microsoft.com/en-us/sql/connect/php/pdo-query

Prepared statements result in RPCs ( Remote Procedure Calls ) in SQL Server, which are run in different processes. If you execute the following lines directly on SQL Server, you will get the same exception.

declare @p1 int;
exec sp_prepexec @p1 output,NULL,N'SET IDENTITY_INSERT mytable ON';
exec sp_prepexec @p1 output,NULL,N'INSERT INTO mytable(id) VALUES(1)'

You can read more about RPCs here: https://technet.microsoft.com/en-us/library/cc738291(v=ws.10).aspx

As you mentioned above, you can include SET IDENTITY_INSERT ON/OFF as part of the prepared statement and your script will work fine.

You can also use PDO::SQLSRV_ATTR_DIRECT_QUERY attribute to specify direct statement execution instead of the default, which is prepared statement execution. So you would add the following line to your script:
$connection->setAttribute(constant('PDO::SQLSRV_ATTR_DIRECT_QUERY'), true);

Read more here: https://docs.microsoft.com/en-us/sql/connect/php/direct-statement-execution-prepared-statement-execution-pdo-sqlsrv-driver

I will keep the issue open for a while more, in case if you need any further clarifications.

@mathieuk

This comment has been minimized.

Show comment
Hide comment
@mathieuk

mathieuk May 11, 2017

@ulvii that's a thorough investigation, thanks for that!

You mention that, by default, all queries are executed as prepared statements and that each prepared statement is ran in its own little unique context. I also understand these contexts will have their own 'settings' active.

I have mostly worked with MySQL so, sorry if this is an obvious question to you: as PHP applications typically work in a Share Nothing configuration; does SQL Server save execution plans/compilation results for prepared statements cross connection? As in, if I prepare a statement in Connection#1 would a new Connection #2 reap the benefit of that extra work it performed? If not, it seems wasteful to default to prepared statements.

Regardless, this default mode where settings are not shared across all queries in that connection is quite confusing. Even if you were to set the SQLSRV_ATTR_DIRECT_QUERY attribute it would be weird to suddenly lose your settings when you'd switch to using a prepared statement somewhere in your code.

It also complicates certain use-cases. See for instance this issue regarding running seeders on MSSQL in Laravel. Projects with ORMs (like Eloquent in Laravel) typically do not, and probably should not, provide a way of modifying queries sent to the actual database, so it would be hard to get that IDENTITY_INSERT option activated properly.

Perhaps interestingly, setting IDENTITY_INSERT and performing a query in 2 separate calls does work as expected (for me) when I use DBLIB as the method of connection to SQL Server.

So I guess the question is: is (and/or: should) this really the intended behaviour?
Again, thanks for investigating!

mathieuk commented May 11, 2017

@ulvii that's a thorough investigation, thanks for that!

You mention that, by default, all queries are executed as prepared statements and that each prepared statement is ran in its own little unique context. I also understand these contexts will have their own 'settings' active.

I have mostly worked with MySQL so, sorry if this is an obvious question to you: as PHP applications typically work in a Share Nothing configuration; does SQL Server save execution plans/compilation results for prepared statements cross connection? As in, if I prepare a statement in Connection#1 would a new Connection #2 reap the benefit of that extra work it performed? If not, it seems wasteful to default to prepared statements.

Regardless, this default mode where settings are not shared across all queries in that connection is quite confusing. Even if you were to set the SQLSRV_ATTR_DIRECT_QUERY attribute it would be weird to suddenly lose your settings when you'd switch to using a prepared statement somewhere in your code.

It also complicates certain use-cases. See for instance this issue regarding running seeders on MSSQL in Laravel. Projects with ORMs (like Eloquent in Laravel) typically do not, and probably should not, provide a way of modifying queries sent to the actual database, so it would be hard to get that IDENTITY_INSERT option activated properly.

Perhaps interestingly, setting IDENTITY_INSERT and performing a query in 2 separate calls does work as expected (for me) when I use DBLIB as the method of connection to SQL Server.

So I guess the question is: is (and/or: should) this really the intended behaviour?
Again, thanks for investigating!

@ulvii

This comment has been minimized.

Show comment
Hide comment
@ulvii

ulvii May 11, 2017

Member

Hi @mathieuk ,

You mention that, by default, all queries are executed as prepared statements and that each prepared statement is ran in its own little unique context.

This applies to query() API, not all queries. In fact, you can use exec() API to directly execute queries without having to prepare them and your scripts will work fine.
https://docs.microsoft.com/en-us/sql/connect/php/pdo-exec

$connection->exec("SET IDENTITY_INSERT mytable ON");

So I guess the question is: is (and/or: should) this really the intended behaviour?

PDO provides a data-access abstraction layer, which means that, regardless of which database you're using, you use the same functions to issue queries and fetch data. A PHP extension that implements PDO interface does not have a control over the interface functions, it provides database specific functions.
http://php.net/manual/en/book.pdo.php

So in case of query() API, PDO layer calls PDO_SQLSRV's preparer and executer methods.

Member

ulvii commented May 11, 2017

Hi @mathieuk ,

You mention that, by default, all queries are executed as prepared statements and that each prepared statement is ran in its own little unique context.

This applies to query() API, not all queries. In fact, you can use exec() API to directly execute queries without having to prepare them and your scripts will work fine.
https://docs.microsoft.com/en-us/sql/connect/php/pdo-exec

$connection->exec("SET IDENTITY_INSERT mytable ON");

So I guess the question is: is (and/or: should) this really the intended behaviour?

PDO provides a data-access abstraction layer, which means that, regardless of which database you're using, you use the same functions to issue queries and fetch data. A PHP extension that implements PDO interface does not have a control over the interface functions, it provides database specific functions.
http://php.net/manual/en/book.pdo.php

So in case of query() API, PDO layer calls PDO_SQLSRV's preparer and executer methods.

@mathieuk

This comment has been minimized.

Show comment
Hide comment
@mathieuk

mathieuk May 13, 2017

Ok so, the documentation you referenced earlier mentions this and I missed that:

If a query requires the context that was set in a previous query, you should execute your queries with PDO::SQLSRV_ATTR_DIRECT_QUERY set to True. For example, if you use temporary tables in your queries, PDO::SQLSRV_ATTR_DIRECT_QUERY must be set to True.

So, it's known and intended behaviour. Sorry for the noise.

Based on your suggestions I've enumerated what effect different methods of querying have. Maybe it'll help someone reading this later on. In all cases I'm executing SET IDENTITY_INSERT mytable ON followed by INSERT INTO mytable (id) VALUES (1):

  1. Using query() for both queries: fails with exception (need identity_insert on)
  2. Using query() for the SET query and prepare()/execute for the INSERT query: fails with exception
  3. Activating PDO::SQLSRV_ATTR_DIRECT_QUERY and using query() for both queries: works
  4. Activating PDO::SQLSRV_ATTR_DIRECT_QUERY, using query() for the SET query and prepare()/execute for the INSERT query: works
  5. Using exec() for both the SET and the INSERT query: works
  6. Using exec() for the SET query and prepare()/execute for the INSERT query: works

@ulvii in case 4, would PDO now use pseudo prepared-statements?
Issue can be closed, thanks for investigating.

mathieuk commented May 13, 2017

Ok so, the documentation you referenced earlier mentions this and I missed that:

If a query requires the context that was set in a previous query, you should execute your queries with PDO::SQLSRV_ATTR_DIRECT_QUERY set to True. For example, if you use temporary tables in your queries, PDO::SQLSRV_ATTR_DIRECT_QUERY must be set to True.

So, it's known and intended behaviour. Sorry for the noise.

Based on your suggestions I've enumerated what effect different methods of querying have. Maybe it'll help someone reading this later on. In all cases I'm executing SET IDENTITY_INSERT mytable ON followed by INSERT INTO mytable (id) VALUES (1):

  1. Using query() for both queries: fails with exception (need identity_insert on)
  2. Using query() for the SET query and prepare()/execute for the INSERT query: fails with exception
  3. Activating PDO::SQLSRV_ATTR_DIRECT_QUERY and using query() for both queries: works
  4. Activating PDO::SQLSRV_ATTR_DIRECT_QUERY, using query() for the SET query and prepare()/execute for the INSERT query: works
  5. Using exec() for both the SET and the INSERT query: works
  6. Using exec() for the SET query and prepare()/execute for the INSERT query: works

@ulvii in case 4, would PDO now use pseudo prepared-statements?
Issue can be closed, thanks for investigating.

@mathieuk mathieuk closed this May 13, 2017

@hrach

This comment has been minimized.

Show comment
Hide comment
@hrach

hrach May 19, 2017

Is there any solution (aka PDO::SQLSRV_ATTR_DIRECT_QUERY) for the non-PDO extension?
EDIT: I realized that replacing sqlsrv_preparec & sqlsrv_exec with sqlsrv_query make this work!

hrach commented May 19, 2017

Is there any solution (aka PDO::SQLSRV_ATTR_DIRECT_QUERY) for the non-PDO extension?
EDIT: I realized that replacing sqlsrv_preparec & sqlsrv_exec with sqlsrv_query make this work!

@Hadis-Fard Hadis-Fard added this to Need Review in msphpsql May 25, 2017

@Hadis-Fard Hadis-Fard moved this from Need Review to Completed in msphpsql May 25, 2017

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