Skip to content
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

[bugfix] #49 Fix last insert id return type in case of FreeTDS (2.x) #50

Closed
wants to merge 3 commits into from
Closed

[bugfix] #49 Fix last insert id return type in case of FreeTDS (2.x) #50

wants to merge 3 commits into from

Conversation

szhajdu
Copy link
Collaborator

@szhajdu szhajdu commented Mar 16, 2023

The sqlsrv extension works fine, but in case of FreeTDS, lastInsertid returns with false when no auto increment on table.

Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szhajdu szhajdu changed the title [bugfix] #49 Fix last insert id return type in case of FreeTDS [bugfix] #49 Fix last insert id return type in case of FreeTDS (2.x) Mar 17, 2023
@Naktibalda
Copy link
Member

Thanks for the effort, now please merge the changes from 2.x branch and fix the tests.

#48 added new binary field to users table, so you have to add it to mssql.sql.

Also it seems that MSSQL driver can't handle binary parameters, can you fix that?

https://github.com/Codeception/module-db/actions/runs/4454329549/jobs/7823434721

[PDOException] SQLSTATE[IMSSP]: An error occurred translating string for input param 1 to UCS-2: Error code 0xB

@szhajdu
Copy link
Collaborator Author

szhajdu commented Mar 18, 2023

I have reviewed the issue and it appears that the solution may be more complex.

For the sqlsrv driver, we will need to use $pdoStatement->bindParam($i, $param, PDO::PARAM_LOB, 0, PDO::SQLSRV_ENCODING_BINARY); in order to properly handle binary parameters. However, this will require some modifications to the Codeception\Lib\Driver\Db::executeQuery method in order to detect whether sqlsrv is being used. Would you happen to have any suggestions on how to approach this? Would it be appropriate to override the method?

Furthermore, I have discovered a bug related to dblib: php/php-src#10312. So related tests cannot be fixed until it is fixed in the driver itself.

I would appreciate your input on how to proceed. Thank you.

@Naktibalda
Copy link
Member

I think that overriding executeQuery in SqlSrv driver is fine.

If tests can't be fixed, they can be skipped.

if ($this instanceof SqlSrv) {
   $this->markTestSkipped('Not supported by SqlSrv');
}

Failing binary assertions must be moved to separate methods.

@szhajdu szhajdu closed this by deleting the head repository May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants