prisma / quaint

SQL Query AST and Visitor for Rust
Apache License 2.0
582 stars 62 forks source link

fix: convert tinyint(1) to bool only when val is 0 or 1 #240

Closed Weakky closed 3 years ago

Weakky commented 3 years ago

Fixes https://github.com/prisma/prisma/issues/4981

pimeys commented 3 years ago

What would happen here if the user wants to use the tinyint as tinyint, and wants to store 0 or 1?

Weakky commented 3 years ago

What would happen here if the user wants to use the tinyint as tinyint, and wants to store 0 or 1?

The answer, for now, would be: coerce the value back to an int just like we're doing in the QE.

I don't know the exact reasons why we're converting tinyint(1)s to booleans in the first place, so it'd be helpful to know that to properly formulate an answer.

That being said, in the comment shared above, it says:

For example, the Connector/J (Java connector) treats tinyint(1) as a boolean value, and instead of returning a numerical result to the application, it converts values to true and false. This can be changed via the tinyInt1isBit=false connection parameter.

Leading me to think that, if we want to keep that conversion, one way would be to have some kind of configuration for that.

This fix doesn't make anything worst than it was though. This edge-case can be reviewed in the future if needed.

pimeys commented 3 years ago

Now, to the next try, I'd say better would be to go and edit

    fn is_bool(&self) -> bool {
        (self.column_type() == ColumnType::MYSQL_TYPE_TINY && self.column_length() == 1)
            || (self.column_type() == ColumnType::MYSQL_TYPE_BIT && self.column_length() == 1)
    }

Now, we could maybe continue converting BIT(1) still to a boolean. Therefore you'd change the test to use BIT(1), but you'd remove the first check here and add a few more tests to see that TINYINT maps to an from an integer.