[postgis-tickets] [PostGIS] #2529: Weird behavior in ST_FlipCoordinates

[PostGIS]

#2529: Weird behavior in ST_FlipCoordinates
-----------------------------------------------------+----------------------
 Reporter:  marcelosoaressouza                       |       Owner:  pramsey      
     Type:  defect                                   |      Status:  new          
 Priority:  medium                                   |   Milestone:  PostGIS 2.0.5
Component:  postgis                                  |     Version:  trunk        
 Keywords:  ST_FlipCoordinates, PostGIS, PostgreSQL  |  
-----------------------------------------------------+----------------------

Comment(by pramsey):

 The actual problem is relatively shallow: the flip points code write into
 the database core memory, using the ptarray_set_point4d to directly
 manipulate the underlying coordinate storage. Point arrays since 2.0 are
 outfitted with a flag that provides their read/write status: point arrays
 that sit on top of core storage are supposed to be read only. The flip
 coordinates code line ignores that, and we get this bug.

 However, if you add in a guard, as in the ptarray_rw_patch and run
 regression, you find that lots of other functions, important core
 functions that have been around forever and a day, also do the same thing!
 They write into core memory. The difference seems to be that they use
 PG_DETOAST_DATUM_COPY instead of PG_DETOAST_DATUM when they read their
 inputs. So they get a copy that they can merrily screw around with.

 However, this is where we get into an inconsistency. The point arrays
 read/write flag indicates not only that its OK to alter values in the
 underlying storage memory, it also indicates then when freed, the point
 array should free the underlying storage too. This is incompatible with
 point arrays built on top of database allocated segments. As *both* the
 PG_DETOAST_DATUM_COPY and PG_DETOAST_DATUM geometries are.

 The "correct" fix, which should be applied to 2.2+, would be to update all
 the memory-altering functions to use PG_DETOAST_DATUM and do an explicit
 lwgeom_clone_deep call before they start altering their geometries. Or,
 that functions like lwgeom_transform and lwgeom_flip_coordinates be
 changed to create new geometries as outputs, **not** to alter their
 inputs. (This would also allow us to declare their inputs const, and avoid
 side-effects, always(?) a good thing)

 The interim fix is just to make flip_coordinates use PG_DETOAST_DATUM_COPY
 instead of PG_DETOAST_DATUM.

-- 
Ticket URL: <http://trac.osgeo.org/postgis/ticket/2529#comment:2>
PostGIS <http://trac.osgeo.org/postgis/>
The PostGIS Trac is used for bug, enhancement & task tracking, a user and developer wiki, and a view into the subversion code repository of PostGIS project.