Failing AST test with non-named column #570
Conversation
|
to implement this I think we need SQLFTW/sqlftw@da4e035 which will be part of the next sqlftw release, since it looks pretty similar to what I had in mind when reporting SQLFTW/sqlftw#15 |
|
perfect, cant wait for this |
|
Today a new sqlftw was released. You can give it a try and update the lob with this PR and add a fix for you problem if you like |
|
I will give it a go over the weekend! Thanks |
…into failing-ast-column-recognition
|
The fix for the correct type inference was easy, but tests are failing @staabm We got some problems with postgres tests, since AST does not work there, it fails (https://github.com/staabm/phpstan-dba/actions/runs/4454918527/jobs/7824413966?pr=570) For the same reason, it also fails for php 7.3 and lower Any idea how to overcome this, please? |
…to failing-ast-column-recognition
|
I think we should move ast tests, e.g. phpstan-dba/tests/default/DbaInferenceTest.php Lines 66 to 68 in a1bb0c6 into a separate test-class and enable the runtime-config for SQL AST only for this new test-class make sure to re-base this PR as in parallel another PR was merged |
|
Is this what you had in mind please? |
There was a problem hiding this comment.
I don't think we need a whole new defaultAst suite, but just a new SqlAstInferenceTest.php within the existing default test-suite, which enables sql-ast like on setup/teardown, like we did for paramter validation in bb2b845#diff-428d052928087e660593dfd032cc0445056ba9fe2780ddb2292983fefe04ac87R18-R26
| @@ -13,6 +13,6 @@ public function count(PDO $pdo): void | |||
| assertType('PDOStatement<array{myemail: numeric-string|null, 0: numeric-string|null}>', $stmt); | |||
|
|
|||
| $stmt = $pdo->query('SELECT count(email) as myemail, count(email) from ada'); | |||
| assertType('PDOStatement<array{myemail: numeric-string|null, 0: numeric-string|null, count(email): numeric-string, 1: numeric-string|null}>', $stmt); | |||
| assertType('PDOStatement<array{myemail: numeric-string|null, 0: numeric-string|null, count(email): numeric-string|null, 1: numeric-string|null}>', $stmt); | |||
There was a problem hiding this comment.
I think this test no longer runs with SQL AST enabled
Hello @staabm
this is related to #566 (comment), this is the reproducing case. When
count(*)(or other functions) is used without alias, it fails to narrow down the type for integer indexed key in the resulting PDOStatement.I tried playing around with ParserInference but couldn't find a easy way to fix this yet