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
Spatial support for PostgreSQL using PostGIS #2423
Conversation
This includes support for both geometry and geography columns as well as GiST indices (used when passing the `spatial: true`). Client representations use GeoJSON (existing MySQL and MS SQL drivers use WKT (well-known text)) for compatibility with geospatial libraries such as Turf, JSTS, etc.
@mojodna Thanks, I was just about to do this! I'm going to try add a test or function for |
Specifying a feature type will pass it as a parameter to the underlying spatial type, allowing PostGIS to validate that stored geometries match expectations. Specifying an SRID will do similarly, with the additional benefit of allowing the coordinate system used for a geometry to be stored in order to transform it into other coordinate systems.
This way, if transforms are applied to parameters, they only need to be implemented in 1 place.
See mojodna#1 for more discussion on this. |
This allows synchronization to modify PostGIS constraints while also surfacing metadata in expected places.
@Column({ type: "geometry" }) | ||
@Index({ spatial: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it broke the build; some of the before/after hooks are now hanging. Is there a good way to diagnose this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it looks like introducing support for spatial indexes in Postgres is conflicts with tests that also execute in MySQL. I think I've just pushed a fix for this.
Since the order-by tests run for multiple database engines, including Postgres-targeted spatial types will break other engines (or cause them to hang on startup).
@@ -370,7 +384,7 @@ export class PostgresDriver implements Driver { | |||
|| columnMetadata.type === "timestamp without time zone") { | |||
return DateUtils.mixedDateToDate(value); | |||
|
|||
} else if (columnMetadata.type === "json" || columnMetadata.type === "jsonb") { | |||
} else if (this.spatialTypes.concat(["json", "jsonb"]).indexOf(columnMetadata.type) >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previous one was better =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waits for Array.includes
in TS...
Local style should be:
} else if (columnMetadata.type === "json" ||
columnMetadata.type === "jsonb" ||
this.spatialTypes.indexOf(columnMetadata.type) >= 0) {
❓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, thats fine as well, you can keep it. I simply don't like concat
, I would either go with "local style" you wrote or ["json", "jsob", ... this.spatialTypes].indexOf()
which looks much better than concat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, I always forget about spreads in this context.
docker-compose.yml
Outdated
@@ -39,7 +39,7 @@ services: | |||
|
|||
# postgres | |||
postgres: | |||
image: "postgres:9.6.1" | |||
image: "mdillon/postgis:9.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess somebody else can easily create a PR for the feature with specific extension and put another docker image with that extension, but without gis let's say. So better solution would be to have a base postgres image with all modifications applied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. mdillon/postgis
is just postgres
+ PostGIS (pretty sure it doesn't include any other additional extensions). Is this worth punting on until someone else needs to add another extension? (We can add a note/apology here ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha yeah... I hope it won't be me 😆
wow, this is huge. This wasn't an easy thing to implement, how much time it took you to bring all this into TypeORM?) Lets ask @AlexMesser if he can review it. |
Thanks! I'm guessing ~8 hours (including getting familiar with TypeORM generally while on a plane) over the span of a week and a half.. |
You are the beast! |
Great work! |
@AlexMesser docs pushed. |
Thank you very much! This will be published in 0.2.8 soon! |
Awesome work @mojodna , thanks again ! |
When can we expect 0.2.8 to be released? |
Yes please to 0.2.8! |
we have a blocking issue with nativescript that is merged and prevent block releasing 0.2.8 |
Any update on the 0.2.8 release @pleerock? |
Looking forward for this |
Hi @pleerock , Any update about the next release ? |
I just want a simple example please: I have this column:
when I try to insert data: it says ST_GeomFromGeoJSON is not defined. Please help |
It looks like you don’t have the PostGIS extension installed. To enable it in a specific database, run |
@mojodna sir, I am also able to run all these from within my nodejs code. (all the commented queries also work fine)
Only issue is when I try to insert data using typeorm, it gives me the error ST_GeoomFromText() is not a function |
This includes support for both geometry and geography columns as well as GiST indices (used when passing the
spatial: true
option). Client representations use GeoJSON (existing MySQL and MS SQL drivers use WKT (well-known text)) for compatibility with geospatial libraries such as Turf, JSTS, etc.Fixes #370