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

Treat ToString as casting to string in the database #20604

Closed
fingers10 opened this issue Apr 11, 2020 · 10 comments
Closed

Treat ToString as casting to string in the database #20604

fingers10 opened this issue Apr 11, 2020 · 10 comments
Labels
area-query 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

Comments

@fingers10
Copy link

I'm trying to do a contains search on enum property in my DbSet and EF Core 3.1 throws the below error

The LINQ expression 'DbSet .Where(d =>
d.Position.ToString().Contains("acc"))' could not be translated.
Either rewrite the query in a form that can be translated, or switch
to client evaluation explicitly by inserting a call to either
AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync()

Entity:

public class DemoEntity
{
    [Key]
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public Position Position { get; set; }
}

Enum - Position:

public enum Position
{
    [Display(Name = "Accountant")]
    Accountant,
    [Display(Name = "Chief Executive Officer (CEO)")]
    ChiefExecutiveOfficer,
    [Display(Name = "Integration Specialist")]
    IntegrationSpecialist,
    [Display(Name = "Junior Technical Author")]
    JuniorTechnicalAuthor,
    [Display(Name = "Pre Sales Support")]
    PreSalesSupport,
    [Display(Name = "Sales Assistant")]
    SalesAssistant,
    [Display(Name = "Senior Javascript Developer")]
    SeniorJavascriptDeveloper,
    [Display(Name = "Software Engineer")]
    SoftwareEngineer
}

DbContext:

public class DemoDbContext : DbContext
{
    public DemoDbContext(DbContextOptions options)
        : base(options) { }

    public DbSet<DemoEntity> Demos { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder
            .Entity<DemoEntity>()
            .Property(e => e.Position)
            .HasConversion<string>();
    }
}

When I query the table as follows I'm getting the error

try
{
    var test = await _context.Demos.Where(x => x.Position.ToString().Contains("acc")).ToListAsync();
}
catch (System.Exception e)
{
    //throw;
}

The Position is of type NVARCHAR(MAX) in my database.

enter image description here

enter image description here

This is not possible? If so please can you help me with explanation?

@ajcvickers ajcvickers changed the title EF Core 3.1 does not allow Contains search on Enum property Treat ToString as casting to string in the database Apr 14, 2020
@ajcvickers
Copy link
Member

@fingers10 This should work:

.Where(x => ((string)(object)x.Position).Contains("acc"))

EF Core then interprets this as a cast to string and generates:

SELECT [d].[Id], [d].[FirstName], [d].[LastName], [d].[Position]
FROM [DemoEntity] AS [d]
WHERE CHARINDEX(N'acc', CAST([d].[Position] AS nvarchar(max))) > 0

However, we discussed that it would be useful to allow ToString to do the same thing--it would avoid the non-obvious trick of casting to object. There is some concern that the output of ToString on on the client could be very different to what is generated on the database, but given the weak semantics of ToString anyway, that's probably okay.

@ajcvickers ajcvickers added this to the Backlog milestone Apr 14, 2020
@smitpatel
Copy link
Contributor

Also see #14205
Convert a different ToString represent can give incorrect results in query.

@fingers10
Copy link
Author

@ajcvickers Thanks for the workaround. It works good and expected.

@Danevandy99
Copy link
Contributor

I'm attempting to fix this issue. My goal right now is to have the p.EnumProperty.ToString().Contains("value") translate to a similar query to ((string)(object)p.EnumProperty).Contains("value") when the database type is a string, but build a case expression for database types that are not strings. The SQL for the second case (where the enum is stored as a number), would look something like this with Sqlite:

SELECT "p"."PostId", "p"."BlogId", "p"."Content", "p"."Title", "p"."Visibility"
FROM "Posts" AS "p"
WHERE 'pr' = '' OR instr(CASE
    WHEN "p"."Visibility" = 0 THEN 'Public'
    WHEN "p"."Visibility" = 1 THEN 'Private'
    ELSE NULL
END, 'pr') > 0
LIMIT 1

@roji
Copy link
Member

roji commented Apr 28, 2024

@Danevandy99 please separate general translation of ToString() to a database-side cast (CAST(x AS nvarchar(max)) - which is what's tracked by this issue and what's already possible via (string)(object) - from any sort of enum-specific string conversion such as the CASE/WHEN you propose above. The latter is orthogonal to the former and requires careful thought (if you'd like to pursue this, please open a separate issue).

@Danevandy99
Copy link
Contributor

@roji Sounds good! @ajcvickers mentioned this in his comment about the casting workaround above about how translating .Tostring() to do a database-side cast could cause unexpected results when compared to a client-side evalutation, but I can see how the CASE/WHEN conversion will require more thought and changes, so I'll make a separate issue for that.

@roji
Copy link
Member

roji commented May 14, 2024

@Danevandy99 we had a thorough design discussion in the team about this, and came to the following conclusions:

  • When ToString() is applied to an enum with the default map-to-int behavior (no value converter), we think it's a good idea to use CASE/WHEN to make the database return the .NET enum label. This is what we already do for ToString() over bool, and having ToString() return the number values as strings seems like it wouldn't be useful behavior for anyone. This is #33635.
  • When a value converter is configured on the enum property, we should check whether it's our own EnumToStringConverter; if it is, we know what it does, and can simply return the string value from the database (we know it corresponds to the .NET enum values).
    • This intersects with #10434, and note that we don't currently do translation based on value converter identification. But whatever we do in #10434, it seems very unlikely that implementing the behavior described above would be bad etc.
  • If the enum property has another value converter, we have no idea what it does, and can't provide a sensible translation. We should refrain from translating in that case (and the user will get an exception, just like today).

Does that sound OK @Danevandy99? If so, you can merge #33701 and #33706 together (close one and integrate its contents into the other - sorry for the churn), update as per the above, and we'll review.

@Danevandy99
Copy link
Contributor

@roji That sounds great! I like the default case refraining from translating if there is no value converter or a value converter other than the EnumToStringConverter. I'll merge those PR's together and make the changes to line up with the behavior you outlined above.

@roji
Copy link
Member

roji commented May 15, 2024

I like the default case refraining from translating if there is no value converter or a value converter other than the EnumToStringConverter.

To be clear, when there's no value converter, EF maps the enum to an integer in the database - for that case we'd do the CASE/WHEN. The only case where we should refrain from translating is when a value converter is configured, but it's not the built-in EnumToStringConverter.

@Danevandy99
Copy link
Contributor

Yes, I stated that incorrectly, but I think we are on the same page. I should've finished my coffee before replying 😆

Danevandy99 added a commit to Danevandy99/efcore that referenced this issue May 27, 2024
Danevandy99 added a commit to Danevandy99/efcore that referenced this issue Jun 3, 2024
@roji roji closed this as completed in 80e9dce Jun 4, 2024
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution labels Jun 5, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0-preview6 Jun 5, 2024
@roji roji modified the milestones: 9.0.0-preview6, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query 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
Development

No branches or pull requests

5 participants