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.66k stars 3.15k forks source link

NetTopologySuite SqlServer provider incorrectly translates Within query #15685

Closed DanielStout5 closed 1 year ago

DanielStout5 commented 5 years ago

I have this query:

var points = new[]
{
    new Coordinate(neLng.Value, neLat.Value), // TR
    new Coordinate(neLng.Value, swLat.Value), // BR
    new Coordinate(swLng.Value, swLat.Value), // BL
    new Coordinate(swLng.Value, neLat.Value), // TL
    new Coordinate(neLng.Value, neLat.Value), // TR
};

var fact = NtsGeometryServices.Instance.CreateGeometryFactory(srid: 4326);
var poly = fact.CreatePolygon(points).Normalized().Reverse();

// This line is only added to demonstrate that local evaluation works correctly
var list = query.ToList().Where(x => x.Location != null && x.Location.Within(poly)).ToList();

// This does not filter to the correct results; it excludes values that should be included, and includes values that should be excluded
query = query.Where(x => x.Location.Within(poly));

The server-evaluated query is incorrect - it omits points within the polygon poly, and includes ones that are outside it. The locally-evaluated version ("bug demo") does return the expected results. They should be returning the same results since they are doing the same thing. query is IQueryable.

The values received for the bounds will be like this:

neLat   60.11131083608817
neLng   -74.22879847513246
swLat   47.2231665957533
swLng   -157.85672816263252

Further technical details

EF Core version: 2.2.3 Database Provider: Microsoft.EntityFrameworkCore.SqlServer

ajcvickers commented 5 years ago

Note for triage: looks like this could be SQL Server polygon direction again. /cc @bricelam

DanielStout5 commented 5 years ago

Not sure this is relevant, but I did try:

As well as the different combinations of the clockwise order/normalize/reverse, and it would either immediately throw an exception (saying they were declared in the wrong order) or work as it does now.

bricelam commented 5 years ago

What is query? Can you attach a standalone repro with data that demonstrates the issue?

I also remember MSSQL having some unorthodox behaviors for things landing exactly on a boundary. Could it be that?

bricelam commented 5 years ago

Could be related to #15668, but it seems unlikely

DanielStout5 commented 5 years ago

Query is IQueryable<Business> where Business has a NetTopologySuite.Geometries.Point property called Location.

I'll see if I can get the time to setup a repo that replicates the issue - I've already spent quite a few hours trying to figure out what was going on here, so that may have to wait a bit.

The locations are not exactly on the boundary - they are many kilometers away.

15668 doesn't look related to me but I'm no expert!

JediLeeSkywalker commented 5 years ago

Hello, I am one of colleague whose the reporter #15668:

Some experience of mine would help :

  1. Type declare of the Location column in the SQLContext directs to be a "geometry"or "geography", if there is none ,the default will be a "geography". it looks like : entity.Property(e => e.Location).HasColumnType("geometry");
  2. Detect counter clockwise then we can decide to reverse the polygon or not.
  3. Or you could try to use Location.intersects(poly)?
  4. Post some translated SQL or error message that are from [ASP.NET Core Web Server] window at bottom of Visual Studio when running the web app, you might get more help.
DanielStout5 commented 5 years ago

Type declare of the Location column in the SQLContext directs to be a "geometry"or "geography", if there is none ,the default will be a "geography"

The Location is of type geography. I thought that was correct, since I'm dealing with latitude/longitude and not arbitrary shapes?

Detect counter clockwise then we can decide to reverse the polygon or not.

I'm not sure that's the issue, since I tried both clockwise and counter-clockwise. One way would always cause an exception, and I have it the other way. It doesn't cause an exception how I have it now, but it also doesn't return the results.

Or you could try to use Location.intersects(poly)?

I've tried Intersects as well. It works very similarly to Within - maybe slightly better, but it's still not returning the correct results.

Just like Within, it works perfectly if I execute Location.Within(poly) locally instead of on SQL Server.

I can move the map to include most of the correct pins:

Before Map

But if I move the map up slightly, most of the pins disappear - even though they are still obviously within the polygon formed by the bounds of the map (as evidenced by the fact that Location.Within(poly) works when executed locally).

After Map

Post some translated SQL or error message that are from [ASP.NET Core Web Server] window at bottom of Visual Studio when running the web app, you might get more help.

There are no errors, but here's the SQL generated:

