dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.77k stars 3.19k forks source link

Wrong query generation when using Contains inside where predicate. #6687

Closed gmartinezsan closed 2 years ago

gmartinezsan commented 8 years ago

Visual Studio 2015 update 3 efcore : V1.0.0 MySQL database

A query with a where predicate that uses string.contains inside of the where . Example:

var result = context.Employees.Where(t => t.LastName.Contains("jo")).ToList();

The like expression generated in DefaultQuerySqlGenerator.cs creates a misplaced parenthesis:

{SELECT `t`.`EmployeeId`, `t`.`DisplayName`, `t`.`FirstName`, `t`.`LastName`, `t`.`Timestamp`
FROM `Employees` AS `t`
WHERE `t`.`LastName` LIKE ('%' + 'jo') + '%'}

Which causes an SQL exception:

  MySql.Data.MySqlClient.MySqlException occurred
  Code=0
  HResult=-2146233088
  Message=You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '+ '%'' at line 3
  Number=1064
  Source=MySql.Data
  StackTrace:
at MySql.Data.MySqlClient.MySqlStream.ReadPacket() in 
 Source\MySql.Data\MySqlStream.cs:line 165

EF Core version: 1.0

Can you please have a look into this?

divega commented 8 years ago

@gmartinezsan curious, does MySQL generally not support this usage of parenthesis in concatenation expressions? Or is there something else that makes this SQL not valid?

divega commented 8 years ago

Somewhat related to this, there are improvements in 1.1 for the translation of String.Contains() but I believe your provider will have to make changes to take advantage. @maumar should be able to help.

maumar commented 8 years ago

@gmartinezsan the fix Diego is referring to is here: https://github.com/aspnet/EntityFramework/commit/5e1c562ae40c16ef49b90731fd015eccce0c1d44

We only did it for SqlServer and Sqlite, so indeed MySql provider would have to add their own MethodCallTranslators (probably quite similar to the Sqlite one - SqliteContainsOptimizedTranslator).

Note that the commit I linked also fixes bug in RelationalCompositeMethodCallTranslator which allows using provider specific translators rather than the default ones. Without this change even if MySql provider adds appropriate translators, they won't be used.

gmartinezsan commented 8 years ago

hi @maumar , any idea when these fixes will be published? IIUC, even if we have done the implementation of the translator optimization for MySQL we would need to wait for these fixes to be published. Is this correct? Thanks,

divega commented 8 years ago

@gmartinezsan those are 1.1 changes. Depending on the answer to my first question on the usage of parenthesis in concatenation we may have a more general issue and the possibility to fix it for 1.0 as well, either on a patch release of EF Core or in the MySQL provider.

gmartinezsan commented 8 years ago

@divega MySQL string concatenation should use the CONCAT function. Unfortunately '+' operator is not supported.

maumar commented 8 years ago

@gmartinezsan we have some extensibility points for this kinds of scenarios. You can override DefaultQuerySqlGenerator.TryGenerateBinaryOperator and generate a custom SQL for string concatenation.

gmartinezsan commented 8 years ago

@divega @maumar will do. Thanks a lot for the help!

gmartinezsan commented 8 years ago

We added the MethodCallTranslators implementation based on the approach used in Sqlite, because changing the concat operator was not enough to cover this case. Any idea when 1.1 will be available? Thanks!

rowanmiller commented 8 years ago

@gmartinezsan I'll email you about dates

rowanmiller commented 8 years ago

@gmartinezsan you are unblocked now right?

@maumar will investigate why the extension point in default sql generator didn't work (but not immediately).

gmartinezsan commented 8 years ago

@rowanmiller hi, yes we have solved this issue, and tested it with latest EF Core version (1.1) and it works fine, but only with 1.1. Thanks for the follow up.

maumar commented 8 years ago

