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

Support geometric types for postgres #166

Open
mehcode opened this issue Mar 26, 2020 · 21 comments · May be fixed by #3449
Open

Support geometric types for postgres #166

mehcode opened this issue Mar 26, 2020 · 21 comments · May be fixed by #3449
Labels
db:postgres Related to PostgreSQL E-medium enhancement New feature or request good first issue Good for newcomers

Comments

@mehcode
Copy link
Member

mehcode commented Mar 26, 2020

https://docs.rs/geo
https://www.postgresql.org/docs/current/datatype-geometric.html

  • POINT -> geo::Coordinate<f64>
  • LINE -> geo::Line<f64>
  • LSEG -> geo::Line<f64>
  • BOX -> geo::Rect<f64>
  • PATH -> geo::LineString<f64>
  • POLYGON -> geo::Polygon<f64>
@mehcode mehcode added enhancement New feature or request good first issue Good for newcomers db:postgres Related to PostgreSQL E-easy E-medium and removed E-easy labels Mar 26, 2020
@abonander
Copy link
Collaborator

abonander commented Mar 30, 2020

All types

OID sources: https://github.com/postgres/postgres/blob/2743d9ae4a490a7d96b5c19d50694bd101a87dc8/src/include/catalog/pg_type.dat

Text encoding descriptions: https://www.postgresql.org/docs/current/datatype-geometric.html
The documentation specifies which text syntax you're looking for. For robustness, the implementation should not assume any specifics about whitespace around characters.

For text encoding, notice that several types share similar formats, so you may not need exactly one function per type.

Binary encoding implementation in C: https://github.com/postgres/postgres/blob/2743d9ae4a490a7d96b5c19d50694bd101a87dc8/src/backend/utils/adt/geo_ops.c

All values are big (network)-endian encoded.

POINT

  • type OID 600, array type OID 1017
    pg_type.dat lines 183-187
  • Binary encoding: geo_ops.c line 1831
f64: x
f64: y
  • Text encoding:
( x , y )

LINE

*type OID 628, array type OID 629
pg_type.dat lines 209-212

  • Binary encoding: geo_ops.c line 1034
f64: A
f64: B
f64: C

where A, B, and C are coefficients in the equation Ax + By + C = 0

  • Text encoding: { A, B, C }

LSEG

  • type OID 601, array type OID 1018
    pg_type.dat lines 188-192
  • Binary encoding: geo_ops.c line 2064
f64: x1
f64: y1
f64: x2
f64: y2
  • Text encoding:
[ ( x1 , y1 ) , ( x2 , y2 ) ]

BOX

  • type OID 603, array type OID 1020
    pg_type.dat lines 198-203
  • Binary encoding: geo_ops.c line 489
f64: x1
f64: y1
f64: x2,
f64: y2

where (x1, y1) is the top-left corner and (x2, y2) is the bottom-right corner.

Text encoding:

( x1 , y1 ) , ( x2 , y2 )

PATH

  • type OID 602, array type OID 1019
    pg_type.dat lines 193-197

  • Binary encoding: geo_ops.c line 1485

i8: closed (0 for false or 1 for true)
i32: N, number of points
for _ in 0 .. N {
    f64: point.x
    f64: point.y
}
  • Text encoding:
[ ( x1, y1) , ... , ( xN , yN ) ] : open form
( ( x1 , y1 ) , ... , ( xN , yN ) ) : closed form

POLYGON

  • type OID 604, array type OID 1027
    pg_types.dat lines 204-208

  • Binary encoding: geo_ops.c line 3556

i32: N, number of points
for _ in 0 .. N {
    f64: point.x
    f64: point.y
}
  • Text encoding:
( ( x1 , y1 ) , ... , ( xN , yN ) )

@Freax13
Copy link
Contributor

Freax13 commented Feb 2, 2021

* `POLYGON` -> `geo::Polygon<f64>`

geo::Polygon is not equivalent to POLYGON as it also handles holes within the bounded area.

@nixpulvis
Copy link

nixpulvis commented Feb 25, 2021

I just left a comment on the geo crate (see mention above) and I'm also aware of https://github.com/pka/geozero which implemented the WKT/WKB types already. As I'm interested in working on this feature for the purpose of getting 3D points for my application, I'm curious what acceptable approach is.

I would frankly find it irritating to need two separate features for 2D and 3D points, but on the other hand I'm not sure if it can be avoided, since PostgreSQL itself implements a 2D point, while something like PostGIS implements others. So perhaps it's just a question of feature naming, "geo" implies the geo crate, while maybe geozero give you the "postgis" features?

I'd be personally tempted to start with the postgis feature, unless anyone can see a way in which these are dependent on each other?

@pka
Copy link
Contributor

pka commented Feb 25, 2021

In the geospatial world, geometries are almost exclusively used together with the PostGIS extension. So implementing the PostgreSQL geometric types has not very high priority. As mentioned, geozero supports encoding/decoding PostGIS geometries with SQLx (example).

@nixpulvis
Copy link

OK, so it sounds like my first task should be to extend geozero with PointZ support for sqlx. It seems like this shouldn't require any changes here, since the conversions are done in that crate, and it's already working for other geometries.

Given that you need to enable the extension postgis for this in PostgreSQL, I would love to have everything you need from geozero inside a postgis module somewhere, even if they're just aliases. This would help new users to find the correct types in the somewhat complex geometry situation.

As for native PostgreSQL geometries, I can hopefully come back to this after I get a solid grasp on the postgis types.

@nixpulvis
Copy link

@pka, IDK maybe I'm just missing something, but I cannot get the example to work in my application, and all the tests for postgis are marked as [ignore]. I'm getting the error error: unsupported type geometry of column #9 ("<column name>").

@pka
Copy link
Contributor

pka commented Feb 25, 2021

@nixpulvis The PostGIS tests were broken indeed. Here are the setup instructions: https://github.com/georust/geozero/blob/master/geozero/tests/data/postgis.sql#L1. Please report any failures in the geozero repo.

@bbqsrc
Copy link

bbqsrc commented Mar 12, 2021

I'm not sure with this given example how to make sqlx and geozero play nicely with a struct, though? Having a field with wkb::Decode doesn't seem to be enough there, and it's not clear how to involve a type override.

@pka
Copy link
Contributor

pka commented Mar 12, 2021