Microsoft.EntityFrameworkCore.Database.Command:Information: Executed DbCommand (1ms) [Parameters=[@__poly_0='0xE61000000104050000002FF82028D19B45401036CB24C1D550C02FF82028D19B...' (Size = 112) (DbType = Binary), @__p_1='0', @__p_2='50'], CommandType='Text', CommandTimeout='30']
SELECT [x].[Id], [... omitted]
FROM [Businesses] AS [x]
WHERE ([x].[Type] = N'Shop') AND ([x].[Location].STIntersects(@__poly_0) = 1)
ORDER BY (SELECT 1)
OFFSET @__p_1 ROWS FETCH NEXT @__p_2 ROWS ONLY
JediLeeSkywalker commented 5 years ago

Hello:   Is it equal to original points array? if you try to test the binary parameter of translated SQL in SSMS Query Analyzer : DECLARE @g geography; SET @g = geography::STGeomFromWKB(0xE61000000104050000002...<--[the complete binary code],4326); SELECT @g.ToString(); ref STGeomFromWKB

Is yours google maps? Maybe this one Pin disappear ? closest date

By the way, I am using EF core 2.2.4 ,NetTopologySuite.core 1.15.2

DanielStout5 commented 5 years ago

Is it equal to original points array? if you try to test the binary parameter of translated SQL in SSMS Query Analyzer

I actually did want to test this, but to be honest I'm not sure how to get the full binary representation of the polygon since the SensitiveDataLogging for EF core only includes the first bit.

I've tried BitConverter.ToString(poly.AsBinary()).Replace("-", ""); and WKBWriter.ToHex(poly.AsBinary()); but they don't match what EF Core is logging. Any idea how to get the full binary data?

EF:    0xE6100000010405000000C174FC786E494640E87E886BD7F44FC0C174FC786E49...
Other: 01030000000100000005000000E87E886BD7F44FC0C174FC786E494640E87E88AB36A14FC0C174FC786E494640E87E88AB36A14FC0EE55BEB0AD5A4640E87E886BD7F44FC0EE55BEB0AD5A4640E87E886BD7F44FC0C174FC786E494640

Is yours google maps? Maybe this one Pin disappear ? closest date

It is Google maps, but I'm quite certain that isn't the issue, or else it wouldn't work when I change from SQL server evaluation to local. The only thing that changes is whether Location.Intersects(poly) executes in C# or on SQL Server. It works when executed in C#, but not when executed by SQL Server.

By the way, I am using EF core 2.2.4 ,NetTopologySuite.core 1.15.2

I'm using Microsoft.EntityFrameworkCore.SqlServer.NetTopologySuite version 2.2.3, and my version of .Net Core is 2.2.103

JediLeeSkywalker commented 5 years ago
  1. I don't know VS 2017 or 2019 's way but I am sure you can get full binary representation by SSMS Profilter.
  2. OK, for google maps , just for a chance.
  3. I suggest you post constant values of the points array to the masters of this git, if you still not get solution.
  4. By the chance, get WKT from google maps , then #15668 Wicket for Google Maps API v3 Android API about WKT Lazy way
DanielStout5 commented 5 years ago

Good idea about the Sql Server Profiler. That did get me the full parameters.

I found two zoom levels (two different bounding polygons), which should share many of the same points. The first returns nothing, the second (the same area except more zoomed in, so it should return less) returns them all.

--declare @poly varbinary(112) = 0xE610000001040500000079DA50E81999434094D172A355B75AC079DA50E8199943404046CB8D963C37C0EAC7100794B84B404046CB8D963C37C0EAC7100794B84B4094D172A355B75AC079DA50E81999434094D172A355B75AC001000000020000000001000000FFFFFFFF0000000003
declare @poly varbinary(112) = 0xE6100000010405000000BC29F036B459444094D172A3B1AB55C0BC29F036B459444028A3E546336F46C0EDE38ED110A0484028A3E546336F46C0EDE38ED110A0484094D172A3B1AB55C0BC29F036B459444094D172A3B1AB55C001000000020000000001000000FFFFFFFF0000000003

select * from businesses where location.STIntersects(@poly) = 1

--select geography::STGeomFromWKB(0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0,4326);

0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0 is a location within both of the poly values, but it only gets returned by the second one.

When I try to convert that point via STGeomFromWKB, it throws this exception:

System.FormatException: 24201: Latitude values must be between -90 and 90 degrees.

Which doesn't make sense - neither coordinate is outside of that range (so even if long/lat was reversed, it shouldn't cause that issue!)

If I try to use STGeomFromWKB on the polygon, it says:

System.FormatException: 24115: The well-known binary (WKB) input is not valid.