The problem is that out VisitBinary operator is hard-coded to a form [leftOperand] [operator] [rightOperand], so it doesn't allow modifying the structure, just what the operator would be. We have StringConcatTranslator that would normally handle this (convert Add binary operator to a method call) but because we already ran a method call translator (translating Contains) which produced the string concat, the second translator (StringConcatTranslator) doesn't run. This happens because we stop running the translators on a given expression once the first one successfully processes that expression. So the correct way to address this is to make all the necessary modifications in the Contains translator that is going to run first, i.e. what @gmartinezsan did.

gmartinezsan commented 7 years ago

@shahafal Still hitting this issue?

shahafal commented 7 years ago

@gmartinezsan Hi gabriela,

1) Yes I'm still having this same issue with version 7.0.4-IR-191 of MySql.Data.EntityFrameworkCore (and Entity Framework core version 1.0), so when I'm using .Contains() method I'm getting a bad sql query like the one in your first post: WHERE Content LIKE ('%' + @__content_0) + '%'.

2) I saw that in version 7.0.6-IR31 you published this issue is fixed when combined with entity framework core 1.1, but when upgrading entity framework core from version 1.0 to 1.1 and mysql connector from 7.0.4 to 7.0.6 I've encountered another problem which I wrote about in https://github.com/aspnet/EntityFramework/issues/7376 In my issue I was reffered to another issue (https://github.com/aspnet/EntityFramework/issues/7223), where the guy's solution was to switch to Pomelo connector (I contacted him personally for his answer).

3) I saw that version 6.10.0-alpha is the newest one and works with entity framework core 1.1, so I tried it and it did work with ef1.1 (as opposed to version 7.0.6-IR31 that threw errors) however the .Contains() problem discussed in this issue still occurs there. these are the configurations I tried for version 6.10.0-alpha:

"Microsoft.EntityFrameworkCore": "1.1.0",
"Microsoft.EntityFrameworkCore.Relational": "1.1.0",
"MySql.Data": "6.10.0-alpha",
"MySql.Data.EntityFrameworkCore": "6.10.0-alpha",

What can I do without trying a different mysql connector?

gmartinezsan commented 7 years ago

@shahafal Thanks for the feedback. 6.10.0 alpha should be enough for this case. I personally tried it today and it is working with EF Core 1.1 and 6.10.0.

Here's the linq expression I'm using:

context.Employees.Where(t => t.FirstName.Contains("jo")).ToList();

Could you please share the code you are using?

Thanks!

shahafal commented 7 years ago

@gmartinezsan this is my project.json:

{
  "version": "1.0.0-*",
  "buildOptions": {
    "debugType": "portable",
    "preserveCompilationContext": true,
    "emitEntryPoint": true
  },
  "dependencies": {},
  "frameworks": {
    "netcoreapp1.1": {
      "dependencies": {
        "Microsoft.NETCore.App": {
          "type": "platform",
          "version": "1.1.0"
        },
        "Microsoft.AspNetCore.Server.Kestrel": "1.1.0",
        "Microsoft.AspNetCore.Mvc": "1.1.0",
        "Microsoft.EntityFrameworkCore": "1.1.0",
        "Microsoft.EntityFrameworkCore.Relational": "1.1.0",
        "MySql.Data": "6.10.0-alpha",
        "MySql.Data.EntityFrameworkCore": "6.10.0-alpha",
        "Microsoft.Extensions.Configuration.FileExtensions": "1.1.0",
        "Microsoft.Extensions.Configuration.Json": "1.1.0",
        "Microsoft.AspNetCore.StaticFiles": "1.1.0",
        "System.Security.Cryptography.Algorithms": "4.3.0",
        "System.Text.Encoding": "4.3.0",
        "Microsoft.AspNetCore.Diagnostics": "1.1.0",
        "Microsoft.Extensions.Logging.Console": "1.1.0"
      },
      "imports": "dnxcore50"
    }
  }
}

this is my code:

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore;
...
...
        public IEnumerable<Message> searchMessagesByContent(string content)
        {
            var messages = this.appDbContext.Messages.Where(x => x.Content.Contains(content)).ToList();
            ...
        }

and the error:

