Fixed error when using PDO::ATTR_STRINGIFY_FETCHES #1468

SakiTakamachi · 2023-08-03T16:13:23Z

This fixes the issue reported here.

Fixes for errors caused by PDO changes in php8.1.22 and 8.2.9.

test code

<?php

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

… after 8.2.9 php version

GrahamCampbell · 2023-08-04T08:38:42Z

Why does this PR change nothing?

SakiTakamachi · 2023-08-04T08:40:16Z

A test was crashing unexpectedly, so I'm temporarily running it without any changes.
Revert when the current test is finished.

… after 8.2.9 php version

SakiTakamachi · 2023-08-04T08:48:10Z

The tests seem to fail even without any changes, and there seems to be some unstable tests.

SakiTakamachi · 2023-08-04T09:38:53Z

@v-makouz
If you don't mind, could you please review it?

SakiTakamachi · 2023-08-04T17:33:03Z

This test will fail even if I didn't make any changes.

I don't understand why this is the case, just by looking at it lightly.

absci · 2023-08-04T18:28:49Z

Thanks for your contribution, there're some tests may fail intermittently due to connection. I'll rerun it.

SakiTakamachi · 2023-08-05T17:47:05Z

@absci

Thank you for confirmation.

Am I correct in thinking that I have to rerun the connection test many times until it succeeds?

youkidearitai · 2023-08-09T11:12:13Z

@absci

Hi, I heard PHP 8.1.22 and 8.2.8 throws Exception when PDO::ATTR_STRINGIFY_FETCHES.
I would appreciate it if you could pass this pull request.

Regards.

absci · 2023-08-09T17:24:33Z

I have merged this PR, it'll be included in the next release.

SakiTakamachi · 2023-08-09T17:29:37Z

Thank you for confirming and merging!

jarkko-hautakorpi · 2023-08-10T07:59:37Z

Are there perhaps some undocumented changes coming, or was it a bug and no changes intended?
https://learn.microsoft.com/en-us/sql/connect/php/pdo-setattribute?view=sql-server-ver16
This documentation is still valid?

SakiTakamachi · 2023-08-10T11:51:49Z

This documentation is still valid?

Yes, it is.

This is a fix that removes the error caused by changes in PDO core.

AbrahamBrookes · 2023-08-11T02:57:15Z

I also got hit with this issue, thanks for the PR and the merge!

pirex360 · 2023-08-11T09:47:04Z

#Hi, I have also : SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object error.

In laravel v9 and PHP 8.1.22 with MSODBC 18, the plesk has auto-updated and the driver for sqlsrv doesn't work anymore.

I tried everything, removing all PDO::ATTR or PDO::SQLSRV, and the result is the same! Here is a sample of the connection data:

array(
                'driver'        => 'sqlsrv',
                'host'          => 'xxx.xxx.xxx.xxx',
                'port'          => 999999,
                'database'      => 'testdb',
                'username'      => 'johndoe',
                'password'      => 'test',
                'charset'       => 'utf8',
                'prefix'        => '',
                'prefix_indexes' => true,
                'options'       => array(
                    PDO::ATTR_EMULATE_PREPARES => true,
                    PDO::ATTR_STRINGIFY_FETCHES => false,
                    PDO::ATTR_CASE => PDO::CASE_LOWER,
                    PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE => true,
                    PDO::SQLSRV_ATTR_FORMAT_DECIMALS => true,
                ),
                'encrypt' => 'yes',     // added for MSSQL 18 to work
                'trust_server_certificate' => true, // added for MSSQL 18 to work
 )

I also tried upgrading to PHP 8.2.8 and the result is the same.
I have pdo_sqlsrv and sqldrv extensions on 5.10.1.

The only way it worked, was downgrading to PHP 7.4 and I make a simple PHP script to test db connection and works, but this is not a solution for me, because I have a lot of composer dependencies for PHP 8.1 and I can't downgrade it! will be a messy job and some app features will be broken.

Can anybody help/guide me to the solution to this problem?

SakiTakamachi · 2023-08-11T10:32:53Z

@pirex360

I also tried upgrading to PHP 8.2.8 and the result is the same.

Is it really php8.2.8?
This issue occurs on 8.2.9 and later.

pirex360 · 2023-08-11T10:42:23Z

@pirex360

I also tried upgrading to PHP 8.2.8 and the result is the same.

Is it really php8.2.8? This issue occurs on 8.2.9 and later.

Sorry, it was my mistake... I put in PHP 8.2.8 and re-install the driver and now it works !!! in PLESK if you change the PHP version you have to install again the driver, even if you have it in 8.2.0.

