SQL SELECT not suport Unicode

[hongbangsoft]

Describe the bug

The current SQL parser does not allow "N'" for Unicode

To Reproduce

Run the following SQL Query
SELECT * FROM ContentItemIndex Where ContentType Like N'%á%'

Result
*Syntax error, expected: ., (, +, -, , /, %, &, |, ^, =, >, <, >=, <=, <>, !=, !<, !>, AND, OR, LIKE, NOT LIKE, NOT, BETWEEN, IN, ,, ), GROUP, HAVING, ORDER, LIMIT, OFFSET, UNION, ;, WITH, SELECT, INNER, LEFT, RIGHT, JOIN, WHERE

[hishamco]

This is weird, I will look to this

[lampersky]

@hongbangsoft

"N" prefix on string literals is a SQL Server thing. Not sure if any other db engine is using it.
Sql parser is supporting SQL92 standard with some additional features from SQL2003 standard.

Have you tried to perform query without N prefix? I can search for string with unicode characters with no problems:

[hishamco]

@lampersky I remembered N is required especially if you are using nvarchar datatype

[lampersky]

@hishamco
I don't remember what type is used for ContentType column, but for DisplayText it is nvarchar, that's why my example is working.

[hishamco]

Anyhow many things will be easier to adapt if we use Parlot :)

[hongbangsoft]

@hishamco
Without N prefix the query can run, it work with "á" but it doesn't work with all unicode characters such as "ả".

[hishamco]

I'm sure it won't work with Arabic letter unless you supplied N, we might need to modify the parser to support such thing

[hishamco]

@sebastienros can we modify the SqlGrammar to support such thing, or do you have another thought?

[sebastienros]

I need to understand when SQL Server requires the N prefix first.

[sebastienros]

From what I am reading, a solution might be to inject N for every string literal in the SQLServer dialect.

[hishamco]

I need to understand when SQL Server requires the N prefix first.

This is required one you are using nchar or nvarchar to support unicode characters

From what I am reading, a solution might be to inject N for every string literal in the SQLServer dialect.

I don't think that's enough coz the parser will assume the string literal starts with a single quote, which is wrong in this case. May be adapt the SqlGrammar parser will fix the issue

[sebastienros]

coz the parser will assume the string literal starts with a single quote

My suggestion is to change the SQL generated for SQL SERVER -> The SQL SERVER DIALECT. So SQL SERVER will have to use unicode for all string literals it parses.

[hishamco]

Got it but how the SqlParser in OC will deal with N without changing the grammar?

[sebastienros]

in the SQLServer dialect.

My suggestion is to change the SQL generated for SQL SERVER -> The SQL SERVER DIALECT

Example, these are "SQL SERVER" queries

before: ... WHERE [a] = 'b'
after: ... WHERE [a] = N'b'

There is no change in the grammar or the input SQL, this is "GENERATED SQL SERVER" SQL

[sebastienros]

Note that I am not sure it will work, but this is my suggestion for now.

[hishamco]

It's fine at YeSQL sebastienros/yessql#482 but the problem should occur here

OrchardCore/src/OrchardCore.Modules/OrchardCore.Queries/Sql/SqlGrammar.cs

Line 14 in 6120831

var string_literal = new StringLiteral("string", "'", StringOptions.AllowsDoubledQuote);

[sebastienros]

The parser will parse 'foo', but the "generated" sql query should be N'foo'. We just need to ensure that parsed string literals are rendered using Quote(string) in the dialect, which is the method I changed in the PR.

[hishamco]

That means the SqlGrammar here is thightly coupled on what YesSQL generated, which I didn't expect when I saw the issue

[lahma]

Also note that if queried column is nvarchar, passing varchar parameter (or the other way around) will cause a conversion operator call for each equality check.

https://stackoverflow.com/questions/4459425/sql-server-query-fast-with-literal-but-slow-with-variable

[sebastienros]

@lahma isn't that an orthogonal issue that would already apply today if we are not matching the types (haven't checked)? I believe that what I am suggesting is only to let SQL SERVER parse a query expecting unicode chars in the string literal.

[lahma]

It's generally a problem yes if not accounted for. I've tripped on this one with Quartz years ago which caused indexes/primary keys become less efficient as each comparison required conversion function. This was just a general note.

[sebastienros]

YesSQL generates NVARCHAR columns so this should be fine
https://github.com/sebastienros/yessql/blob/main/src/YesSql.Provider.SqlServer/SqlServerDialect.cs#L36

[hishamco]

Seb the issue is not with the actual type, rather than supporting the parsing of the N in the value, @hongbangsoft correct me if I'm wrong.

[sebastienros]

Seb the issue is not with the actual type
I was responding to marko's comment.

[sebastienros]

@hishamco please try to repro the issue before and after updating yessql. Hopefully the yessql update should fix it. (irrespective of the tests which will need to be updated as you mentioned in your PR)

[hishamco]

I already reproduced the issue before, I might need to test it in the PR #14491

[hishamco]

@sebastienros the issue still exists after updating the YesSQL

[sebastienros]

Thanks, I will check then. Might need another yessql version :/

[sebastienros]

@lampersky 's explanation is correct. There is no issue. The query syntax is db agnostic, the N prefix is a SQL SERVER concept. Don't use that in this query editor.

However the resulting SQL SERVER query that OC generates might be better with using N because we know that YesSQL creates NVARCHAR columns. This is handled by a new version.