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

string.StartsWith/EndsWith do not respect StringComparison arg. #2947

Closed
jack128 opened this issue Apr 12, 2021 · 7 comments · Fixed by #2952
Closed

string.StartsWith/EndsWith do not respect StringComparison arg. #2947

jack128 opened this issue Apr 12, 2021 · 7 comments · Fixed by #2952
Assignees
Milestone

Comments

@jack128
Copy link
Contributor

jack128 commented Apr 12, 2021

This bug hits atleast Firebird and Sqlite providers.

		[Test, Combinatorial]
		public void SearchStringTest(
			[IncludeDataSources(TestProvName.AllSQLite, TestProvName.AllFirebird)] string context, 
			[Values(nameof(string.StartsWith), nameof(string.EndsWith))] string methodName,
			[Values]bool ignoreCase)
		{
			var stringComparison = ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
			Expression<Func<Model.Person, bool>> GetPredicate(string param) => methodName switch
			{
				nameof(string.StartsWith) => (Model.Person p) => p.LastName.StartsWith(param, stringComparison),
				nameof(string.EndsWith) => (Model.Person p) => p.LastName.EndsWith(param, stringComparison),				
				_ => throw new ArgumentException("Unknown method name " + methodName)
			};

			using (var db = GetDataContext(context))
			{
				Assert.IsTrue(db.Person.Any(GetPredicate("Pupkin")));
				Assert.AreEqual(ignoreCase, db.Person.Any(GetPredicate("pupkin")));
			}			
		}

Also there is FB specific issue in FirebirdSqlOptimizer.ConvertSearchStringPredicate. "starting with" is case-sensitive, but linq2db use it in "ignorecase" branch

Environment details

linq2db version: 3.3
Database Server: Firebird

@sdanyliv
Copy link
Member

Will prepare fix for that.

@sdanyliv sdanyliv self-assigned this Apr 12, 2021
@jack128
Copy link
Contributor Author

jack128 commented Apr 12, 2021

I believe the bug is here

ExpressionBuilder.SqlBuilder.cs

ISqlPredicate ConvertPredicate(IBuildContext? context, Expression expression)
{
...
    switch (e.Method.Name)
    {
        case "Contains"   : predicate = CreateStringPredicate(context, e, SqlPredicate.SearchString.SearchKind.Contains);   break;
        case "StartsWith" : predicate = CreateStringPredicate(context, e, SqlPredicate.SearchString.SearchKind.StartsWith); break;
	case "EndsWith"   : predicate = CreateStringPredicate(context, e, SqlPredicate.SearchString.SearchKind.EndsWith);   break;
    }
...
}

ISqlPredicate CreateStringPredicate(IBuildContext? context, MethodCallExpression expression, SqlPredicate.SearchString.SearchKind kind)
{
    var e = expression;
    var o = ConvertToSql(context, e.Object);
    var a = ConvertToSql(context, e.Arguments[0]);

    return new SqlPredicate.SearchString(o, false, a, kind, true); // !!!!!!!!!!! true means ignoreCase. linq2db always ignore case
}

@sdanyliv
Copy link
Member

Don't worry I know all places which I have to fix. This is one of them.

@sdanyliv
Copy link
Member

"starting with" is case-sensitive

Looking at documentation, it is not. Where is Truth with Firebird?

@jack128
Copy link
Contributor Author

jack128 commented Apr 12, 2021

https://firebirdsql.org/file/documentation/html/en/refdocs/fblangref25/firebird-25-language-reference.html#fblangref25-commons-predstartwith

The STARTING WITH predicate searches for a string or a string-like type that starts with the characters in its value argument. The search is case-sensitive.

@sdanyliv
Copy link
Member

sdanyliv commented Apr 12, 2021

What is the better way to do non-case sensitive search?

@jack128
Copy link
Contributor Author

jack128 commented Apr 13, 2021

Well, FB doesnt have any special support for non-case sensitive search. So i'd use lower function.
"lower(Field) starting with @p"

@MaceWindu MaceWindu added this to the 3.4.0 milestone May 8, 2021
MaceWindu added a commit that referenced this issue May 16, 2021
* Fix for #2947. Added support for case sensitive search.

* merge

* Fix for #2947. Added support for case sensitive search.

* merge

* Improved query caching support.

* Added constant handling.

* case-insensitive search

* add more tests and fix implementations

* to hell with oracle 12 collations

Co-authored-by: MaceWindu <MaceWindu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants