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

Reconsider the decision to throw an error on ring orientation mismatch #9

Closed
airbreather opened this issue Oct 4, 2019 · 3 comments

Comments

@airbreather
Copy link
Member

@IanKemp my recommendation would be to orient your polygon rings earlier than this, ideally as the last step of whatever processing you do when building the polygon (or, if possible, do the processing in a way that will naturally result in them being oriented this way). The polygon data you got had to come from somewhere. If it came from SQL Server and you haven't touched it since, then it should be good already. Otherwise, wherever you touched it, that's where the reorientation should happen.

I strongly disagree with this argument. That SQL server requires polygons to be stored as it does is SQL server's problem, i.e. a concern of the persistence layer - what you are suggesting is to make it a concern of layers higher up the stack, which is completely the wrong thing to do because those layers don't and shouldn't need to know or care about the nitty-gritty of data persistence and retrieval.

And fundamentally, by throwing an exception what you are saying is "your data is wrong". But it's not - it's just that SQL Server chooses to store it in a way that may make it look wrong when you later retrieve it. Which, again, is an issue that no other RDMBS would have.

An EF hook to transform data on the way in/out of the persistence provider would solve this problem, because then I could just do the geo normalisation + reverse in a general manner there. (Which is essentially what I'm attempting to do in my DbContext above.) But it would be a very edge-case scenario and as such seems like something the EF team wouldn't be particularly interested in prioritising, which means I can't rely on it happening anytime soon.

SQL Server's geography type uses ring orientation, rather than defining rings as "exterior / interior" like NTS does, to determine which side of a ring is "inside" vs. "outside".

God that documentation is useless. The only reference to the storage format is the vague sentence "The interior of the polygon in an ellipsoidal system is defined by the left-hand rule" - there is nothing that says "your polygon has to be defined counterclockwise or SQL Server's EF provider will refuse to save it".

I suspect that even if we implemented an "opt in" flag, we'd still get some confusion about "why are my polygons different after saving / loading" because frankly it seems just really easy to forget that "5 months ago, I copy-pasted a line of code that I found on StackOverflow somewhere that made everything work".

I suspect rather less confusion with the option, as opposed to a lot of unhappiness without it, as currently.

Originally posted by @IanKemp in #4 (comment)

@airbreather
Copy link
Member Author

[...] SQL server requires polygons to be stored as it does [...]
[...]
The only reference to the storage format is the vague sentence [...]

@IanKemp I don't have the time to respond to your entire comment right now (in fact, I shouldn't even be spending the time I'm spending to type out this comment right now), but I did want to respond to these bits -- the disconnect has very little to do with the storage format as you suggest. Rather, there is a fundamental gap between NTS and SQL Server over the very nature of the data being stored.

In NTS, a polygon ring is defined as either a "shell" or as a "hole". It gets away with this because its algorithms interpret the point data as locations on a plane that extends "infinitely" in two dimensions.

In SQL Server, the geography type needs more information than just "is it a shell or a hole?" in order to determine how to partition the world into "things outside the ring" and "things inside the ring". If you put down a fence around yourself, for example, it could mean one of two things: either "I am standing inside this fence" or "everything in the world is inside this fence except for me".

  • This seems kinda silly, until you consider the fact that "the fence" can actually be a straight line around the equator, and so you now have to determine which hemisphere to pick as "inside".

So in SQL Server, the developers define the "inside" / "outside" determination using the ring's orientation.

I think the distinction is important, because if it was "just" a difference in the storage format, then it would absolutely make sense for that to be handled here.

@airbreather
Copy link
Member Author

After NetTopologySuite/NetTopologySuite@a819cdd makes it to a release, we can start recommending that people start using GeometryFactoryEx and setting OrientationOfExteriorRing as appropriate, which could help resolve this in a lot of situations.

@FObermaier
Copy link
Member

After NetTopologySuite/NetTopologySuite@a819cdd makes it to a release, we can start recommending that people start using GeometryFactoryEx and setting OrientationOfExteriorRing as appropriate, which could help resolve this in a lot of situations.

Has been released with v2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants