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

Contains does not understand nullable types #4114

Closed
francoisbeaussier opened this issue Dec 17, 2015 · 4 comments
Closed

Contains does not understand nullable types #4114

francoisbeaussier opened this issue Dec 17, 2015 · 4 comments

Comments

@francoisbeaussier
Copy link

Using 7.0.0-rc1-final

I have the following nullable enum property:

public CategoryTypeEnum? CategoryType { get; set; }

Here is the model builder mapping:

entity.Property(e => e.CategoryType).HasColumnName("CategoryTypeId");

And here is how it's used in the query:

public CategoryType[] CategoryTypes { get; set; }

[...]

if (CategoryTypes != null)
{
    query = query.Where(e => CategoryTypes.Contains(e.CategoryType.Value));
}

During execution, the following is displayed in the output window:

_Microsoft.Data.Entity.Query.Internal.SqlServerQueryCompilationContextFactory: Warning: The LINQ expression '{_CategoryTypes_1 => Contains(Convert([m].CategoryType))}' could not be translated and will be evaluated locally.

And I get an InvalidOperationException "Nullable object must have a value" because it's trying to execute the Where clause in C#.

NOTE1: If I make the property non-nullable (e.g. public CategoryTypeEnum CategoryType { get; set; }), everything works as expected and the sql query looks like:

exec sp_executesql N'SELECT [...] FROM [MyTable] AS [m] WHERE [m].[CategoryTypeId] IN (2, 4) ORDER BY @@ROWCOUNT OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY'

NOTE2: using CategoryTypes.Contains(e.CategoryType.GetValueOrDefault()) does not help

@rowanmiller
Copy link
Contributor

This is somewhat by design.

We currently don't recognize the call to .Value as something that can be translated to SQL (though I opened #4247 as this is an improvement we could make). So, as the warning states, this is evaluated on the client.

Client eval aside, the problem is that your data contains null values for CategoryType and calling .Value on a nullable type is not valid if the property is assigned a null value. And that is the exception you are getting.

It could well be that when we start translating this we are a little lenient in how we enforce the C# semantics and you will get away with the code as is... but really you should change the code to account for the fact that there may be null values.

There are two options here, you could change your array to be of the nullable type and then ditch the .Value call:

public CategoryType?[] CategoryTypes { get; set; }

[...]

if (CategoryTypes != null)
{
    query = query.Where(e => CategoryTypes.Contains(e.CategoryType));
}

Alternatively, you could add a null check to the filter:

query = query.Where(e =>  e.CategoryType.HasValue && categoryTypes.Contains(e.CategoryType.Value));

@francoisbeaussier
Copy link
Author

Hi Rowan,

You captured my point exactly with #4247. In the meantime, I'm currently using a nullable array like you're suggesting :)

Cheers,

Francois

@MaxxDelusional
Copy link

@rowanmiller I tried using your second suggestions. The query still seems to execute locally, even after adding the null check.

long[] modifiedById = new long[] { 123,456 };

var query = db.ChangeLogItems.Where(
    x => x.ChangedById.HasValue
    && modifiedById.Contains(x.ChangedById.Value));

The logger says The LINQ expression '{__modifiedById_0 => Contains(Convert([x].ChangedById))}' could not be translated and will be evaluated locally.

@francoisbeaussier
Copy link
Author

@MaxxDelusional As far as I can tell, @rowanmiller's suggestion to add a null check was simply to prevent the nullReferenceException that may happen when the client execute the .Value locally. He did not mean that the evaluation would happen on the server.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants