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

provider/postgis: Drop Z dimension for 3D geometries #449

Open
ARolek opened this issue Jul 3, 2018 · 10 comments · Fixed by #710
Open

provider/postgis: Drop Z dimension for 3D geometries #449

ARolek opened this issue Jul 3, 2018 · 10 comments · Fixed by #710
Assignees

Comments

@ARolek
Copy link
Member

ARolek commented Jul 3, 2018

Currently tegola's PostGIS driver ignores geometries that have a 3rd dimension. A better solution would be to support decoding PosGIS's EWKB format and drop the 3rd dimension. This issue is dependent on go-spatial/geom#16.

@ARolek ARolek changed the title PostGIS: Drop Z dimension for 3D geometries provider/postgis: Drop Z dimension for 3D geometries Jul 28, 2018
@gorbypark
Copy link

It would be nice to at least have a warning in the logs for this. I spent a couple hours trying to figure out why a MultiPolygonZ layer was not showing up, assuming that it would work or at least work in 2d. Turns out I just had to wrap the geom column in a ST_Force2D. It would be great to have actual 3d support as well, though!

@ARolek
Copy link
Member Author

ARolek commented Aug 25, 2019

@gorbypark yeah it's a nasty bug, one I would like to get addressed but we have some higher priorities right now. We're open to a PR on the geom package to support EWKB which we can then implement in the PostGIS driver. Thanks for documenting your solution here in case someone else hits it.

@gbroccolo
Copy link

@ARolek I started working on go-spatial/geom#16, but I realised there's still no support for the extended geometries Z (3D), ZM (4D) and geometries with projection reference - srid - info (S). In order to avoid too much changes related to a single issue, what do you think if I work on this before? I also include @gdey in the discussion here since he is involved in the other issue.

This is the list of geometry types that should be included to integrate EWKB protocol:

PointZ             // 2147483649
LineStringZ        // 2147483650
PolygonZ           // 2147483651
MultiPointZ        // 2147483652
MultiLineStringZ   // 2147483653
MultiPolygonZ      // 2147483654
CollectionZ        // 2147483655

PointM             // 1073741825
LineStringM        // 1073741826
PolygonM           // 1073741827
MultiPointM        // 1073741828
MultiLineStringM   // 1073741829
MultiPolygonM      // 1073741830
CollectionM        // 1073741831

PointZM            // 3221225473
LineStringZM       // 3221225474
PolygonZM          // 3221225475
MultiPointZM       // 3221225476
MultiLineStringZM  // 3221225477
MultiPolygonZM     // 3221225478
CollectionZM       // 3221225479

PointS             // 536870913
LineStringS        // 536870914
PolygonS           // 536870915
MultiPointS        // 536870916
MultiLineStringS   // 536870917
MultiPolygonS      // 536870918
CollectionS        // 536870919

PointZS            // 2684354561
LineStringZS       // 2684354562
PolygonZS          // 2684354563
MultiPointZS       // 2684354564
MultiLineStringZS  // 2684354565
MultiPolygonZS     // 2684354566
CollectionZS       // 2684354567

PointMS            // 1610612737
LineStringMS       // 1610612738
PolygonMS          // 1610612739
MultiPointMS       // 1610612740
MultiLineStringMS  // 1610612741
MultiPolygonMS     // 1610612742
CollectionMS       // 1610612743

PointZMS           // 3758096385
LineStringZMS      // 3758096386
PolygonZMS         // 3758096387
MultiPointZMS      // 3758096388
MultiLineStringZMS // 3758096389
MultiPolygonZMS    // 3758096390
CollectionZMS      // 3758096391

that should extend the 2D geometries at the moment included in this repo:

Point              // 1
LineString         // 2
Polygon            // 3
MultiPoint         // 4
MultiLineString    // 5
MultiPolygon       // 6
Collection         // 7

@gdey
Copy link
Member

gdey commented May 1, 2020

@gbroccolo yeah, I'm not sure what to do. It does sound like this should be worked on first.

Supporting multiple dimensions could result in breaking geom.Point, (as well as the other types) or maybe create new types called geom.PointZ, geom.PointM, geom.PointZM?

@ARolek
Copy link
Member Author

ARolek commented May 1, 2020

@gbroccolo apologies for the slow response here, I have been trying to think through a way forward. You bring up an excellent point here: the geom package does not support Z, M or S and therefore we can't really drop the Z dimension in this issue. I think this conversation belongs more in the geom package, as the intent here is to have tegola use the geom package for all the geometry processing.

This considered, the geom package was originally intended to be a 2D only package, but I'm now thinking we need to start supporting Z and M. Talking with @gdey he agrees, so maybe we can start working on the data types to support this.

Maybe it would be quicker to talk through this in the Go-Spatial slack workspace for geom: https://slack.go-spatial.org.

@gbroccolo
Copy link

Hi @ARolek ,

I tried to connect to the slack workspace, clicking the link, but it didn't work. Should I maybe be invited in some way?

About the rest: I started with introducing new geometry types like PointZ, PointM, PointS, etc. it's a big piece of work, so I'd be happy to discuss.

@ARolek
Copy link
Member Author

ARolek commented May 4, 2020

@gbroccolo looks like I messed up the protocol, try this one: http://slack.go-spatial.org

I'm hoping we can figure out a vector to attack with the new data types so you don't have to make them work everywhere right off the bat. That's indeed a good deal of work.

I'm looking forward to chatting more!

@polastre
Copy link

polastre commented Jul 8, 2020

I also spent hours debugging why Tegola wasn't rendering the data returned by postgis and instead returning empty tiles. Through sheer luck I found this issue. After using ST_Force2D, my tiles render. I fully agree that silently dropping features with Z dimensions is very frustrating, and should at least warrant a log entry.

@ARolek
Copy link
Member Author

ARolek commented Jul 9, 2020

@polastre yes I agree, its the worst kind of bug too (silent). @gbroccolo has been working on the extended geometry types so we can drop the Z value correctly. Let me see if we can get a log in there for the time being as we're getting closer to releasing a v0.12.0 version.

@ARolek
Copy link
Member Author

ARolek commented Aug 10, 2020

We have added a log when the postgis provider encounters an UnknownGeometryType (previously it was just a continue) to the v0.12.x if you would like to give it a try. @gbroccolo has been adding the enhanced geometry types to the geom package, so once that's done we can finalize this issue by dropping the z dimension, but in the meantime, the log will help identify the issue rather than silently drop the geometries.

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

Successfully merging a pull request may close this issue.

5 participants