Filtering NVARCHAR column is missing "N" when using string literal

[gdoron]

Hi,

I've a very simple Forum entity:

public class Forum
    {
        public int Id { get; set; }
        public string Name { get; set; }

        public string Description { get; set; }

        public virtual List<Post> Posts{ get; set; }
    }

And nothing extra related to Forum in the OnModelCreating method.
EF correctly generates Name as NVARCHAR(MAX) and filtering with a variable generates the correct SQL:

var var = "דורון";
var scoops = _context.Forums.Single(x => x.Name == var);

exec sp_executesql N'SELECT TOP(2) [x].[Id], [x].[Description], [x].[Name]
FROM [Forums] AS [x]
WHERE [x].[Name] = @__var_0',N'@__var_0 nvarchar(4000)',@__var_0=N'דורון'

But when I use a string literal:

var scoops = _context.Forums.Single(x => x.Name == "דורון");

EF generates the following SQL:

SELECT TOP(2) [x].[Id], [x].[Description], [x].[Name]
FROM [Forums] AS [x]
WHERE [x].[Name] = 'דורון'

Note that the parameter is treated as VARCHAR instead of NVARCHAR

• Using RC1
• The generated SQL was taken from SQL Server profiler.

[gdoron]

@smitpatel by the way, I tried to SQL inject myself with

_context.Forums.Where(x => x.Name == "1' or '1' = '1").ToList();

But EF escapes the ' even if the parameter is a string literal so it didn't work... 😄

[smitpatel]

@gdoron - Its good to know EF stops SQL injection. 😄
The cause of the issue is we do not have type information about the constant used in the query. And string type constant is generated in SQL as is. While it is not easy to infer if the string constant is varchar or nvarchar, we may possibly need another function to specify either of them and use the other as default (that's what we did in EF6 I think).
There is sadly no work-around for it now. The only way is to use the parameter.

[gdoron]

@smitpatel Isn't it possible to always treat the string literal as NVARCHAR even if unicode isn't needed just like as when using a string parameter?
Why should they differ?

[divega]

@smitpatel @gdoron From what I remember besides the EntityFunctions.AsNonUnicode() and EntityFunctions.AsUnicode() methods in EF6 we actually had some logic that tried to figure out the right value for the Unicode facet based on certain common patterns, e.g. if there is an expression of the form SomeNonUnicodeColumn = SomeLiteral then we know that we can render the literal as non Unicode by borrowing the facet from the column.

I think that kind of inference for simple patterns is what we should try to do here. To @gdoron's point I think it is probably a good idea to default to Unicode literals when we can't figure out better.

[mikes-gh]

What problem does the merge for this solve besides the generated sql being more syntactically correct.
From what I read using a non Unicode string in the predicate maintains compatibility between varchar and nvarchar indexes. Adding the N'. By default breaks varchar indexes as sql server cannot do 16bit to 8bit conversion on the predicate and has to upsize all the index from 8 bit to 16 bit.
If we leave the parameter or literal non Unicode it can do 8bit to 16bit conversion of the paramter which in the case of a large index is 1000s of times faster

[gdoron]

@mikes-gh it is not just a syntax error, the query won't yield results in case of non Unicode characters.

[mikes-gh]

@gdoron
Thanks I understand now.
The following issue made it clearer to me
#4781