Connection::insert data inference does not use indicated Types

[noemi-salaun]

Connection::insert data inference does not use indicated Types

[staabm]

@hemberger could you look into that one?

[hemberger]

Sure, I'll look into this today.

[hemberger]

Here's my test setup:

CREATE TABLE testing (
    id INT PRIMARY KEY AUTO_INCREMENT,
    my_data JSON NOT NULL,
    my_bool BOOLEAN NOT NULL
);

$db->insert(
    'testing',
    [   
        'my_bool' => true,
        'my_data' => [1, 2, 3], 
    ],  
    [   
        'my_bool' => Types::BOOLEAN,
        'my_data' => Types::JSON,
    ],  
);

And my results:

Query error: Column "testing.my_bool" expects value type int<-128, 127>, got type true
Query error: Column "testing.my_data" expects value type string, got type array{1, 2, 3}

Some things to note:

1. In my query error above, the BOOLEAN column is properly identified as int<-128, 127>, because this pseudo-type is an alias for TINYINT(1) (at least in MySQL). As such, I think it would be prudent to use the bool PHP type for MySQL columns that are identically TINYINT(1). See, e.g. https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-19.html#mysqld-8-0-19-deprecation-removal.

   MySQL Connectors make the assumption that TINYINT(1) columns originated as BOOLEAN columns

2. Even if I use TINYINT(1) UNSIGNED instead of BOOLEAN, I still cannot reproduce the error as you have described it:

   Query error: Column "testing.my_bool" expects value type int, got type true

   Maybe you are using an enum instead of TINYINT(1) UNSIGNED? More details about your setup would be helpful here.

3. DoctrineKeyValueStyleRule is not yet advanced enough to account for the many Doctrine\DBAL\Type classes that may be used to convert an input value to its corresponding database type (e.g. PHP array to JSON string, as in this issue). I'm willing to help implement these, but I'm not sure how to best do this. It seems like we'd need to maintain a mapping between input and output types for each Doctrine\DBAL\Type class.

   We might be able to use reflection on the Type class itself to get the output type (e.g. JsonType::convertToDatabaseValue), but the Type classes, by design, accept any input type. Some input types will cause an exception to be thrown, but others will result in a dangerous type coercion (e.g. casting a non-numeric string to int). This is the reason that this rule does type checks in the first place -- to provide the validation that Doctrine\DBAL does not.

@staabm Assuming you eventually want more complex type converters to be supported, do you have any opinions about how to proceed with this?

[staabm]

Doctrine types have been requested before #278

I think we could just have a separate class which maps the doctrine type constant-types to phpstan types.

Just a guess: Maybe we could get some inspiration from phpstan-doctrine

doctrine relfection could utilize it. I don't think we should plug it directly into SchemaReflection or QueryReflection

We could also add a rule which validates that the used doctrine type is compatible with the underlying schema column type - if this sounds useful for doctrine users.