Now my question shifted...what will happen if PLESK auto updates and forces us to use PHP 8.2.9 or + ? Is this a temporary fix?

SakiTakamachi · 2023-08-11T11:44:26Z

@pirex360

This fix is still in the process of being merged into the dev branch and has not been released.

It will be included in the next release.

If you update to php8.2.9 again before the release, of course the problem will reoccur.

Using the driver that is new version after release will permanently fix this issue.

ssswang · 2023-08-11T19:06:21Z

@pirex360
If you upgrade to php 8.2.9, you will get SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object because of PDO::ATTR_STRINGIFY_FETCHES => false,

In my own limited experiment and testing, it is safe to remove this line. Please correct me if I am wrong.

This is also a solution wrote by SakiTakamachi using a try catch block without removing PDO::ATTR_STRINGIFY_FETCHES => false,
laravel/framework#47937 (comment)

SakiTakamachi · 2023-08-11T19:51:16Z

@ssswang

Yes, you are right.

In the current php specification, the initial value of PDO::ATTR_STRINGIFY_FETCHES is false.

Therefore, it can be controlled by specifying SQLSRV_ATTR_FETCHES_NUMERIC_TYPE.

You can safely delete the PDO::ATTR_STRINGIFY_FETCHES line.

neclimdul · 2023-08-11T20:09:51Z

"after release" 😞

ssswang · 2023-08-11T20:09:52Z

@SakiTakamachi
Arigato, Sakichi. Thank you for confirming.
It's 5 AM in Japan. Don't work too hard. ^^

pirex360 · 2023-08-12T10:45:44Z

@pirex360 If you upgrade to php 8.2.9, you will get SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object because of PDO::ATTR_STRINGIFY_FETCHES => false,

In my own limited experiment and testing, it is safe to remove this line. Please correct me if I am wrong.

This is also a solution wrote by SakiTakamachi using a try catch block without removing PDO::ATTR_STRINGIFY_FETCHES => false, laravel/framework#47937 (comment)

It's weird, In my testing... I remove all the options array from the connection :

 'options'       => array(
                    PDO::ATTR_EMULATE_PREPARES => true,
                    PDO::ATTR_STRINGIFY_FETCHES => false,
                    PDO::ATTR_CASE => PDO::CASE_LOWER,
                    PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE => true,
                    PDO::SQLSRV_ATTR_FORMAT_DECIMALS => true,
                ),

And still, the error appears...

SakiTakamachi · 2023-08-12T10:51:52Z

It doesn't make much sense to test it in Laravel. Because the initial value is set inside Laravel, it is not possible to completely remove the option specification.

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Connectors/SqlServerConnector.php#L19

SakiTakamachi · 2023-08-12T10:54:35Z

And removing an option doesn't mean setting it to false, it means not specifying anything.

pirex360 · 2023-08-12T12:14:59Z

And removing an option doesn't mean setting it to false, it means not specifying anything.

Sure, You are right... it fallback to the defaults... but doesn't the default supported by the driver?
The key is your "Because the initial value is set inside Laravel, it is not possible to completely remove the option specification.", it points to the problem of Laravel defaults and not Driver Default.

Thanks for your time, it was enlightenment.

JGlueck-WIKA · 2023-08-17T05:58:56Z

@absci Any news on when the next release will take place?

absci · 2023-08-17T22:56:02Z

@absci Any news on when the next release will take place?

There's no exact time at the moment, possibly the end of 2023.

SakiTakamachi · 2023-08-17T23:33:31Z

@absci

Is it difficult to release a patch version that addresses this issue as 5.11.1 first?

absci · 2023-08-18T00:36:37Z

@absci

Is it difficult to release a patch version that addresses this issue as 5.11.1 first?

I'll discuss it with management levels. Maybe we can do a hotfix release.

SakiTakamachi · 2023-08-18T00:47:20Z

I'll discuss it with management levels. Maybe we can do a hotfix release.

I apologize for the trouble.
Thanks so much.

skylord123 · 2023-08-18T17:06:25Z

I'll discuss it with management levels. Maybe we can do a hotfix release.

This is currently breaking the build process for my company. Since we build our container off Ubuntu using Ondrej repo we are stuck on PHP 8.1.22 that has this problem. So a hotfix would be a godsend to getting this resolved.

Also thanks @SakiTakamachi for a good job finding and fixing this.

kokoshneta · 2023-08-22T07:49:51Z

@absci
Is it difficult to release a patch version that addresses this issue as 5.11.1 first?