It seems that the format I'm getting from the SQL Server Profiler (which starts with 0xE61...) isn't actually WKB, and I'm not sure how to convert it into a more readable format.

If I do this in C#: var bin = WKBWriter.ToHex(poly.AsBinary());

Then I can use that result in STGeomFromWKB in SQL Server:

SQL:
select geography::STGeomFromWKB(0x0103000000010000000500000070561335C66056C096218CB798B04440E0AC266A5CD947C096218CB798B04440E0AC266A5CD947C04D2EEDD5D1EA484070561335C66056C04D2EEDD5D1EA484070561335C66056C096218CB798B04440, 4326).ToString()

Output:
POLYGON ((-89.512097615131779 41.37966055242866, -47.698132771381779 41.37966055242866, -47.698132771381779 49.834528675852745, -89.512097615131779 49.834528675852745, -89.512097615131779 41.37966055242866))

But I don't think this result is actually useful - the problem isn't on the C# side which is where that WKB representation is coming from.

bricelam commented 5 years ago

It seems that the format I'm getting from the SQL Server Profiler (which starts with 0xE61...) isn't actually WKB, and I'm not sure how to convert it into a more readable format.

Correct, it's the SQL Server serialization format. You can turn it into WKT like this:

SELECT geography::Deserialize(0xE61...).STAsText()
JediLeeSkywalker commented 5 years ago

Sorry, My wrong: DECLARE @g geography; SET @g =0xE6100000010405000000BC29F036B459444094D172A3B1AB55C0BC29F036B459444028A3E546336F46C0EDE38ED110A0484028A3E546336F46C0EDE38ED110A0484094D172A3B1AB55C0BC29F036B459444094D172A3B1AB55C001000000020000000001000000FFFFFFFF0000000003 ; SELECT @g.ToString() is right way.

DECLARE @g geography; SET @g =0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0; SELECT @g.ToString()

DanielStout5 commented 5 years ago

Correct, it's the SQL Server serialization format. You can turn it into WKT like this:

Oh, that's great - thanks!

So here is an example of the polygon/point that doesn't work.

SQL:

declare @poly varbinary(112) = 0xE610000001040500000079DA50E81999434094D172A355B75AC079DA50E8199943404046CB8D963C37C0EAC7100794B84B404046CB8D963C37C0EAC7100794B84B4094D172A355B75AC079DA50E81999434094D172A355B75AC001000000020000000001000000FFFFFFFF0000000003;
declare @pt varbinary(112) = 0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0;

select geography::Deserialize(@poly).STAsText();
select geography::Deserialize(@pt).STAsText()
select geography::Deserialize(@pt).STIntersects(@poly)

Output:

POLYGON ((-106.86460195744286 39.1961031336586, -23.2366722699428 39.1961031336586, -23.2366722699428 55.442017443841749, -106.86460195744286 55.442017443841749, -106.86460195744286 39.1961031336586))

POINT (-63.57745 44.64129)

0

If I pop those WKT formats into bboxfinder.com, I get this map:

image

I think it's pretty clear that point is within the polygon, so why is STIntersects returning 0?

bricelam commented 5 years ago

I think you're looking for STContains...

bricelam commented 5 years ago

..err, I mean STWithin.

bricelam commented 5 years ago

I think STIntersects means anything that crosses the polygon boundary on SQL Server

DanielStout5 commented 5 years ago

I've tried STWithin, STContains, STIntersects, STOverlaps, as well as STDistance <= 0, and they all return 0/false.

STWithin is what I originally started with.

If I use a slightly different polygon, then both STWithin and STIntersects return true.

So this polygon says it doesn't contain the point:

POLYGON ((-106.86460195744286 39.1961031336586, -23.2366722699428 39.1961031336586, -23.2366722699428 55.442017443841749, -106.86460195744286 55.442017443841749, -106.86460195744286 39.1961031336586))

While this one does contain it:

POLYGON ((-86.682717191817858 40.700812213198759, -44.868752348067858 40.700812213198759, -44.868752348067858 49.250513262530013, -86.682717191817858 49.250513262530013, -86.682717191817858 40.700812213198759))

Here's the map of the one that does work:

image

Very similar to the one I posted earlier that doesn't work!

JediLeeSkywalker commented 5 years ago

declare @poly varbinary(112) = 0xE610000001040500000079DA50E81999434094D172A355B75AC079DA50E8199943404046CB8D963C37C0EAC7100794B84B404046CB8D963C37C0EAC7100794B84B4094D172A355B75AC079DA50E81999434094D172A355B75AC001000000020000000001000000FFFFFFFF0000000003; declare @pt varbinary(112) = 0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0;