info: Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommandBuilderFactory[1]
      Executed DbCommand (272ms) [Parameters=[@__content_0='?' (Size = 4)], CommandType='Text', CommandTimeout='30']
      SELECT `x`.`MessageId`, `x`.`Content`
      FROM `Messages` AS `x`
      WHERE `x`.`Content` LIKE ('%' + @__content_0) + '%'
Loaded '/Users/myuser/.nuget/packages/System.Diagnostics.StackTrace/4.3.0/lib/netstandard1.3/System.Diagnostics.StackTrace.dll'. Cannot find or open the symbol file.
fail: Microsoft.EntityFrameworkCore.Query.RelationalQueryCompilationContextFactory[1]
      An exception occurred in the database while iterating the results of a query.
      MySql.Data.MySqlClient.MySqlException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '+ '%'' at line 3
         at MySql.Data.MySqlClient.MySqlStream.ReadPacket()
         at MySql.Data.MySqlClient.NativeDriver.GetResult(Int32& affectedRow, Int64& insertedId)
         at MySql.Data.MySqlClient.Driver.GetResult(Int32 statementId, Int32& affectedRows, Int64& insertedId)
         at MySql.Data.MySqlClient.Driver.NextResult(Int32 statementId, Boolean force)
         at MySql.Data.MySqlClient.MySqlDataReader.NextResult()
         at MySql.Data.MySqlClient.MySqlCommand.ExecuteReader(CommandBehavior behavior)
         at MySql.Data.MySqlClient.MySqlCommand.ExecuteDbDataReader(CommandBehavior behavior)
         at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, String executeMethod, IReadOnlyDictionary`2 parameterValues, Boolean openConnection, Boolean closeConnection)
         at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues, Boolean manageConnection)
         at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable.Enumerator.MoveNext()
         at Microsoft.EntityFrameworkCore.Query.QueryMethodProvider.<_ShapedQuery>d__3`1.MoveNext()
         at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__15`2.MoveNext()
         at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
MySql.Data.MySqlClient.MySqlException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '+ '%'' at line 3

I wonder if perhaps the reason your code passed and mine failed is due to cached nuget packages? (like the issue in upgrading entity framework version from v1.0 to v1.1 where the package "Microsoft.EntityFrameworkCore.Relational" is needed to be added to project json with the new 1.1 version otherwise its v1.0 is being used and an error is thrown)

My project.json file has had upgrades for entity framework from v1.0 to v.1.1 and for mysql connector from v7.0.4 to v7.0.6 to v.6.10.0 in the following packages:

MySql.Data.EntityFrameworkCore
MySql.Data
MySql.Data.Core

Is there a chance that one of them (the entity framework or mysql connector) refers to an older package? Because in such case maybe adding the right dependency package in the right version to project.json would solve this.

Is there a package you can think of that I can try and add to my project.json with the correct version so the app won't use its cached version in hopes that it would solve the error with Contains()?

gmartinezsan commented 7 years ago

@shahafal just tried using a variable instead of a constant or hard coded value as I was doing and only the last one worked. I added a bug in our community site so you can follow up the fix. http://bugs.mysql.com/bug.php?id=84505 Thank you for your feedback.

maumar commented 7 years ago

@gmartinezsan could you provide the entire stack trace of the issue that you were experiencing? This particular scenario seems to be working on Sql provider, but I wonder if I can force the issue here as well if I modify a query somewhat.

gmartinezsan commented 7 years ago

@maumar Sure, Here is the call stack

System.InvalidOperationException was unhandled by user code HResult=-2146233079 Message=When called from 'VisitChildren', rewriting a node of type 'System.Linq.Expressions.Expression' must return a non-null value of the same type. Alternatively, override 'VisitChildren' and change it to not visit children of this type. Source=System.Linq.Expressions StackTrace: at System.Linq.Expressions.ExpressionVisitor.VisitAndConvert[T](ReadOnlyCollection1 nodes, String callerName) at Microsoft.EntityFrameworkCore.Query.Expressions.SqlFunctionExpression.VisitChildren(ExpressionVisitor visitor) at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.VisitExtension(Expression node) at Microsoft.EntityFrameworkCore.Query.Expressions.SqlFunctionExpression.Accept(ExpressionVisitor visitor) at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ConditionalRemovingExpressionVisitor.Visit(Expression node) at Microsoft.EntityFrameworkCore.Query.Expressions.LikeExpression.VisitChildren(ExpressionVisitor visitor) at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.VisitExtension(Expression node) at Microsoft.EntityFrameworkCore.Query.Expressions.LikeExpression.Accept(ExpressionVisitor visitor) at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ConditionalRemovingExpressionVisitor.Visit(Expression node) at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitWhereClause(WhereClause whereClause, QueryModel queryModel, Int32 index) at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection1 bodyClauses, QueryModel queryModel) at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel) at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel) at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel) at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel) --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, INodeTypeProvider nodeTypeProvider, IDatabase database, ILogger logger, Type contextType) at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_01.<CompileQuery>b__0() at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func1 compiler) at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) at Remotion.Linq.QueryableBase1.GetEnumerator() at System.Collections.Generic.List1..ctor(IEnumerable1 collection) at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source) at MySql.Data.EntityFrameworkCore.Tests.FluentAPITests.CanUseContainsInQuery() in Tests\MySql.EntityFrameworkCore.Basic.Tests\FluentAPITests.cs:line 265

shahafal commented 7 years ago

@gmartinezsan in my previous message I was still having this error even after upgrading to mysql v6.10.0-alpha: MySql.Data.MySqlClient.MySqlException: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '+ '%'' at line 3 I managed to get past the error - The problem was - I upgraded dotnet core version from 1.0 to 1.1 so it works with entity framework core 1.1, but in Launch.json file I was still referencing the app's dll of version 1.0.

The fix was to change this line:

"program": "${workspaceRoot}/bin/Debug/netcoreapp1.0/MyApp.dll",

to this:

"program": "${workspaceRoot}/bin/Debug/netcoreapp1.1/MyApp.dll",

After fixing this problem, I am now having the error you wrote in your last message regarding passing parameter instead of plaintext into .Contains().

I opened a post about it in stackoverflow thinking I might miss something in the way .Contains() work: http://stackoverflow.com/questions/41637714/use-parameter-from-method-argument-inside-linq-contains but now that I see you encountered this too and reported it as a bug, I'm looking forward to hearing how to overcome this. Will you write the solution in the url you posted to mysql forum or here?

@maumar in case this helps, here is my code:

public IEnumerable<Message> search(string content) {
    var messages = this.appDbContext.Messages.Where(x => x.Content.Contains(content)).ToList();
    ...

and my stacktrace:

fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[0]
      An unhandled exception has occurred while executing the request
System.InvalidOperationException: When called from 'VisitChildren', rewriting a node of type 'System.Linq.Expressions.Expression' must return a non-null value of the same type. Alternatively, override 'VisitChildren' and change it to not visit children of this type.
   at System.Linq.Expressions.ExpressionVisitor.VisitAndConvert[T](ReadOnlyCollection`1 nodes, String callerName)
   at Microsoft.EntityFrameworkCore.Query.Expressions.SqlFunctionExpression.VisitChildren(ExpressionVisitor visitor)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.VisitExtension(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Expressions.SqlFunctionExpression.Accept(ExpressionVisitor visitor)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ConditionalRemovingExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Expressions.LikeExpression.VisitChildren(ExpressionVisitor visitor)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.VisitExtension(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Expressions.LikeExpression.Accept(ExpressionVisitor visitor)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ConditionalRemovingExpressionVisitor.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at System.Linq.Expressions.BinaryExpression.Accept(ExpressionVisitor visitor)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.ConditionalRemovingExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitWhereClause(WhereClause whereClause, QueryModel queryModel, Int32 index)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
--- End of stack trace from previous location where exception was thrown ---