I'll discuss it with management levels. Maybe we can do a hotfix release.

Since this is currently breaking every single Laravel install where the developer is unable to control the minor version of PHP (e.g., Plesk-hosted – once you’re on 8.2.9, it seems you can’t downgrade back to 8.2.8 without Plesk auto-re-upgrading), a hotfix would be highly appreciated.

JGlueck-WIKA · 2023-08-22T08:41:11Z

For those of you with a Laravel installation also suffering from this problem at the moment - the workaround suggested here helped us to circumnavigate that issue for now.

Anyhow, a release of this fix would be highly appreciated.

wwwDESIGN-basti · 2023-08-22T20:39:05Z

Here is the easiest und final solution:
https://github.com/SakiTakamachi/laravel-sqlsrv-err-avoid

install it and the problem is gone.

skylord123 · 2023-08-22T21:14:11Z

Here is the easiest und final solution: https://github.com/SakiTakamachi/laravel-sqlsrv-err-avoid

install it and the problem is gone.

Not a final solution, just a bandaid. Basically just ignores the problem this driver is currently causing. The final solution would be to get a patched version of msphpsql.

I have over 25+ services so manually installing this bandaid until this is fixed then removing it is quite a large effort.

David-Engel · 2023-08-22T22:40:55Z

Think this PR should not have been merged as-is. The fix should be made in PHP (the offending PR should be reverted), where the break happened. This change will be fine if it is needed for PHP 8.3. But don't make everyone else patch out-of-band for an upstream error.

Changes that break backwards compatibility should only be made on version changes where you might expect to have to test for version compatibility. Upgrading from PHP 8.2.8 to PHP 8.2.9 shouldn't break downstream apps.

SakiTakamachi · 2023-08-23T00:49:21Z

@David-Engel

All other pdo exts do not raise an error under the same conditions.

Only PDO SQLSRV raises an error.

So I personally don't think this is a BC break.

It takes a lot of effort to consider all downstream proprietary specifications in order to make upstream modifications.

I have checked all 16 pdo drivers present in pecl and only sqlsrv causes the error, even those not in php-src.

SakiTakamachi · 2023-08-23T02:03:05Z

Even if you look at the PDO documentation, it doesn't say that setAttribute throw an exception, and it states that it simply returns false on failure.

https://www.php.net/manual/en/pdo.setattribute.php#refsect1-pdo.setattribute-returnvalues

Therefore, I believe that the essence of this problem is that PDO SQLSRV does not conform to the PDO specification.

youkidearitai · 2023-08-23T15:13:01Z

@David-Engel No, I don't think PHP 8.2.9 is BC break. Because this is done for the convenience of throwing parameters other than the allowed list that msphpsql is doing arbitrarily. I think should fix to msphpsql.

youkidearitai · 2023-08-23T15:17:39Z

@David-Engel Or are you saying that you don't allow modifying ext/pdo/pdo_dbh.c in php-src?

youkidearitai · 2023-08-23T15:23:52Z

@David-Engel Or are you saying that you don't allow modifying ext/pdo/pdo_dbh.c in php-src?

I think one thing, If msphpsql isn't going to chase php-src, it should be proposed in an RFC to have it integrated into php-src.

SakiTakamachi · 2023-08-24T12:51:06Z

@David-Engel
This is a problem that this repository should solve, as the proprietary error throwing spec is causing this issue.
I really hope for an early hotfix release.
If you want a fix at the underlying specification level rather than a monkey patch, I'm ready to help with that fix.

SakiTakamachi · 2023-08-30T16:34:52Z

I have created an issue because I can't keep talking forever in this PR.
#1474

* fix accept PDO_ATTR_STRINGIFY_FETCHES to set_attr on after 8.1.22 and after 8.2.9 php version * try to put it back * fix accept PDO_ATTR_STRINGIFY_FETCHES to set_attr on after 8.1.22 and after 8.2.9 php version

SakiTakamachi marked this pull request as ready for review August 3, 2023 16:18

SakiTakamachi changed the base branch from master to dev August 4, 2023 06:23

fix accept PDO_ATTR_STRINGIFY_FETCHES to set_attr on after 8.1.22 and…

3651a6e

… after 8.2.9 php version

SakiTakamachi force-pushed the fix/using_attr_stringify_fetches branch from 85b09d1 to 3651a6e Compare August 4, 2023 07:01

try to put it back

3193854

GrahamCampbell mentioned this pull request Aug 4, 2023

laravel PDO invalid attribute exception with PHP 8.2.9/8.1.22 laravel/framework#47937

