Changed the MERGE queries for Oracle and SQL Server to use question mark parameter markers so they work with emulation disabled or enabled -fixes#17284

carsonbot added the Status: Needs Review label

Apr 10, 2016

javiereguiluz added Bug HttpFoundation labels

Apr 10, 2016

Copy link

Member

xabbuh commentedApr 11, 2016

Imo a proper fix would rather change the queries to not use a named parameter more than once.

Copy link

slootjes commentedApr 18, 2016

I can confirm this fix is working for me.

Copy link

Member

fabpot commentedApr 28, 2016

@Tobion Can you have a look at this one?

Copy link

Member

fabpot commentedJun 16, 2016

@hjkl Have you tested@xabbuh's suggestion instead?

Copy link

Contributor

Tobion commentedJun 16, 2016•
edited
Loading

@hjkl please changegetMergeSql (probably rename it as well togetMergeStatement) to return the PDO statement directly with the params already bound. Since the params and the sql are now dependent on each other, it should be done in one place.
Apart from that it looks correct to me.

Copy link

Contributor

Tobion commentedJun 16, 2016

This PR needs a rebase.

Fixed oci and sqlsrv merge queries when emulation is disabled -fixess…

ebf3a2f

…ymfony#17284

hjkl force-pushed theticket_17284 branch froma7a42b3 toebf3a2fCompare

June 18, 2016 19:15

Copy link

ContributorAuthor

hjkl commentedJun 18, 2016

@fabpot I haven't attempted@xabbuh's suggestion - I don't have the TSQL expertise to refactor the merge query to only use each parameter once.

@Tobion Done, thanks for the advice.

Copy link

Contributor

Tobion commentedJun 18, 2016•
edited
Loading

Imo a proper fix would rather change the queries to not use a named parameter more than once.

This is exactly what this PR does. NAMED params cannot be used more than once but it's not possible to change the MERGE queries to only use each param once as they need to be repeated. It only works for the MySQL for example because mysql supports theVALUES($this->dataCol) syntax to reuse values. Otherwise we would also be forced into using numeric values several times for mysql. For reference, if you disable emulated prepares in MySQL and use the same named param more than once you get aSQLSTATE[HY093]: Invalid parameter number error as I just tried.

👍 good to merge
Status: Reviewed

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jun 18, 2016

Copy link

Member

fabpot commentedJun 19, 2016

Thank you@hjkl.

fabpot merged commitebf3a2f intosymfony:2.7

Jun 19, 2016

fabpot added a commit that referenced this pull request

Jun 19, 2016

bug#18501[HttpFoundation] changed MERGE queries (hjkl)

6a8a113

This PR was merged into the 2.7 branch.Discussion----------[HttpFoundation] changed MERGE queries| Q             | A| ------------- | ---| Branch?       | 2.7| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#17284| License | MIT| Doc PR | N/AChanged the MERGE queries for Oracle and SQL Server to use question mark parameter markers so they work with emulation disabled or enabled -fixes#17284Commits-------ebf3a2f Fixed oci and sqlsrv merge queries when emulation is disabled -fixes#17284

phansys reviewed

Jun 19, 2016

View reviewed changes

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php

		$mergeStmt->bindParam(6,$data, \PDO::PARAM_LOB);
		$mergeStmt->bindParam(7,$maxlifetime, \PDO::PARAM_INT);
		$mergeStmt->bindValue(8,time(), \PDO::PARAM_INT);
		}