Check parameters for nullable types in EF.Functions extensions

[joakimriedel]

I'm currently spending this week annotating a large project to .NET 6.

In this process I have some code that calls EF.Functions.Like with a property that is now of type nullable string string?. This will produce a warning since this method is annotated to only accept non-nullable string.

See

efcore/src/EFCore/DbFunctionsExtensions.cs

Line 39 in f62cf1b

string matchExpression,

I think it should be string? matchExpression ?

[roji]

Thanks for the feedback. This was discussed a few times in the past - what's your argument for annotating this as nullable?

[joakimriedel]

@roji

In my code I've got something similar to ..

dbQuery = dbQuery.Where(c =>
    EF.Functions.Like(c.Company, likeMatch) ||
    EF.Functions.Like(c.FirstName, likeMatch) ||
    EF.Functions.Like(c.LastName, likeMatch) ||
    EF.Functions.Like(c.Email, likeMatch) ||
    EF.Functions.Like(c.PhoneNumber, likeMatch)
    );

.. to quickly search entities by any key. A contact is defined as

    public class Contact
    {
        public string? FirstName { get; set; }
        public string? LastName { get; set; }
        public string? Company { get; set; }
        public string? Email { get; set; }
        public string? PhoneNumber { get; set; }
    }

where all fields are optional.

On upgrade to .NET 6, I get a squiggly warning in Visual Studio 2022 like this

I would prefer not to have to force not-null them, since they can be null and SQL LIKE supports null in raw query such as

select * from Contacts where FirstName like '%foo%'

[ajcvickers]

@joakimriedel The types used here have been a source of debate over the last few years. There are advantages in constraining them, but also advantages in completely opening them up--see #20962.

[joakimriedel]

@ajcvickers I'm not sure how it is with other providers, but SQL Server is perfectly happy to use an int or datetime2 column as source for LIKE as well since it will be magically converted to character string type.

I would support having multiple overloads such as

public static bool Like(this DbFunctions _, string? matchExpression, string pattern)
public static bool Like(this DbFunctions _, int? matchExpression, string pattern)
public static bool Like(this DbFunctions _, DateTime? matchExpression, string pattern)

but my main concern is that it's currently annotated string and not string? is there any drawbacks by changing that?

[ajcvickers]

@joakimriedel That question results in heated debate on the team. :-) I'm not sure that I fully understand both sides of the argument, so I will refrain from commenting more at this time. The "needs-design" label indicates that we need to discuss this more and come to a conclusion.

[ajcvickers]

We have discussed this and come to the conclusion that we will make the parameters nullable. The arguments against are that the BCL methods for similar kinds of function mapping do not allow nulls, and often for this kind of function it makes sense not to allow nulls from the C# perspective. On the other hand, most SQL functions allow null, and return null if null is passed. To match this, we would need to return a nullable type, but this is quite inconvenient in C# code. However, the functions are often used in translations where the return value is never materialized to a C# value, for example in a WHERE clause. In this case, passing null is useful and the return is handled by SQL. (Although see #26735.)

So...since these are methods only used to translate to SQL, we will make the parameters nullable, but not change the return types to nullable. This applies to Like, and @smitpatel will make a pass to determine which other methods should be changed similarly.

[joakimriedel]

Sounds good, this will make it easier to migrate to net 6 without any null-forgiving. Thanks!