staabm / phpstan-dba

PHPStan based SQL static analysis and type inference for the database access layer
https://staabm.github.io/archive.html#phpstan-dba
MIT License
250 stars 17 forks source link

AST: inconsistent behavior with functions and aliases #578

Closed hemberger closed 1 year ago

hemberger commented 1 year ago

There is the following test in sql-ast-narrowing.php:

$stmt = $pdo->query('SELECT avg(freigabe1u1) as avg from ada');
assertType('PDOStatement<array{avg: int<-128, 127>, 0: int<-128, 127>}>', $stmt);

If I remove the alias, then I get a different (and inconsistent) type:

$stmt = $pdo->query('SELECT avg(freigabe1u1) from ada');
assertType('PDOStatement<array{avg(freigabe1u1): numeric-string|null, 0: int<-128, 127>}>', $stmt);

It looks like the only place it is correct is the associative parameter in the unaliased query, where it gets avg(freigabe1u1): numeric-string|null (since the result of avg shouldn't be an integer).

Here's another example with min instead of avg.

$stmt = $pdo->query('SELECT min(freigabe1u1) as min from ada');
assertType('PDOStatement<array{min: int<-128, 127>, 0: int<-128, 127>}>', $stmt);
$stmt = $pdo->query('SELECT min(freigabe1u1) from ada');
assertType('PDOStatement<array{min(freigabe1u1): int<-32768, 32767>|null, 0: int<-128, 127>}>', $stmt);

Again, only min(freigabe1u1): int<-32768, 32767>|null is correct, because the field is a SMALLINT, not a TINYINT.

I've spot checked a few of the numeric functions, and so far they have all exhibited this behavior (avg, min, max, sum, round, abs).

staabm commented 1 year ago

I think this is the same problem as https://github.com/staabm/phpstan-dba/pull/570

staabm commented 1 year ago

@hemberger this one can be closed now?

hemberger commented 1 year ago

@hemberger this one can be closed now?

I think all the alias and integer issues have been resolved, but there is still an issue with AVG (it should not be restricted to integers). Do you want me to close this and report that separately?

staabm commented 1 year ago

no lets leave it open then. thx for the update