AST doubts regarding null/not-null

[jakubvojacek]

Hello @staabm

I am not sure if the AST narrow type down of min/max/avg (and perhaps more functions) is working properly.

For example test (https://github.com/staabm/phpstan-dba/blob/main/tests/default/data/sql-ast-narrowing.php#L219)

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

phpstan-dba thinks that avg will never be null. But it will in case ada is an empty table (or when there'd be some where condition returning zero rows), right?

mysql> truncate ada;
Query OK, 0 rows affected (0.03 sec)

mysql> SELECT avg(freigabe1u1) as avg from ada;
+------+
| avg  |
+------+
| NULL |
+------+
1 row in set (0.01 sec)

it can return null but phpstan will think that there will never be null.

Is that correct behaviour? Am I missing here something, please?

[staabm]

But it will in case ada is an empty table

this is a fact that I learned today. I think its a pretty rare scenario that tables are empty.. I don't have a idea/opinion yet on this. I am open to discussion :)

most important in the end is, what the actual DB API which is beeing used returns at this point. so even if mysql returns a plain NULL, this might not necessarily mean that you also get that NULL from PDO/mysqli/dibi/..

or when there'd be some where condition returning zero rows

learning/narrowing types from WHERE is a open todo.

also using the correct types when result sets are empty (might be the same as when tables are empty)

[jakubvojacek]

well, the table can either be empty or the where/join condition will make the resulting set empty.

Either way, min/max/avg can be nullable in certain conditions that cannot be determined via AST (or can they?)

I think that the only correct solution is to make the returning type nullable?

[hemberger]

I think that the only correct solution is to make the returning type nullable?

I agree with this. Most aggregate functions (notably: avg, max, min, std, sum, variance) return null if the result set is empty, and so if the result set can be empty, then I think the return type should be nullable.

[staabm]

I agree that at AST level we should leave it null, unless we properly can detect non null result-sets