@bbqsrc if you want to read a geometry as part of a struct, you have to use wkb::Decode<YourGeomType> as field type (Example), or you implement sqlx::decode::Decode for your geometry type (see also https://docs.rs/geozero/0.7.4/geozero/index.html#macros).

@Gearme
Copy link

Gearme commented Mar 14, 2021

I'm not sure why this issue drifted to geospatial support when the OP clearly meant only geometric support? Also, did you note the pull request by @qtbeee for support of geometric types? In fact, @abonander actually mentioned that this was nothing to do with geospatial support in the conversation on that pull request.

I'm aware that @pka said:

In the geospatial world, geometries are almost exclusively used together with the PostGIS extension. So implementing the PostgreSQL geometric types has not very high priority. As mentioned, geozero supports encoding/decoding PostGIS geometries with SQLx (example).

But note that he (correctly) mentioned that this was the case in what he calls the geospatial world. There are use cases (such as image and document analysis) which require geometric support that have nothing to do with GIS. I therefore respectfully disagree with the implementation of geometric types having a low priority simply because they're less important to geoinformatics.

I suggest starting a new issue for geospatial support and making this ticket about Postgres Geometry types as the OP intended.

@nixpulvis
Copy link

I'm not sure why this issue drifted to geospatial support when the OP clearly meant only geometric support?

I'm sorry for causing some drifting here. My main issue was that I needed 3D points, which unfortunately aren't supported without extensions. I wouldn't say that 3D points are fundamentally any more geospatial than geometrical, you just happen to need PostGIS to get 3D points.

I wasn't sure if there was some desire to unify the types between "native" and PostGIS primitives.

@camsjams
Copy link

camsjams commented Jul 7, 2021

Are there any workarounds for just inserting a point into a new row? Should I just use unchecked?

@abonander
Copy link
Collaborator

Given how often we've run into semver issues with using external crates for data modeling, I think I'd rather just provide bespoke structs for mapping these types.

@stevemarin
Copy link

Just for reference, I could not get the query! macro to work with inserting geometry types in PostGIS by following the linked examples. As @camsjams alludes, everything works fine using the query function. I was seeing the same error as @nixpulvis above when using the macro.

I spent a lot of time trying to get the macro to work fruitlessly, while the function worked fine. Not complaining! Sqlx is great! Thank you so much! Just pointing out for others running into the same issue, since it wasn't explicitly pointed out here.

@VincentWo
Copy link

Hi, I would very much like to contribute geometry type support (and postgis type support, if wanted) - but I am a bit confused about the current state of affairs?

Is postgis support natively in sqlx wanted? (In my opinion it would be a good thing, it is a semi-popular standard and hidden behind a feature gate it wouldn't incure any cost for people not using postgis)

This especially matters for me since as far as I see it, it is not possible to add third party types to sqlx which are checked at compile time

So - if support for postgis is wanted I would be happy to contribute, but should we add custom types to sqlx itself or use a different rust library for that? Intuitivly I would choose a different, more established library, but @abonander argued against it and imo best suited library geo-types didn't hit 1.0 yet (even though the release cycle doesn't seem to be too fast)

And should the WKB decoding be bespoke? There is https://docs.rs/wkb/latest/wkb/, but this would lock us into geo-types and would be another potential semver issue. And postgis uses EWKB anyway, which is not difficult to implement, but I don't want to impose an unwanted maintenance burden on anyone

I would love feedback and I would also feel comfortable writing a draft PR for support, but only if this is actually a wanted feature, apology if I oversaw any prior discussion.

@mio991
Copy link

mio991 commented Feb 24, 2024

What is the current consensus, do we use the geo-types or roll our own?

As a compromise I would propose we add public traits to allow custom types for the deserialization. Throug New Types you could use these traits to add suport for geo-types for example.

Or we make traits with assoziate types, so you can create a ZST implementing these to simulate implementing them for external types.

PS: if we roll our own type do we want a feature flag for them?

@tumluliu
Copy link

same question here. What is still missing/required to proceed? And it seems also blocking the postgis support from the SeaORM side if my understanding is correct?

@abonander
Copy link
Collaborator

There should be sufficient traits to implement decoding for any given type. You'd just have to request it using the type override syntax (see the docs), making the macros choose an external type by default is a different question entirely.

The biggest issue with integrating an external crate is stability. Is it yet another crate we're going to have to upgrade with every minor (0.x.y -> 0.{x+1}.0 release? That's just something we have to consider due to its effects on the burden of maintenance.

@LockedThread
Copy link

Any updates?

@Tienisto
Copy link

Tienisto commented Jul 2, 2024

In addition to @Gearme , in the official documentation of PostGIS, they state that:

The geography type is very convenient for people whose data is of global extent and for people who do not want to learn about projected coordinate systems. However, calculations on the spheroid are very expensive, so many queries will be slower in geography than geometry.

Which means that it is perfectly reasonable to use the simpler geometric type when

  • the distance is so small that the 2D geometric interpretation is good enough
  • you are not calculating the distance (or any other operations) anyway and just want to save the location and visualize it on the frontend
  • you are using ARM64: ARM-64 build postgis/docker-postgis#216

Edit 1:
If this issue is too big, we can also support an f64 pair (f64, f64) that would be interpreted as a Point type which should already cover most use cases. In this case, we don't use any third-party library and we also don't declare a new type - we just have a sane interpretation of how to encode/decode a pair of f64 in a database.

@jayy-lmao
Copy link
Contributor

Wouldn’t mind having a crack at the types outlined in #166 (comment).
So long as I can steer clear of postgis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:postgres Related to PostgreSQL E-medium enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.