search

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

Expand TypeMapper tests to include mysqli/pgsql #573

Closed hemberger closed 1 year ago

hemberger commented 1 year ago

This mirrors the test in pdo-mysql.php, but with the mysqli driver instead of PDO. It tests all the types in the typemix table.

Requested in https://github.com/staabm/phpstan-dba/pull/572#issuecomment-1471467377.

hemberger commented 1 year ago

I'll be honest, I don't really understand why a postgres test is failing here, since the only change is an added mysqli test. :thinking:

staabm commented 1 year ago

I think we missed to update the https://github.com/staabm/phpstan-dba/blob/main/tests/pgsql-schema.sql while adding a new column in https://github.com/staabm/phpstan-dba/blob/main/tests/schema.sql

hemberger commented 1 year ago

Yeah, the typemix table is really different between the two schemas. I spent quite a while trying to make them equivalent, but there are some differences between pgsql and mysql that can't be avoided, for example, pgsql doesn't have an UNSIGNED modifier.

My takeaway is that this particular test case shouldn't be run in the postgres tests. Do you want me to do this, or would you rather I leave it as-is so someone else can investigate?

staabm commented 1 year ago

I think we should run the test but have different expectations depending on the used db backend

hemberger commented 1 year ago

Thanks for the recommendation. This should be ready for review again.

staabm commented 1 year ago

thank you