Opened 7 years ago

Closed 5 years ago

#2395 closed defect (fixed)

Cannot read KML with non-closed LinearRing

Reported by: strk Owned by: pramsey
Priority: medium Milestone: PostGIS 2.2.0
Component: postgis Version: 2.0.x
Keywords: kml Cc:


According to the specs you're supposed to close LinearRing? coordinates, but FusionTable? doesn't care about exporting unclosed ones. Do we want to be unable to import an export from FT ? I think we don't. We could raise a WARNING about it not being closed but continue to read it. What do you think ?

Change History (11)

comment:1 Changed 7 years ago by jatorre

Sorry, this is not entirely correct. Fusion Tables does not export incorrectly. I think that the source was wrong when uploaded to FusionTables? but FusionTables? is less picky about these errors. And when you export it does not change what you have uploaded.

I still think PostGIS is a bit picky here.

comment:2 Changed 7 years ago by robe

Milestone: PostGIS 2.1.0PostGIS 2.2.0

Pushing to 2.2.0.

If we decide to change this behavior, I think it will have to go into 2.2.0 since I'm planning to release 2.1 soon and can't deal with behavior changes so late in the game.

comment:3 Changed 7 years ago by pramsey

I'd be prepared to see this fix in a 2.1 patch release. Or maybe not, we throw up on invalid WKT:

postgis21=# select 'POLYGON((0 0, 1 0, 1 1, 0 1))'::geometry;
ERROR:  geometry contains non-closed rings
LINE 1: select 'POLYGON((0 0, 1 0, 1 1, 0 1))'::geometry;

Another GUC waiting to be born: SET postgis_autofix=yes

comment:4 Changed 7 years ago by robe

Well I am planning to release 2.1 tomorrow (more likely when the clock strikes midnight on the East Coast of US. :)

2.1 is at feature freeze and adding another GUC sounds like a new feature. So I'm afraid, the answer is that this must wait til 2.2 unless you can convince us this is not a new feature.

comment:5 Changed 7 years ago by robe

I think throwing up an invalid WKT will be fine for 2.1 patch release, but that said we need to separate the GUCy of this ticket from the error throwing that is 2.1 compatible.

comment:6 Changed 7 years ago by strk

I'd default to accept (not fixing) those kind of broken things. They'd be fixable with ST_MakeValid and they'd be reported as invalid by ST_IsValid. Same story with WKT input.

I wouldn't mind switching to the new "accept" policy within 2.1 and even 2.0.

comment:7 Changed 7 years ago by pramsey

Accepting structurally invalid WKT is a massive behavior change, -10 on allowing it into 2.1, and start the thread on -devel about allowing it into trunk, so we can bash away on it.

comment:8 Changed 7 years ago by robe

-10 for me too. But I can be persuaded for 2.2 if strk feels strongly about it and voices it on devel.

comment:9 Changed 7 years ago by strk

Ok let's keep this one for KML, a thread for WKT is already started in the mailing list. Started on postgis-users by a user who tried to import shapefiles from GSHHS:

comment:10 Changed 5 years ago by pramsey

Type: enhancementdefect

comment:11 Changed 5 years ago by pramsey

Resolution: fixed
Status: newclosed

Done in r13834

Note: See TracTickets for help on using tickets.