select geography::Deserialize(@poly).STAsText(); select geography::Deserialize(@pt).STAsText() select geometry::Deserialize(@pt).STAsText() select geometry::Deserialize(@pt).STIntersects(@poly)

result: POLYGON ((-106.86460195744286 39.1961031336586, -23.2366722699428 39.1961031336586, -23.2366722699428 55.442017443841749, -106.86460195744286 55.442017443841749, -106.86460195744286 39.1961031336586))

POINT (-63.57745 44.64129)

POINT (44.64129 -63.57745)

1

DanielStout5 commented 5 years ago

select geometry::Deserialize(@pt).STIntersects(@poly)

That's interesting.. So if the point is Geometry instead of Geography, it is counted as being within the polgyon.

I could convert the stored Location from geography to geometry, but the problem is that the application uses distance queries.

select geometry::Deserialize(@pt).STDistance(@pt2)
select geography::Deserialize(@pt).STDistance(@pt2)

Result:

0.053120579816113

4393.5315056106

The latter is the correct distance in meters. The first is something else, maybe degrees?

JediLeeSkywalker commented 5 years ago

It is a easy way that change the poly switches x y to y x , or anyway to get poly object to real geography?

DanielStout5 commented 5 years ago

It is a easy way that change the poly switches x y to y x , or anyway to get poly object to real geography?

I'm not entirely sure what you mean.

I create the polygon in my code based on the map bounds the frontend sends up:

var points = new[]
{
    new Coordinate(neLng.Value, neLat.Value), // TR
    new Coordinate(neLng.Value, swLat.Value), // BR
    new Coordinate(swLng.Value, swLat.Value), // BL
    new Coordinate(swLng.Value, neLat.Value), // TL
    new Coordinate(neLng.Value, neLat.Value), // TR
};

var fact = NtsGeometryServices.Instance.CreateGeometryFactory(srid: 4326);
var poly = fact.CreatePolygon(points).Normalized().Reverse();

I'm just not sure why switching the x/y would be necessary - I put the WKT result into bboxfinder.com and it matches what I expected.

I saw a comment a minute ago about setting the Z value of the points and polygon to 0 fixing the STIntersect. Did that not actually work?

JediLeeSkywalker commented 5 years ago

No it not work, I found the incorrect code that I put @g1.STIntersects(@g1), Z point not work.

this works: declare @g1 geography = geography::STPolyFromText('POLYGON ((-106.86460195744286 39.1961031336586,-106.86460195744286 55.442017443841749, -23.2366722699428 55.442017443841749, -23.2366722699428 39.1961031336586,-106.86460195744286 39.1961031336586))',4326); declare @g2 geography = geography::STGeomFromText('POINT (-63.57745 44.64129 0)',4326);

select @g1.STIntersects(@g2)

result: 1

DanielStout5 commented 5 years ago

Hmm.. That polygon you're using is different than the original - the points are in a different order. Specifically the 2nd and 4th are switched (You're going counter-clockwise, I think?)

If I change the creation of the polygon to this:

var points = new[]
{
    new Coordinate(neLng.Value, neLat.Value), // TR
    new Coordinate(swLng.Value, neLat.Value), // TL
    new Coordinate(swLng.Value, swLat.Value), // BL
    new Coordinate(neLng.Value, swLat.Value), // BR
    new Coordinate(neLng.Value, neLat.Value), // TR
};

It still has the same issues as before. I'm not sure it's happening the exact same way, but at certain zoom levels and pan positions (i.e. certain bounding polygons) the STWithin/STIntersects still returns false where it shouldn't. Calling .Normalized().Reverse() has essentially no effect.

JediLeeSkywalker commented 5 years ago

I had searched what is different from STPolyFromText and STGeomFromText, and more about direction of polygon, I found diff is most closest this question. It mentions Reverse order at last reply, or you could make the WKT reader , I wish NTS would do some work like STPolyFromTexts' doing. Another saying ,get WKT by regular API, then you would get right polygon.

DanielStout5 commented 5 years ago

I think this might be part of the issue:

Turns out SSMS has a Spatial Results visualizer:

declare @poly varbinary(112) = 0xE610000001040500000079DA50E81999434094D172A355B75AC079DA50E8199943404046CB8D963C37C0EAC7100794B84B404046CB8D963C37C0EAC7100794B84B4094D172A355B75AC079DA50E81999434094D172A355B75AC001000000020000000001000000FFFFFFFF0000000003;
declare @pt varbinary(112) = 0xE6100000010C3CA06CCA15524640A089B0E1E9C94FC0;

