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.73k stars 3.18k forks source link

Geometry.Buffer gets evaluated on client side instead of being translated into SQL #23764

Closed noelex closed 2 years ago

noelex commented 3 years ago

I have a table stores geo locations in SQL Server as geography type. I noticed that EF Core evaluates spatial operations if no column is involved. For example, the following code will produce an geometry in euclidean space (apparently not a valid geography) on client side and causes an ArgumentException:

            var radius = 10000;
            var point = new Point(100, 10);

            var result = await dbContext.Set<Entity>()
                .Where(x => point.Buffer(radius).Intersects(x.Location))
                .ToArrayAsync(cancellationToken);
System.ArgumentException: When writing a SQL Server geography value, the shell of a polygon must be oriented counter-clockwise.
         at NetTopologySuite.IO.SqlServerBytesWriter.ToGeography(Geometry geometry)
         at NetTopologySuite.IO.SqlServerBytesWriter.Write(Geometry geometry, Stream stream)
         at NetTopologySuite.IO.SqlServerBytesWriter.Write(Geometry geometry)
         at lambda_method733(Closure , Geometry )
         at Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter`2.<>c__DisplayClass3_0`2.<SanitizeConverter>b__0(Object v)
         at Microsoft.EntityFrameworkCore.Storage.RelationalGeometryTypeMapping`2.CreateParameter(DbCommand command, String name, Object value, Nullable`1 nullable)
         at Microsoft.EntityFrameworkCore.Storage.Internal.TypeMappedRelationalParameter.AddDbParameter(DbCommand command, Object value)
         at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalParameterBase.AddDbParameter(DbCommand command, IReadOnlyDictionary`2 parameterValues)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.CreateDbCommand(RelationalCommandParameterObject parameterObject, Guid commandId, DbCommandMethod commandMethod)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.InitializeReader(DbContext _, Boolean result)
         at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
         at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
         at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)

And the following code works because it can't be evaluted on client side:

            var result = await dbContext.Set<Entity>()
                .Where(x => point.IsWithinDistance(x.Location, radius))
                .ToArrayAsync(cancellationToken);

There won't be any problem if database operates on geometry type which is the same as NTS. But if I need to operate on geograhpy, I have to be extra careful which statement is evaluated on client side and which is not.

It would be nice if EF Core allows me to force all spatial operations to be evaludated on server side.

Version Info

EF Core version: 5.0 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 5.0

ajcvickers commented 3 years ago

Putting this on the backlog to do something specific for spatial types that will evaluate differently in geography/geometry. See also #23819 for a general mechanism to allow this to be changed.

bricelam commented 3 years ago

See also https://github.com/NetTopologySuite/NetTopologySuite/issues/233 & https://github.com/NetTopologySuite/NetTopologySuite.IO.SqlServerBytes/issues/9

bmckilligan commented 3 years ago

I would like to voice my support for some changes to the Spatial data types. The implementation using NetTopologySuite is problematic and trying to use .Net core with spatial types is a step back from the older .Net framework.

The spatial support using Microsoft.SqlServer.Types is rock solid. Has there been any developments towards porting this to .Net core?

bricelam commented 3 years ago

@bmckilligan There have been some community efforts to port it to .NET Core.

You can also use the .NET Framework assembly directly on .NET Core/5 so long as you stay on Windows. All of the spatial logic is implemented in a native library (SqlServerSpatial140.dll), and the P/Invoke's will work fine on .NET Core.

The SQL Server team is aware of the gap and has a plan to address it, but I personally wouldn't (and didn't for EF Core 2.2) hold my breath.

bricelam commented 3 years ago

Note to implementor, use IEvaluatableExpressionFilterPlugin to force translating to SQL

smitpatel commented 2 years ago

I looked at implementing this fix for this issue. While we can manage to avoid parameter extraction to evaluate point.Buffer(radius) on client side, the issue still remains that we don't know which type to use when sending it to server since both the arguments involved are client side parameters. So we don't know if it is geometry or geography. This is something different from UDF (which user explicitly asked us to evaluate on server) or EF.Functions which correlates to server side functions only and doesn't have client implementations. So this is where client implementation doesn't match server side and we cannot do anything because of server having 2 different types too. Hence we cannot fix this. Given there are issues filed on NTS to improve client side implementation this may not require handling on EF core side.

In the meantime, as work-around

bmckilligan commented 2 years ago

.Net Core needs to properly support SQL server geography and geometry data types. As a user trying to use geography data types this is a hot mess.

roji commented 2 years ago

@bmckilligan that's tracked by https://github.com/dotnet/SqlClient/issues/30, and is not within the scope of EF Core, but rather SqlClient.

Having said that, our experience with the NetTopologySuite library has been generally very good, and is probably superior to SqlGeometry/SqlGeography in various ways.

UmairB commented 2 years ago

@bmckilligan that's tracked by dotnet/SqlClient#30, and is not within the scope of EF Core, but rather SqlClient.

Having said that, our experience with the NetTopologySuite library has been generally very good, and is probably superior to SqlGeometry/SqlGeography in various ways.

It might be superior but IMHO the SqlGeometry/SqlGeography data types and functionality is a lot more intuitive. Having to deal with both the sql side of things and the NetTopologySuite is indeed a hot mess :(

roji commented 2 years ago

@UmairB as indicated above, this isn't something over which the EF team has control; please add your vote to the SqlClient issue tracking this.