Skip to content
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

Add TreatTinyAsBoolean connection string option #141

Closed
bgrainger opened this issue Dec 3, 2016 · 10 comments
Closed

Add TreatTinyAsBoolean connection string option #141

bgrainger opened this issue Dec 3, 2016 · 10 comments
Assignees
Milestone

Comments

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Dec 3, 2016

Implement the TreatTinyAsBoolean connection string option:

  • false - TINYINT(1) is a byte/sbyte
  • true (default) - TINYINT(1) is a bool
@caleblloyd
Copy link
Contributor

@caleblloyd caleblloyd commented Dec 9, 2016

I am about to start work on this, but I think it is worth discussing if TreatTinyAsBoolean=true is a sensible default. In my opinion, changing the data types based off length is unpredictable and therefore is not a sensible default, I would prefer to use TreatTinyAsBoolean=false as the default

I can see how it is confusing if somebody was using BOOL or BOOLEAN as their field types, however. MySql has created an alias of BOOL and BOOLEAN to map to TINYINT(1) (they should have made the alias map to BIT(1), but too late for that). In this case, I understand why somebody would expect TreatTinyAsBoolean=true as the default.

@bgrainger what is your opinion?

@bgrainger
Copy link
Member Author

@bgrainger bgrainger commented Dec 9, 2016

Since MySQL doesn't support BOOL (but just aliases BOOL to TINYINT(1)), any .NET developer would be highly surprised if BOOL columns didn't map to bool by default. We should match Connector/Net here and keep true as the default.

I'm unsure of the value of supporting setting this option to false; when would a TINYINT(1) column be created explicitly in a table schema? The default for TINYINT is TINYINT(3). (Maybe if someone thinks (1) is the number of bytes, not the number of decimal digits to use when formatting the column by default?)

OTOH, if implementing this isn't a huge pain, then no real objections to adding the option; people may be using it in the wild.

@bgrainger
Copy link
Member Author

@bgrainger bgrainger commented Dec 9, 2016

It's not popular, but it is used.

@caleblloyd
Copy link
Contributor

@caleblloyd caleblloyd commented Dec 9, 2016

I'm unsure of the value of supporting setting this option to false; when would a TINYINT(1) column be created explicitly in a table schema?

Sometimes I see TINYINT(1) used as a small status field, like 0=invited, 1=activated, 2=deactivated. Lots of times I see it used as a boolean. But you're right, the number really only applies to zerofill even though there's a common misconception that it represents the number of bytes.

I'll implement it with true as the default.

caleblloyd added a commit to caleblloyd/MySqlConnector that referenced this issue Dec 12, 2016
@bgrainger
Copy link
Member Author

@bgrainger bgrainger commented Dec 13, 2016

Shipped in 0.9.0.

@AccessViolator
Copy link

@AccessViolator AccessViolator commented Jul 17, 2018

@bgrainger
Strangely enough I can't read TINYINT(1) as bool with default setting with Pomelo: link. I closed that issue, as there's a workaround, but it still feels like a bug, though most likely Pomelo's. What do you think?

@bgrainger
Copy link
Member Author

@bgrainger bgrainger commented Jul 17, 2018

@AccessViolator I don't know enough about Pomelo and EF.Core to comment, but I would naively expect System.Boolean in .NET to be mapped to/from TINYINT(1) (aka BOOL) in MySQL.

@a-patel
Copy link

@a-patel a-patel commented Jun 20, 2019

@bgrainger
Any update, I am facing a similar issue using ADO.NET.

@bgrainger
Copy link
Member Author

@bgrainger bgrainger commented Jun 20, 2019

@a-patel This issue is for tracking the implementation of the connection string option, which is done. If you have a bug report or question, please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants