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

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

Closed
noelex opened this issue Dec 25, 2020 · 10 comments
Closed
Assignees
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported punted-for-6.0

Comments

@noelex
Copy link

noelex commented Dec 25, 2020

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
Copy link
Member

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
Copy link
Contributor

@bmckilligan
Copy link

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
Copy link
Contributor

bricelam commented May 24, 2021

@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
Copy link
Contributor

Note to implementor, use IEvaluatableExpressionFilterPlugin to force translating to SQL

@AndriySvyryd AndriySvyryd modified the milestones: 6.0.0, Backlog Sep 15, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Nov 10, 2021
@smitpatel smitpatel added the easy-for-smit Easy query bugs label May 20, 2022
@smitpatel smitpatel removed this from the 7.0.0 milestone May 23, 2022
@smitpatel smitpatel added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed type-bug easy-for-smit Easy query bugs labels May 23, 2022
@smitpatel
Copy link
Member

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

  • Either try to evaluate point.Buffer(radius) using raw SQL to get result from the server or,
  • Map a UDF function to "STBuffer" on server side where you can assign store type to parameter or result of the function and use that function in the query. We will use the UDF function translation to send it to server.

@bmckilligan
Copy link

.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
Copy link
Member

roji commented May 24, 2022

@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.

@UmairB
Copy link

UmairB commented Sep 6, 2022

@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
Copy link
Member

roji commented Sep 6, 2022

@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.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported punted-for-6.0
Projects
None yet
Development

No branches or pull requests

9 participants