Closed

fix accept PDO_ATTR_STRINGIFY_FETCHES to set_attr on after 8.1.22 and…

6102236

… after 8.2.9 php version

SakiTakamachi mentioned this pull request Aug 9, 2023

After php8.1, when PDO::ATTR_EMULATE_PREPARES is true and PDO::ATTR_STRINGIFY_FETCHES is true, decimal zeros are no longer filled. php/php-src#11587

Closed

absci approved these changes Aug 9, 2023

View reviewed changes

absci merged commit c648caf into microsoft:dev Aug 9, 2023

henry11996 mentioned this pull request Aug 21, 2023

Build an older version for php8.1.22 and 8.2.9 Namoshek/docker-php-mssql#11

Closed

boboldehampsink mentioned this pull request Aug 22, 2023

ext-pdo_sqlsrv robuust/heroku-php#7

Draft

SakiTakamachi mentioned this pull request Aug 23, 2023

An error occurs in set_attribute when using PDO SQLSRV php/php-src#12038

Closed

This was referenced Aug 23, 2023

[10.x] Fix createOrFirst on transactions laravel/framework#48144

Merged

[10.x] Ignore MSSQL exception about PDO::ATTR_STRINGIFY_FETCHES laravel/framework#48158

Closed

SakiTakamachi mentioned this pull request Aug 30, 2023

Setting PDO::ATTR_STRINGIFY_FETCHES throws exception #1474

Closed

Fixed error when using PDO::ATTR_STRINGIFY_FETCHES #1468

Fixed error when using PDO::ATTR_STRINGIFY_FETCHES #1468

Conversation

SakiTakamachi commented Aug 3, 2023 • edited Loading

GrahamCampbell commented Aug 4, 2023

SakiTakamachi commented Aug 4, 2023

SakiTakamachi commented Aug 4, 2023

SakiTakamachi commented Aug 4, 2023

SakiTakamachi commented Aug 4, 2023 • edited Loading

absci commented Aug 4, 2023

SakiTakamachi commented Aug 5, 2023

youkidearitai commented Aug 9, 2023

absci commented Aug 9, 2023

SakiTakamachi commented Aug 9, 2023

jarkko-hautakorpi commented Aug 10, 2023 • edited Loading

SakiTakamachi commented Aug 10, 2023

AbrahamBrookes commented Aug 11, 2023

pirex360 commented Aug 11, 2023 • edited Loading

SakiTakamachi commented Aug 11, 2023

pirex360 commented Aug 11, 2023

SakiTakamachi commented Aug 11, 2023

ssswang commented Aug 11, 2023 • edited Loading

SakiTakamachi commented Aug 11, 2023 • edited Loading

neclimdul commented Aug 11, 2023

ssswang commented Aug 11, 2023

pirex360 commented Aug 12, 2023

SakiTakamachi commented Aug 12, 2023

SakiTakamachi commented Aug 12, 2023

pirex360 commented Aug 12, 2023

JGlueck-WIKA commented Aug 17, 2023

absci commented Aug 17, 2023

SakiTakamachi commented Aug 17, 2023

absci commented Aug 18, 2023

SakiTakamachi commented Aug 18, 2023

skylord123 commented Aug 18, 2023

kokoshneta commented Aug 22, 2023

JGlueck-WIKA commented Aug 22, 2023 • edited Loading

wwwDESIGN-basti commented Aug 22, 2023

skylord123 commented Aug 22, 2023

David-Engel commented Aug 22, 2023

SakiTakamachi commented Aug 23, 2023 • edited Loading

SakiTakamachi commented Aug 23, 2023 • edited Loading

youkidearitai commented Aug 23, 2023

youkidearitai commented Aug 23, 2023

youkidearitai commented Aug 23, 2023

SakiTakamachi commented Aug 24, 2023

SakiTakamachi commented Aug 30, 2023

SakiTakamachi commented Aug 3, 2023 •

edited

Loading

SakiTakamachi commented Aug 4, 2023 •

edited

Loading

jarkko-hautakorpi commented Aug 10, 2023 •

edited

Loading

pirex360 commented Aug 11, 2023 •

edited

Loading

ssswang commented Aug 11, 2023 •

edited

Loading

SakiTakamachi commented Aug 11, 2023 •

edited

Loading

JGlueck-WIKA commented Aug 22, 2023 •

edited

Loading

SakiTakamachi commented Aug 23, 2023 •

edited

Loading

SakiTakamachi commented Aug 23, 2023 •

edited

Loading