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

Support numeric conversions in MySqlDataReader #54

Closed
bgrainger opened this issue Sep 14, 2016 · 3 comments
Closed

Support numeric conversions in MySqlDataReader #54

bgrainger opened this issue Sep 14, 2016 · 3 comments
Assignees
Milestone

Comments

@bgrainger
Copy link
Member

PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#52 suggests that other MySQL connectors allow MySqlDataReader.GetInt32 to be called on a decimal column. Investigate this and add missing implicit numeric conversions.

@bgrainger
Copy link
Member Author

MySql.Data allows GetInt32 to be called on a DECIMAL column, and will perform a lossy conversion. However, this implicit conversion isn't consistently available across all numeric data types; calling GetByte (for example) will throw an InvalidCastException for a DECIMAL or INT column, even if the value can be expressed as a byte.

SqlDataReader.GetInt32 documentation states:

No conversions are performed; therefore, the data retrieved must already be a 32-bit signed integer.

Calling SqlDataReader.GetInt32 on a NUMERIC column will throw an InvalidCastException. This seems to better match the documented behaviour on the abstract base class DbDataReader.

I'm not in favour of cloning MySQL's lossy and inconsistent data conversion behaviour by default, but can see that it might be important for people porting from the MySql.Data connector.

The solution might be to add a new connection string parameter Use Lossy Conversions (or similar) that opts into behaviour that matches MySql.Data.

@bgrainger
Copy link
Member Author

Assuming we use a "strict mode" by default, someone who want's MySql.Data's behaviour can get it simply by calling Convert.ToInt32(reader.GetValue(n)) instead of reader.GetInt32(n). This makes it seem like it shouldn't be part of the default behaviour of the library, but opt-in by the caller if they want to pay the price of runtime conversions.

@bgrainger
Copy link
Member Author

One complication is that "The SUM() and AVG() functions return a DECIMAL value for exact-value arguments (integer or DECIMAL)" (http://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html); that is, aggregate functions will promote an int column to decimal.

Users of another DBMS may not be expecting this (for example, SQL Server returns an int when summing an int column), so we should allow an implicit conversion from MySQL's implicitly-promoted decimal back to int.

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

No branches or pull requests

1 participant