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.

Sign up for GitHub

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

Jump to bottom

Translation for Nullable<T>.ToString() #33942

Closed
ranma42 opened this issue Jun 9, 2024 · 5 comments
Closed

Translation for Nullable<T>.ToString() #33942

ranma42 opened this issue Jun 9, 2024 · 5 comments
Assignees
@maumar @roji @cincuranet
Labels
area-query breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone
9.0.0-preview7

Comments

@ranma42
Copy link
Contributor

ranma42 commented Jun 9, 2024 •
edited
Loading

What is the intended translation for Nullable<T>.ToString()?

It is currently unclear (at least to me) what is the intended result (and/or translation) because:

  • the C# documentation states that it returns

    The text representation of the value of the current Nullable object if the HasValue property is true, or an empty string ("") if the HasValue property is false.

  • EFCore is attempting to test that the translation matches the C# definition in ToString_boolean_property_nullable, but it does so without ever populating the column with NULL
  • EFCore returns NULL for NULLs in bool? columns in SQL Server and Sqlite
  • EFCore returns the "True" string for NULL in other bool? expressions
    reported as Inconsistent results for Nullable<bool>.ToString() #33941
  • EFCore returns "" for enums
  • EFCore converts (hence propagates NULL) for several types in SQL Server and Sqlite

I realized that the situation was unclear while working on #33940
@roji
Copy link
Member

roji commented Jun 14, 2024

@ranma42 thanks for digging into this, the current situation indeed seems quite problematic.

Without going into specifics, we'd ideally align the translation with the .NET behavior, which as you say is to return an empty string; this would be a small breaking change, but one which I think would be accepted (as the current behavior is simply buggy). I'll bring this up in a design discussion to make sure the team agrees.

@roji roji mentioned this issue Jun 15, 2024
Closed
@ranma42 ranma42 mentioned this issue Jun 17, 2024
Merged
@roji
Copy link
Member

roji commented Jun 17, 2024

Design discussion: we agree that we should align the Nullable ToString() behavior to the .NET one in all cases (empty string); although this is a breaking change, we consider it a minor one, and the current behavior is buggy.

@ranma42 ranma42 mentioned this issue Jun 18, 2024
Merged
@ranma42
Copy link
Contributor Author

ranma42 commented Jul 16, 2024

This should now be fixed in the codebase. Given that it is a breaking change, should it be mentioned in https://github.com/dotnet/EntityFramework.Docs/blob/main/entity-framework/core/what-is-new/ef-core-9.0/breaking-changes.md before closing?

@maumar maumar added this to the 9.0.0 milestone Jul 16, 2024
@maumar maumar mentioned this issue Jul 16, 2024
Open
@maumar
Copy link
Contributor

maumar commented Jul 16, 2024

We can do the documentation note later, I filed an issue to track this - dotnet/EntityFramework.Docs#4765

@maumar
Copy link
Contributor

maumar commented Jul 16, 2024

fixed by @ranma42 in ef5693f and 61bb028

@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 16, 2024
@maumar maumar closed this as completed Jul 16, 2024
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview7 Aug 21, 2024
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
bdfeee3 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
@maumar maumar mentioned this issue Sep 30, 2024
Open
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
0173596 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
7ef8594 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
a70c6ae 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
c41c259 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
637e37d 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
d70bff9 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Sep 30, 2024
@maumar
- Whats new in EF9 docs
f73b7f5 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
maumar added a commit to dotnet/EntityFramework.Docs that referenced this issue Oct 1, 2024
@maumar
- Whats new in EF9 docs
6a7752d 
- Breaking change note for dotnet/efcore#33942
- Update function mappings

Fixes #4765
Fixes #4805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maumar maumar

@roji roji

@cincuranet cincuranet

Labels
area-query breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
Milestone
9.0.0-preview7
Development

No branches or pull requests
5 participants
@ajcvickers @ranma42 @maumar @roji @cincuranet