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

Setting PDO::ATTR_STRINGIFY_FETCHES throws exception #1474

Closed
SakiTakamachi opened this issue Aug 30, 2023 · 14 comments
Closed

Setting PDO::ATTR_STRINGIFY_FETCHES throws exception #1474

SakiTakamachi opened this issue Aug 30, 2023 · 14 comments
Assignees

Comments

@SakiTakamachi
Copy link
Contributor

It is not reasonable to communicate forever in the PR that have already been merged, so I have created a new issue.
Reference: #1468

I've been working on this issue for almost a month now and the PR that fixes the issue was already merged into the dev branch 3 weeks ago.
What's stopping you from releasing it?

I'm sure all contributors willing to help this repository will do so once it's clear what the obstacles are that are preventing a release.

PHP version
PHP 8.1.22+ or 8.2.9+ or 8.3.0+

PHP SQLSRV or PDO_SQLSRV version
All version

Problem description
Due to modifications made to PDO Core's set_attributes function, attempting to set PDO::ATTR_STRINGIFY_FETCHES will throw an exception.

Expected behavior and actual behavior
I was expecting no exception to be thrown.

Actual result:

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

Repro code or steps to reproduce

<?php

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

@David-Engel
I've opened an issue, so please respond here.
We've been left alone for about a week, so I'm hoping for some response soon.

@youkidearitai
Copy link

@David-Engel Upstream(php-src) rejected to your opinion and we should fight to Microsoft pressure. You should release new version that include this PR.
related: php/php-src#12038

@David-Engel
Copy link
Contributor

Had a discussion with the team the other day. We'll release a hotfix to accommodate the behavior change made upstream. The original fix needs some adjustment, though. We only release PHP minor-version-specific binaries, so the compile time #if logic needs adjustment. @absci is working on the release.

@SakiTakamachi
Copy link
Contributor Author

@David-Engel
I understand the situation. Thank you very much for your efforts in implementing the hotfix.

I am very relieved to receive this news.
We are looking forward to the release.

@absci
Copy link
Contributor

absci commented Aug 30, 2023

Hi, I'll be working on the hotfix release.

@SakiTakamachi
Copy link
Contributor Author

@absci
Thank you. This is truly great news.

@youkidearitai
Copy link

@absci
@David-Engel
Thanks very much for moving forward and I'm sorry to using strong words.

ndm2 added a commit to icings/partitionable that referenced this issue Sep 1, 2023
ndm2 added a commit to icings/partitionable that referenced this issue Sep 1, 2023
@qqzm
Copy link

qqzm commented Sep 6, 2023

Hi,
Is this hotfix available yet? If not, what is the timescale?
How do we install it?

Thanks,
Stephen

@absci
Copy link
Contributor

absci commented Sep 7, 2023

The new release should be ready by the end of this week.

@agould
Copy link

agould commented Sep 8, 2023

I see 5.11.1 has been released to fix this.

Will there also be a 5.10.x release that resolves this issue? My company is unfortunately stuck on SQL Server 2012 so I cannot use 5.11.x.

@David-Engel
Copy link
Contributor

Will there also be a 5.10.x release that resolves this issue? My company is unfortunately stuck on SQL Server 2012 so I cannot use 5.11.x.

@agould

There are no plans to backport the change to 5.10.

Have you tried 5.11 against SQL Server 2012? Since most of the functionality is in the underlying ODBC driver and we haven't made significant changes from 5.10 to 5.11, I would be surprised if 5.11 didn't work fine against it. I realize 5.11 doesn't technically "support" SQL 2012, but that's mainly because we stop testing against SQL versions that are out of support.

@agould
Copy link

agould commented Sep 8, 2023

@David-Engel Thanks for the response. I have not tried 5.11. I will give it a try in one of our test environments.

One thing I did try this morning was removing the ATTR_STRINGIFY_FETCHES attribute:

$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
//$dbh->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);
$dbh->setAttribute(PDO::SQLSRV_ATTR_FETCHES_NUMERIC_TYPE, true);
$dbh->setAttribute(PDO::SQLSRV_ATTR_QUERY_TIMEOUT, 60);

We'd been using the first two lines since we used MySQL/MariaDB, prior to moving to SQL Server about 5 years ago, so I never thought to revisit these attributes as they just worked.

Without the ATTR_STRINGIFY_FETCHES, I don't see any negative affects. Seems the SQLSRV_ATTR_FETCHES_NUMERIC_TYPE attribute handles what we need.

@David-Engel
Copy link
Contributor

removing the ATTR_STRINGIFY_FETCHES attribute

That should work for you, too, since that's the source of the issue being fixed. 👍

@SakiTakamachi
Copy link
Contributor Author

Thank you for releasing the hotfix!
Thank you for taking the time out of your busy schedule.

I close this issue.

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

No branches or pull requests

6 participants