select geography::Deserialize(@poly).STAsText();
select geography::Deserialize(@pt).STAsText()
select geography::Deserialize(@pt).STIntersects(@poly)

select geography::Deserialize(@poly) union all select geography::Deserialize(@pt)

image

That little black dot is the @pt

So the point would be within the polygon if it had straight lines, but they connected by curves. Perhaps Reverse would just flip the polygon on the X plane - so it might work for the bottom points but wouldn't work for the top ones..

Do you know if there is some alternative way to give that polygon to EF Core that would change how it interprets the coordinates?

JediLeeSkywalker commented 5 years ago

I am not sure if this info could help. If I change compatibility_level of SQL Server Database to 100, I get an error running above SQL:

Microsoft.SqlServer.Types.GLArgumentException: 24205: The specified input does not represent a valid geography instance because it exceeds a single hemisphere. Each geography instance must fit inside a single hemisphere. A common reason for this error is that a polygon has the wrong ring orientation. To create a larger than hemisphere geography instance, upgrade the version of SQL Server and change the database compatibility level to at least 110.

DanielStout5 commented 5 years ago

Interesting.. but I've already tried both ring orientations - assuming that's referring to the order the points are passed into IGeometryFactory.CreatePolygon. And the polygon definitely doesn't span hemispheres.

JediLeeSkywalker commented 5 years ago

I tried to draw a similar polygon in Wicket, then I get the result WKT put to SQL, then it works. Get the WKT by API that before I mentioned, then use WKTReader.

POLYGON((-97.5701683636928 54.333628305112036,-29.3670433636928 54.333628305112036,-29.3670433636928 38.50235178337623,-97.5701683636928 38.50235178337623,-97.5701683636928 54.333628305112036)) image

DanielStout5 commented 5 years ago

The only reason that works is because you have chosen a different polgyon that happens to include the point. That isn't a solution to this issue, because the polygon is dynamic depending on where the user has panned/zoomed the map to.

image

The problem is that the polygon doesn't have the correct shape for some reason when passed to SQL Server, even though it does when evaluated locally.

ajcvickers commented 5 years ago

Closing this as external since it seems there are no issues in the translation to SQL, just some confusion as to how SQL Server interprets these queries on a sphere.

jasonhjohnson commented 4 years ago

@DanielStout5 Daniel, did you ever get this figured out? I'm running into the same issue.

DanielStout5 commented 4 years ago

Unfortunately not. I ended up just loading all of the data and doing the intersection calculation locally in C#.

I think the problem is to do with the coordinate system used by SQL Server as default. I'm not that familiar with GIS - if you end up finding a solution I'd be very eager to hear it!

jasonhjohnson commented 4 years ago

My issue didn't appear to be exactly the same as I wasn't getting the proper match when evaluating locally or via sql server. It's working for me now when I rearranged the coordinates as follows:

var points = new[]
            {
                new Coordinate(neLat, neLng), // TR
                new Coordinate(neLat, swLng), // BR
                new Coordinate(swLat, swLng), // BL
                new Coordinate(swLat, neLng), // TL
                new Coordinate(neLat, neLng), // TR
            };
bricelam commented 4 years ago

I suspect this is an orientation issue. With SQL Server geography, the shell of a polygon must be oriented counter-clockwise, and the holes must be oriented clockwise. If all the rings are oriented clockwise, it's interpreted as holes in FullGlobe. I think that's what the blue part of this image is showing:

image

DanielStout5 commented 4 years ago

@bricelam That screenshot was just a demonstration. This is the one that was actually in use:

And in the code, the points are defined in counter-clockwise order:

var points = new[]
{
    new Coordinate(neLng.Value, neLat.Value), // Top Right
    new Coordinate(swLng.Value, neLat.Value), // Top Left
    new Coordinate(swLng.Value, swLat.Value), // Bottom Left
    new Coordinate(neLng.Value, swLat.Value), // Bottom Right
    new Coordinate(neLng.Value, neLat.Value), // Top Right
};

var fact = NtsGeometryServices.Instance.CreateGeometryFactory(srid: 4326);
var poly = fact.CreatePolygon(points);

var within = query.Where(x => x.Location != null)
    .ToList() // Force local evaluation
    .Where(x => x.Location.Within(poly))
    .ToList();

return Ok(PagedData<Shop>.FromList(within));

//query = query
//    .Where(x => x.Location.Within(poly));