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

Read DATE as DateOnly value #1451

Open
bgrainger opened this issue Feb 14, 2024 · 6 comments
Open

Read DATE as DateOnly value #1451

bgrainger opened this issue Feb 14, 2024 · 6 comments

Comments

@bgrainger
Copy link
Member

For backwards-compatibility reasons, MySqlDataReader.GetValue will return a DateTime object for DATE columns, even though this could be represented by a .NET DateOnly struct.

For users who want a DateOnly, there are two simple workarounds:

  • MySqlDataReader.GetFieldValue<DateOnly>
  • MySqlDataReader.GetDateOnly

However, there may be cases such as PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#1836 where it would be useful for the "natural" type of the column (retrieved by GetValue) to be DateOnly so no explicit conversion has to take place.

Since there is already a connection string option (AllowZeroDateTime) that causes GetValue to return MySqlDateTime (instead of DateTime), it's possible that a new connection string option could be introduced that switches the behavior of GetValue for DATE columns to return DateOnly. (Perhaps over time this could even become the default behavior and users would have to opt out of the new behavior instead of opting in.)

We could also consider returning TIME columns as a TimeOnly type instead of TimeSpan, but that would only work if the user was deliberately limiting values stored in that column to the range 00:00:00 to 23:59:59.9999999 instead of the full range of the TIME type, which is -838:59:59.000000 to 838:59:59.000000. This seems like it would always need to be an opt-in feature.

See also #963 (comment).

@iamcarbon
Copy link
Contributor

iamcarbon commented Feb 14, 2024

Would it make sense to add a similar option to read JSON columns as byte[] (instead of a string)? This would allow the column to be bound to a JsonObject / JsonElement without additional conversions / allocations.

@bgrainger
Copy link
Member Author

Another existing request for DATE columns to be returned as DateOnly: #1231 (comment)

@bgrainger
Copy link
Member Author

@iamcarbon Created #1454 for this idea. Note that MySqlDataReader.GetStream provides a Stream over the existing in-memory data without allocating a new byte[] (although it does of course allocate a Stream object).

@BogdanYarotsky
Copy link

Hello @bgrainger, I'm looking for a good first issue to contribute. Can I work on this one? Do you have any additional tips regarding the implementation? Should the TIME type also be covered or only DATE for now?

@bgrainger
Copy link
Member Author

Related issue for Npgsql: npgsql/npgsql#5328

@bgrainger
Copy link
Member Author

@BogdanYarotsky #375 is probably the best "first issue", as there are existing examples and the code should be fairly straightforward. (For this particular issue, the design isn't finalised yet, particularly around a connection string option to control it.)

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

No branches or pull requests

3 participants