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

Panic when selecting 3D geometry #89

Closed
russss opened this issue Dec 24, 2016 · 14 comments
Closed

Panic when selecting 3D geometry #89

russss opened this issue Dec 24, 2016 · 14 comments
Assignees
Labels
Milestone

Comments

@russss
Copy link

russss commented Dec 24, 2016

When selecting a 3D geometry from a table, tegola crashes with:

2016/12/24 14:13:37 Error Getting MVTLayer: Was unable to decode geometry field(<field>) into wkb for layer <layer>

Using ST_Force_2D in the SQL fixes this but it would be useful if tegola could automatically discard the third dimension.

@ARolek ARolek added the bug label Dec 27, 2016
@ARolek
Copy link
Member

ARolek commented Dec 27, 2016

@russss thanks for the bug report. Is there an open dataset I can use to recreate the problem?

@russss
Copy link
Author

russss commented Dec 27, 2016

I think a minimal test case should be as simple as:

CREATE TABLE test (id SERIAL, geom geometry(PointZ, 3857));
INSERT INTO test(geom) VALUES (ST_GeomFromText('PointZ(1000 1000 10)', 3857));

@ARolek
Copy link
Member

ARolek commented Dec 29, 2016

Even better. Thanks @russss.

@gdey gdey self-assigned this Aug 19, 2017
@stvno
Copy link
Contributor

stvno commented Jun 7, 2018

We've run into this as well, are there any plans to support, or at least not crash on, 3D geometries?

@stvno
Copy link
Contributor

stvno commented Jun 7, 2018

Just checked the code and I realized that tegola does support PointZ, but not the PolygonZ which is what we are using, nor LinestringZ, multilinestringZ and multipolygonZ for that matter

@gdey
Copy link
Member

gdey commented Jun 7, 2018

There is currently no plans to support the Z versions since we have not had a need, but I do plan to make it not crash. This should be an easy fix, where we ignore the z and m components. I just have gotten to it.

@ARolek
Copy link
Member

ARolek commented Jun 7, 2018

@gdey we should not panic on 3D geoms though.

@stvno we're working on cutting a version of tegola this month. Let me see if I can get this fix in.

@ARolek ARolek added this to the v0.7.0 milestone Jun 7, 2018
ARolek added a commit that referenced this issue Jun 19, 2018
@ARolek
Copy link
Member

ARolek commented Jun 21, 2018

@stvno I pushed up a PR that addresses this issue. A warning is now logged if there is an UnknownGeometry type error but no error is returned. Please test if you have some time.

@ARolek
Copy link
Member

ARolek commented Jul 2, 2018

@stvno did you have a chance to try the PR for gracefully handling 3d geoms? I hoping to get some feedback before we merge it into the v0.7.0 branch.

@stvno
Copy link
Contributor

stvno commented Jul 3, 2018

I'm sorry, I missed this one. I've some other issues to attend to first, but I'll try it when I have a chance and let you know.

@stvno
Copy link
Contributor

stvno commented Jul 3, 2018

Alright I have tested it with a PolygonZ and tegola keeps running:

config.toml:

 [[providers.layers]]
    name = "buildings"
	geometry_fieldname = "geovlak_3857"
	srid = 3857
	id_fieldname = "gid"
        sql =  "SELECT gid, ST_AsBinary(geovlak_3857) AS geovlak_3857 FROM bag.pand WHERE geovlak_3857 && !BBOX!"

PostGIS table:

CREATE TABLE bag.pand
(
    gid integer NOT NULL DEFAULT nextval('bag.pand_gid_seq'::regclass),
    identificatie character varying(16) COLLATE pg_catalog."default",
    aanduidingrecordinactief boolean,
    aanduidingrecordcorrectie integer,
    officieel boolean,
    inonderzoek boolean,
    begindatumtijdvakgeldigheid timestamp with time zone,
    einddatumtijdvakgeldigheid timestamp with time zone,
    documentnummer character varying(20) COLLATE pg_catalog."default",
    documentdatum date,
    pandstatus bag.pandstatus,
    bouwjaar numeric(4,0),
    geom_valid boolean,
    geovlak geometry(PolygonZ,28992),
    geovlak_3857 geometry(PolygonZ,3857),
    geometry_vlak_3857 geometry(Polygon,3857),
    CONSTRAINT pand_pkey PRIMARY KEY (gid)
)

And the commandline output generated by Tegola:

2018/07/03 11:46:32 postgis.go:509: unknown geometry type (1003) for layer (buildings) with geometry field (geovlak_3857) where (gid = 4902339), skipping

Still it would be nice if it would support 3D featuretypes, even when they are just rendered in 2D (as you can see I've added an extra column to the table which contains the 2D geometry for now)

@ARolek
Copy link
Member

ARolek commented Jul 3, 2018

@stvno our first idea was to drop the 3rd dimension but WKB does not support more than 2 dimensions. PostGIS has an extended form of WKB known as EWKB. We don't currently have a decoder for EWKB, though this would be a great addition to the geom package. If an EWKB encoder / decoder was authored It would make sense to swap out the WKB reader in the PostGIS driver with the EWKB one and then we could properly drop Z values.

We should open two new issues for this, one on the the geom package and one on tegola that depends on the geom package issue.

@stvno
Copy link
Contributor

stvno commented Jul 3, 2018

@ARolek seems like a sensible approach. I wasn't aware that WKB didn't support the 3rd dimension. I'd vote to close this issue as in it doesn't crash anymore and indeed create two issues to track the EKWB support.

@ARolek
Copy link
Member

ARolek commented Jul 3, 2018

Perfect!

@ARolek ARolek closed this as completed Jul 3, 2018
ARolek added a commit that referenced this issue Jul 3, 2018
* gracefully handle 3d geometries. closes #89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants