Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Query result type inference too generic for certain function results #272

Closed
janedbal opened this issue Jan 27, 2022 · 10 comments
Closed

Query result type inference too generic for certain function results #272

janedbal opened this issue Jan 27, 2022 · 10 comments

Comments

@janedbal
Copy link
Contributor

janedbal commented Jan 27, 2022

  • See result of functions here and here. I hope I'm not mistaken, but those functions (COUNT, SUM, AVG, MIN, MAX, LENGTH) should always return int, do they not? Those are now infered as int<0, max>|numeric-string.
  • Also, this CASE should always be int imo.
  • And IDENTITY function seems to be infered as mixed even though the type of the FK should be known.
@ondrejmirtes
Copy link
Member

/cc @arnaud-lb

@arnaud-lb
Copy link
Contributor

arnaud-lb commented Jan 27, 2022

Hi @janedbal

Thank you for your detailed bug reports :)

  • See result of functions here and here. I hope I'm not mistaken, but those functions (COUNT, SUM, AVG, LENGTH) should always return int, do they not? Those are now infered as int<0, max>|numeric-string.
  • Also, this CASE should always be int imo.

Doctrine does a good job at consistently returning the right type for entity fields. However, for expressions or function calls it just returns the raw value from the database.

So the type of function calls and arithmetic expressions really depends on the database driver (and the driver version).

For instance, the pdo_mysql and pdo_sqlite drivers return integers and floats as strings in PHP < 8.1, or as integer/float in PHP 8.1 (unless PDO::ATTR_STRINGIFY_FETCHES is set, and unless the integer/float is too large to fit in PHP's integer/float types).

Since Doctrine 2.8, the functions COUNT and LENGTH are casted as int by Doctrine, though, so phpstan-doctrine infers these expressions as just int<0,max> when using Doctrine >= 2.8.

For other expressions, I chose to infer types that are correct in all environments, for now. It may be possible to refine these in a future version, although it may be challenging in some cases, and also lead to type instability (it depends on too many factors that may change depending on the environment).

In short, depending on many factors including the PHP version, drivers, and driver options, integer expressions in DQL queries may be returned as int or numeric-string, so phpstan-doctrine infers them as int|numeric-string for now.

  • And IDENTITY function seems to be infered as mixed even though the type of the FK should be known.

IDENTITY is one of the few features that was not supported in the initial PR, but it should be easy to add it now.

@janedbal
Copy link
Contributor Author

Thank you, now I see the huge complexity you were dealing when implementing this. Good work!

Anyway, shouldnt you be able to detect all those settings from EM when provided (parameters.doctrine.objectManagerLoader)? Driver should be known, PHP version too, Doctrine version as well. Problematic might be PDO setup (if you dont want to query the database).

@arnaud-lb
Copy link
Contributor

Anyway, shouldnt you be able to detect all those settings from EM when provided

We could definitely, but there is the question of the type stability: It can actually be valuable that inferred types don't change with the environment, as it makes the resulting code more portable. Maybe we could add a config parameter for this.

@janedbal
Copy link
Contributor Author

Maybe we could add a config parameter for this.

I think that would be beneficial.

@janedbal
Copy link
Contributor Author

it makes the resulting code more portable

That might be super important for libraries, but not for company codebase, where you use single driver and single setup. So as said, some config might be handy so that everybody could decide what is better for his codebase.

@DarthLegiON
Copy link

DarthLegiON commented Jun 8, 2024

Can I bump this up?
I have a problem:

return $this->createQueryBuilder('pt')
    ->select(['pt as postTag', 'count(p) as cnt'])
    //...
    ->getResult();

Doctrine 3, Postgres 15.
Such code must return list<array{postTag: PostTag, cnt: int}> but by phpstan, returns list<array{postTag: PostTag, cnt: int<0, max>|numeric-string}>. This problem started only in phpstan/doctrine-bundle:1.4.* and doesn't reproduce in 1.3.69. How can I fix this quickly (except for downgrading the package)?

@janedbal
Copy link
Contributor Author

I'm currently working on precise implementation of types coming from such DQLs: #506 So you can expect this to be correct in near future.

@DarthLegiON
Copy link

I can't understand why it's been broken from 1.4, but OK, let's wait for improve. Thanks!

@janedbal janedbal closed this as completed Aug 7, 2024
Copy link

github-actions bot commented Sep 8, 2024

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants