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

An error occurs in set_attribute when using PDO SQLSRV #12038

Closed
SakiTakamachi opened this issue Aug 23, 2023 · 4 comments
Closed

An error occurs in set_attribute when using PDO SQLSRV #12038

SakiTakamachi opened this issue Aug 23, 2023 · 4 comments

Comments

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Aug 23, 2023

Description

The following code:

<?php

// my env
$pdo = new PDO('sqlsrv:server=mssql-server;database=test;', 'test_user', 'p@ssw0rd');
$pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);

Resulted in this output:

Fatal error: Uncaught PDOException: SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object. in /var/www/html/test.php:5
Stack trace:
#0 /var/www/html/test.php(5): PDO->setAttribute(17, false)
#1 {main}
  thrown in /var/www/html/test.php on line 5

A change here is triggering the error.
#11622

The essence of the problem is that PDO SQLSRV is throwing an error instead of returning false when set_attribute fails.
microsoft/msphpsql#1468
laravel/framework#47937

I've been working on this issue for about 20 days now and the hotfix for PDO SQLSRV has not yet been released and things are looking a little shady.

However, since there are many people who are in trouble, I thought about creating a PR for a workaround with php-src. What do you think?
#12039

remarks:
I've checked all the PDO drivers provided by PECL and found that only one, PDO SQLSRV, gives an error under the same conditions.

PHP Version

PHP 8.1.22+, 8.2.9+, 8.3.0+

Operating System

No response

@SakiTakamachi
Copy link
Member Author

@Girgias
Since you were reviewing #11622, I would be happy if you could give me your opinion.

@Girgias
Copy link
Member

Girgias commented Aug 24, 2023

I don't think we should be monkey patching around a database driver, even though in some sense, the previous PR was monkey patching the MySQL driver. But this has good reasons, as the MySQLnd driver supports differing behaviour.

That PDO attribute has existed since at least PHP 5.3, and was probably somewhat of a bug to not propagate this down to the underlying driver.

Moreover, there might be reasons for the underlying driver to actually warn/throw. I don't understand why we should be monkey patching when seemingly the only thing preventing this issue to be fixed is for a multi-billion corporation to release a new version of their driver without doing any work as you provided the fix.

People who are affected by this should be pressuring Microsoft in releasing a new driver, not pressuring us.

@SakiTakamachi
Copy link
Member Author

@Girgias
Thank you for your feedback!

I don't understand why we should be monkey patching when seemingly the only thing preventing this issue to be fixed is for a multi-billion corporation to release a new version of their driver without doing any work as you provided the fix.

Yes, honestly I completely agree.
I was proposing a fix that I wasn't satisfied with, but I was a little relieved to find someone with the same opinion.

I haven't heard back from the PDO SQLSRV lead yet, so I'll wait for his response first.

Thank you for your precious time.

@SakiTakamachi
Copy link
Member Author

I am closing this issue and the associated PR.
I hope it never opens again.

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

No branches or pull requests

2 participants