Skip to content

Conversation

@marmichalski
Copy link
Contributor

Closes #428.

// fallbacks
if (null === $phpstanType) {
switch (strtoupper($mysqlType)) {
case 'DECIMAL':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(this is probably redundant)

$query = 'SELECT MAX(adaid), MIN(adaid), COUNT(adaid), AVG(adaid) FROM ada WHERE adaid = 1';
$stmt = $pdo->query($query, PDO::FETCH_ASSOC);
assertType('PDOStatement<array{MAX(adaid): int<-32768, 32767>|null, MIN(adaid): int<-32768, 32767>|null, COUNT(adaid): int, AVG(adaid): float|null}>', $stmt);
assertType('PDOStatement<array{MAX(adaid): int<-32768, 32767>|null, MIN(adaid): int<-32768, 32767>|null, COUNT(adaid): int, AVG(adaid): numeric-string|null}>', $stmt);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested and result of AVG is also a string.

Copy link
Owner

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Looks great thx. Did you verify the results on mysqli and pdo+mysql?

switch (strtoupper($mysqlType)) {
case 'DECIMAL':
case 'NEWDECIMAL':
$phpstanType = new AccessoryNumericStringType();
Copy link
Owner

@staabm staabm Sep 25, 2022

Choose a reason for hiding this comment

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

See line 161 how a numeric string is built. An accessory type cannot stand on its own in the phpstan type system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I've seen it, but instantiated it just like this and it seems to work ? 🙈

Copy link
Owner

Choose a reason for hiding this comment

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

It works for the tests but will not work in the bigger picture when the type needs to be combined with other logic within phpstan

@marmichalski
Copy link
Contributor Author

Looks great thx. Did you verify the results on mysqli and pdo+mysql?

I have tested only with pdo. I can do some more testing before we merge this

@marmichalski
Copy link
Contributor Author

Looks great thx. Did you verify the results on mysqli and pdo+mysql?

I have tested only with pdo. I can do some more testing before we merge this

best code
// pdo5 -> pdo + mysql:5.7, pdo8 -> pdo + mysql:8.0; mysqli is mysqli
var_dump(
    $pdo5->query('SELECT eadavk FROM ak')->fetch(\PDO::FETCH_ASSOC),
    $pdo8->query('SELECT eadavk FROM ak')->fetch(\PDO::FETCH_ASSOC),

    $mysqli5->query('SELECT eadavk FROM ak')->fetch_row(),
    $mysqli8->query('SELECT eadavk FROM ak')->fetch_row(),
);
array(1) {
  ["eadavk"]=>
  string(6) "154.23"
}
array(1) {
  ["eadavk"]=>
  string(6) "154.23"
}
array(1) {
  [0]=>
  string(6) "154.23"
}
array(1) {
  [0]=>
  string(6) "154.23"
}

@staabm staabm merged commit b39afd7 into staabm:main Sep 25, 2022
@staabm
Copy link
Owner

staabm commented Sep 25, 2022

Awesome thx, just pushed a new bugfix release

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.

PHP 8.1, PDO, MySQL, Decimal column

2 participants