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

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

Closed
SakiTakamachi opened this issue Jul 4, 2023 · 16 comments

Comments

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Jul 4, 2023

Description

PDO::ATTR_EMULATE_PREPARES = true
PDO::ATTR_STRINGIFY_FETCHES = true

// DB value is 3.60, type is float

// php8.0
"3.60"

// after php8.1
"3.6"

document
MySQL Driver

Integers and floats in result sets will now be returned using native PHP types instead of strings when using emulated prepared statements. This matches the behavior of native prepared statements. The previous behaviour can be restored by enabling the PDO::ATTR_STRINGIFY_FETCHES option.

I do not know if this is the intended change, but I believe that either the implementation or the documentation must be corrected, since at least the documentation is different from the actual situation.

What do you think?

I posted the same information in the document issue.
php/doc-en#2557

PHP Version

PHP 8.2.7

Operating System

No response

@youkidearitai
Copy link
Contributor

I think need more detailed explanation.

  • What is MySQL version?
  • Why do you think PHP's bug? (Why don't think MySQL's bug?)
  • If PHP's bug, what code is that? (I need to reproduce code.)

At least, MySQL 8.0.33 (Server version: 8.0.33-0ubuntu0.20.04.2 (Ubuntu)) and PHP 8.0 using PDO, displays 3.6.

mysql> select cast(3.60 as double);
+----------------------+
| cast(3.60 as double) |
+----------------------+
|                  3.6 |
+----------------------+
1 row in set (0.00 sec)

I using below code, I can't reproduced on 8.0 and 8.1

<?php
// This is development environment (my local environment) :)
$pdo = new PDO("mysql:host=127.0.0.1;dbname=ore", "root", "password");
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
$pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true);

$results = $pdo->query("SELECT cast(3.60 AS float)");
foreach ($results as $result) {
        var_dump($result);
}

Result is same.

array(2) {
  ["cast(3.60 AS float)"]=>
  string(3) "3.6"
  [0]=>
  string(3) "3.6"
}

@SakiTakamachi
Copy link
Member Author

SakiTakamachi commented Jul 5, 2023

I certainly didn't have enough information.
I built a new docker environment and tested it.

php:8.1.20-cli
php:8.0.29-cli
mysql:8.0.33

Also, I described it as a float type, but it was actually a float(3,2) type, sorry.

I have created a table with the following structure.

create table test (
    float_col float(3, 2),
    double_col double(3, 2),
    decimal_col decimal(3, 2)
);

I then inserted the following record.

insert into test (float_col, double_col, decimal_col) values (2.60, 3.60, 4.60);

With that in place, I tested the following code.

<?php
$pdo = new PDO("mysql:host=mysql;dbname=test", "root", ""); // my env
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
$pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true);

$results = $pdo->query("SELECT * from test");
foreach ($results as $result) {
     var_dump($result);
}

The results are as follows.

// PHP 8.1

array(6) {
  ["float_col"]=>
  string(3) "2.6"
  [0]=>
  string(3) "2.6"
  ["double_col"]=>
  string(3) "3.6"
  [1]=>
  string(3) "3.6"
  ["decimal_col"]=>
  string(4) "4.60"
  [2]=>
  string(4) "4.60"
}
// PHP 8.0
array(6) {
  ["float_col"]=>
  string(4) "2.60"
  [0]=>
  string(4) "2.60"
  ["double_col"]=>
  string(4) "3.60"
  [1]=>
  string(4) "3.60"
  ["decimal_col"]=>
  string(4) "4.60"
  [2]=>
  string(4) "4.60"
}

The decimal type shows no difference.
However, for other types, the results are different for 8.1 and 8.0.

I know that these types will not be supported in the future.

But in any case, I reported it because there are cases where the "behavior of previous versions" cannot be reproduced as described in the documentation.

I could not determine if it was a bug in the first place, but I could not find the proper place to post the issue, so I posted it here.

@youkidearitai
Copy link
Contributor

@SakiTakamachi Thanks for further information! I don't know well ext/pdo, ext/pdo_mysql. I'll try research.

Does anyone know?

@SakiTakamachi
Copy link
Member Author

SakiTakamachi commented Jul 5, 2023

Thank you for confirming!

It seems that PDO::ATTR_STRINGIFY_FETCHES is simply cast to string type.

Running the same test with PDO::ATTR_STRINGIFY_FETCHES = false results in:

// PHP 8.1

array(6) {
  ["float_col"]=>
  float(2.6)
  [0]=>
  float(2.6)
  ["double_col"]=>
  float(3.6)
  [1]=>
  float(3.6)
  ["decimal_col"]=>
  string(4) "4.60"
  [2]=>
  string(4) "4.60"
}
// PHP 8.0

array(6) {
  ["float_col"]=>
  string(4) "2.60"
  [0]=>
  string(4) "2.60"
  ["double_col"]=>
  string(4) "3.60"
  [1]=>
  string(4) "3.60"
  ["decimal_col"]=>
  string(4) "4.60"
  [2]=>
  string(4) "4.60"
}

Presumably due to the change in behavior when PDO::ATTR_EMULATE_PREPARES is true, it tries primitive numeric types once before casting to string, so zerofill information is lost at that time.

@youkidearitai
Copy link
Contributor

@SakiTakamachi
Please see manual: Backward Incompatible Changes MySQL Driver
Japanese: 下位互換性のない変更点 MySQL ドライバ

Integers and floats in result sets will now be returned using native PHP types instead of strings when using emulated prepared statements.

This seems behavior change is intentional to PHP 8.1.

@SakiTakamachi
Copy link
Member Author

@youkidearitai

I have already read this document.

As described in this issue, The issue is that there is a statement in the documentation that says:

The previous behaviour can be restored by enabling the PDO::ATTR_STRINGIFY_FETCHES option.

It may not be restored in practice, so either the implementation or the documentation should be modified to eliminate the discrepancy.

@youkidearitai
Copy link
Contributor

@SakiTakamachi Thanks, I understand.
I want to simple this issue.
This issue title is thinks that. "When PDO::ATTR_STRINGIFY_FETCHES is true, 0 is not output when floating point with fixed digits ends in 0".

I found this behavior changes commit caa7100, but convert double to string is not changed.
PHP 8.1 is changed logic convert double to string: 6048481. This around may be related.

@youkidearitai
Copy link
Contributor

youkidearitai commented Jul 6, 2023

I think floating point problem changes. @nikic if you'd like I would you thoughts.

@youkidearitai
Copy link
Contributor

Oops, I misunderstanding. I'm sorry. Reproduce in PDO::ATTR_EMULATE_PREPARES should be true.
I experimented below. (written Japanese).
https://gist.github.com/youkidearitai/8f3a8046fbf0e26cc8872b2c0e791086

@SakiTakamachi
Copy link
Member Author

SakiTakamachi commented Jul 7, 2023

Thank you for confirming.

I am preparing a pull request.
#11616
#11622

SakiTakamachi added a commit to SakiTakamachi/php-src that referenced this issue Jul 7, 2023
Girgias added a commit that referenced this issue Jul 17, 2023
* PHP-8.1:
  Fix GH-11587 PDO::ATTR_STRINGIFY_FETCHES should return strings even in if PDO::ATTR_EMULATE_PREPARES is enabled
Girgias added a commit that referenced this issue Jul 17, 2023
* PHP-8.2:
  Fix GH-11587 PDO::ATTR_STRINGIFY_FETCHES should return strings even in if PDO::ATTR_EMULATE_PREPARES is enabled
@LichP
Copy link

LichP commented Aug 9, 2023

Noting that upgrading php from 8.1.21 to 8.1.22 is resulting in the following exception being thrown on Drupal sites using the sqlsrv module:

PDOException  SQLSTATE[IMSSP]: An invalid attribute was designated on the PDO object.

This exception is being thrown by $connection->setAttribute(\PDO::ATTR_STRINGIFY_FETCHES, TRUE);. In both confirmed cases the common factor has been upgrading to php 8.1.22, in my case there was also an upgrade to the msodbcsql17 driver package that happened at the same time. Our Drupal.org issue for this is #3379887

@youkidearitai
Copy link
Contributor

@LichP If you said correct, maybe is this https://github.com/php/php-src/pull/11622/files#diff-9a6d6a4359a98668fef29400492f877938424d1b1d2e84933c91ab34161cf360R795-R797 ?

This issue/PR is about pdo/mysql, I never thought it would affect SQL Server.

@SakiTakamachi Any thoughts?

@SakiTakamachi
Copy link
Member Author

This is certainly an effect of my modification.

First reported here.
laravel/framework#47937
#11622 (comment)

PDO SQLSRV is an ext outside the php-src and I have a fix PR now, but no progress....
microsoft/msphpsql#1468

@youkidearitai
Copy link
Contributor

@LichP Thank you very much for report.

@SakiTakamachi trying microsoft/msphpsql#1468 , but seems don't good response, Therefore, could you think about implementation workaround?

@LichP
Copy link

LichP commented Aug 9, 2023

Thanks everyone for looking into this - hopefully @SakiTakamachi 's msphpsql PR will be committed soon. To work around this I've wrapped the setAttribute call in the Drupal sqlsrv module in a try / catch. As I understand it the exception is only triggered by the call into the driver's set attribute implementation after the database handle's stringify attribute has been updated, so the setAttribute call effectively succeeds even though an exception is thrown.

@SakiTakamachi
Copy link
Member Author

This is a very interesting error avoidance method.

I will try it later too.

Thank you!

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

3 participants