Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3361 closed defect (fixed)

gserialized_from_lwgeom takes is_geodetic arg but does nothing with it, geography_serialize may give you a geometry if not careful

Reported by: robe Owned by: robe
Priority: medium Milestone: PostGIS 2.3.0
Component: postgis Version: master
Keywords: Cc:

Description (last modified by robe)

This is not so much a bug as a change the code to keep my sanity and so that other developers don't fall into a wtf trap.

In hopping threw the wonderland of gserialized I ended up in the rabbit hole of

gserialized_from_lwgeom(LWGEOM * geom,int is_geodetic, size_t * size )

in g_serialized c.

http://postgis.net/docs/doxygen/2.3/da/d21/g__serialized_8c_a0b9fd37dce2089c4fecb8a23ed102323.html#a0b9fd37dce2089c4fecb8a23ed102323

All roads from geometry and geography eventually lead to this function, and I was puzzled why every call would both do a

lwgeom_set_geodetic(lwgeom, true);

and then still pass along this is_geodetic bit.

On closer inspection — that is_geodetic bit is never used, just always ignored like some vestigial cecum that turned into a useless appendix. In fact the internal bit in lwgeom is always used, so we should just get rid of this argument.

I should add — geography_serialize(lwgeom); I thought would be smart enough to set the geodetic bit too from the name of the function as isn't the point to get a geography serialized out of an lwgeom, nope. If you feed in an lwgeom that is not geodetic, you end up with a gserialized that is not geodetic also.

Anyway I'd like to change at the very least that gserialized_from_lwgeom and changed geography_serialize to guarantee always getting back a geography. But I would like to get some opinions before I do that. Of course I would only do this in 2.3 and not backport.

Change History (5)

comment:1 by robe, 8 years ago

Description: modified (diff)

comment:2 by robe, 8 years ago

Owner: changed from pramsey to robe

comment:3 by robe, 8 years ago

In going thru this exercise, I came across this function -

http://postgis.net/docs/doxygen/2.3/dc/d52/geography__measurement_8c_a7990577ab258de83502c7fb56ebfc202.html#a7990577ab258de83502c7fb56ebfc202

geography_point_outside

Which if I am not mistaken will end up returning a geometry (not a geography), because of the bogusness of the gserialized_from_lwgeom (since it's passing a geometry), presumably it will get a geometry back.

Do we use this function anywhere?

comment:4 by robe, 8 years ago

Okay got my pull request ready. https://github.com/postgis/postgis/pull/72

Can you guys look over and see if you find issue. My local tests passed after the change.

comment:5 by robe, 8 years ago

Resolution: fixed
Status: newclosed

changed for 2.3 at r14453

Last edited 8 years ago by robe (previous) (diff)
Note: See TracTickets for help on using tickets.