efcore.pg icon indicating copy to clipboard operation
efcore.pg copied to clipboard

Provide mapping for `Envelope`

Open blair-ahlquist opened this issue 5 years ago • 10 comments

When I use NetTopologySuite.Geometries.Envelope in my model, and generate a migration, I get: The entity type 'Envelope' requires a primary key to be defined. If you intended to use a keyless entity type call 'HasNoKey()'. I was able to successfully use other geometry types such as Point and Polygon. It seems then that Envelope is simply not mapped.

blair-ahlquist avatar Mar 18 '20 18:03 blair-ahlquist

Would a PR be accepted for this issue? Perhaps one that gets most of the way there? I'm pretty sure I can mimic what you have done with the other geometry types.

blair-ahlquist avatar Mar 19 '20 19:03 blair-ahlquist

The problem is that PostGIS (the PostgreSQL spatial extension) doesn't have an envelope type, so it's not clear what a mapping here would do exactly.

However, the provider does translate the Envelope method NetTopologySuite's Geometry type to PostGIS ST_Envelope function, is that not sufficient for your purposes?

roji avatar Mar 21 '20 19:03 roji

@roji maybe the Box2D type could be used for this purpose?

fromm1990 avatar May 25 '20 14:05 fromm1990

@fromm1990 I'm not a spatial expert, but I think an envelope isn't necessarily 2D. Am still curious as to why the ST_Envelope function - which we already translate - isn't sufficient...

roji avatar May 27 '20 09:05 roji

@roji I'm no expert myself, however, here is my reasoning:

I use Box2d as the type for my envelopes, simply because nothing else can be stored in them. Using ST_Envelope() will output a geometry of type polygon. This is not a problem if you always put an envelope into the geometry/polygon column. However, nothing is preventing you from polluting that column with randomly formatted polygons either. I simply like the safety-net provided by the Box2d type. Of course, one could make a CHECK constraint to omit this problem, but in my opinion, the column type should prevent it from happening in the first place.

When it comes to the envelope type in NTS, it's my understanding that it is two dimensional exclusively, however, ST_Envelope support both 2D and 3D envelopes.

As I understand, the box2d and box3d types needs a binary formats in order to integrate with Npqsql, which is not currently the case?

fromm1990 avatar May 28 '20 08:05 fromm1990

Reading up, it does seem like NTS defines Envelope in a similar way that PostGIS defines box2d. However, the actual functions supported by NTS seem quite different from [those supported by PostGIS on box2d, so is there really much value in this mapping?

As I understand, the box2d and box3d types needs a binary formats in order to integrate with Npqsql, which is not currently the case?

That's a good question - if there's indeed no PostGIS binary read/write support for box2d/box3d, then Npgsql can't read/write those types from PG. It's certainly worth checking.

roji avatar May 28 '20 09:05 roji

@roji I think the power of the mapping comes down to ease of use and PostGIS type support. I know this is slightly different from what OP addressed, but let me elaborate. As stated in https://github.com/npgsql/efcore.pg/issues/1313#issuecomment-635202797 I like the "geometric type" (box2d does not inherit from geometry, however it's a rectangle witch is a polygon which is a geometry) to guide what content is allowed. This is why I personally think box2d and NTS Envelope makes a good pairing. If, I'm correct, you cannot instantiate an envelope such it would not be compatible with box2d. This means that a greater set of exceptions will be caught at compile time.

Now to the issue if box2d has no binary read/write format. I have "solved" this in my current project by having a NpgsqlSimpleTypeHandler<Envelope> that will handle a geometry to envelope type conversion. My queries therefore cast my box2d type like so bounding_box::geometry such i get it into a binary read/write format (If this is not done you will get Npgsql.PostgresException: '42883: no binary output function available for type box2d'). Of course this allows any geometry to be read as an Envelope, which is suboptimal.

This leads me to the two following questions:

  1. Is it possible for Npgsql to always cast a box2d to a geometry (box2d::geometry) when read and always call Box2D(geometry) when written to the database?
  2. Is it possible for Npgsql to know that the original pgtype was a box2d such the TypeHandler would be able to differentiate between geometry::{polygon | linestring | point | etc. } and a geometry that whas casted from a box2d?

If both are possible, I believe it's possible to write a box2d to geometry (and vice versa) converter regardless of box2d has a binary read/write format. Please let me know if I'm wrong on this matter?

fromm1990 avatar May 28 '20 10:05 fromm1990

Is it possible for Npgsql to always cast a box2d to a geometry (box2d::geometry) when read and always call Box2D(geometry) when written to the database? Is it possible for Npgsql to know that the original pgtype was a box2d such the TypeHandler would be able to differentiate between geometry::{polygon | linestring | point | etc. } and a geometry that whas casted from a box2d?

It may be possible to do some extremely dirty hacks to make all this work somehow, but I don't think that makes sense only because the type is lacking binary encoding in PostGIS - and pretty much exclusively for a need that a CHECK constraint can pretty much solve... I'd definitely first check with PostGIS about adding binary encoding, and if there's maybe a good reason they haven't done this.

You could still convert your PostGIS envelope polygons to box2d and then back to polygons. When reading these polygons, you would therefore be certain that they represent 2D shapes, even if the type doesn't constrain it.

roji avatar May 28 '20 10:05 roji

I agree that a dirty hack to solve this problem is not desired either. The two questions was simply due to my lack of knowledge on the internals of Npgsql.

If no binary read/write format for box2d exists I believe it's not intentional https://lists.osgeo.org/pipermail/postgis-devel/2015-May/024905.html (note this is from 2015).

fromm1990 avatar May 28 '20 10:05 fromm1990

Yeah, you're right that this seems to be the case. It may be a good idea to at least send them a message to let them know this is needed...

roji avatar May 28 '20 11:05 roji