-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
AST doubts regarding null/not-null #577
Comments
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.
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) |
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? |
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. |
I agree that at AST level we should leave it null, unless we properly can detect non null result-sets |
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)
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?it can return null but phpstan will think that there will never be null.
Is that correct behaviour? Am I missing here something, please?
The text was updated successfully, but these errors were encountered: