geometry to geography cast, always adds a box even for 2 point linestrings and single points, but geography from text or binary do not

[robe]

I noticed this when fixing #3355 that geometry to geography cast always adds a bounding box. I had actually copied the solution from that function to fix #3355. For ST_Segmentize it's probably a rare case you'll segmentize and still end up with a 2 point linestring, but I can see this as being more of an issue when casting between geometry /geography.

Observe:

SELECT ST_Summary(wkt::geometry) As wkt_geom, 
   ST_Summary(wkt::geography) As wkt_geog,
   ST_Summary(wkt::geometry::geography) As geom_geog

FROM (SELECT 
     'LINESTRING(124.983539 1.419224,91.181596 29.647798)'::text As wkt) As f;

outputs:

          wkt_geom          |           wkt_geog           |           geom_geog
----------------------------+------------------------------+-------------------------------
 LineString[] with 2 points | LineString[GS] with 2 points | LineString[BGS] with 2 points
(1 row)

[robe]

(In [14350]) Don't add unnecessary boxes during geography casting references #3356 (fix for 2.3 branch) references #3355 redo of fix for 2.3 branch for segmentize

[robe]

pramsey want to check my work on this to make sure I did the right thing. I changed to use both for geography_from_geometry and geography_segmentize

 gserialized_geography_from_lwgeom(lwgeom, -1);

so it behaves just like when you cast from text,

instead of that long painful list of add/drop/set_geodetic.

If it looks okay to you I will backport to 2.2. Not sure it makes sense for 2.1.

[robe]

okay this sucks. It seems something in KNN recheck requires gboxes on points as debbie's 9.5 build failed when I committed, oh but a tangled web I weave when I try to understand. travis failed too though travis seems to just gone completely crazy. strk yes I did test locally before committing, but only on 9.4.

[robe]

(In [14351]) make sure to drop whatever boxes first references #3356 (fix for 2.3 branch) references #3355 redo of fix for 2.3 branch for segmentize

[robe]

(In [14352]) cleanup whitespace and comments references #3356 (for 2.3 branch) references #3355 (for 2.3 branch for segmentize)

[robe]

okay debbie is happy again (guess the gserialized_geography_from_lwgeom is not smart enough to drop a box on point/2 point linestring maybe.. travis is furious and I don't know why. Must be memory leak somewhere.

[pramsey]

No, you don't want to strip the boxes from teh geographies, there's a big difference between geometry and geography boxes, because for geometry 2-point boxes are trivial (just read first and last point) but for geographies they aren't (you have to check the range of the 3-space box, because sometimes the extrema are not the end points. Similarly (though less bad) even for points, because the box in geography is 3-space while the point is only 2d, the box requires more calculation, so having it pre-calculated and on the object is not a bad thing.

[robe]

okay so in that case, we should change the geography from text function, because it doesn't add boxes for 2 linestrings and points.

[robe]

aha - found it - add box is happening via geography_serialize via the gserialized_from_lwgeom call. So it seems our change to not add a box to point and 2 point linestring for geometry inadvertently affected geography_from_text and geography_from_binary

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

shouldn't that be changed then to also consider if a geodetic flag is set and just do the right thing or higher up in food chain:

​http://postgis.net/docs/doxygen/2.3/da/de7/liblwgeom_8h_a6c0c2003b8cb0f5c5c272448f9a945c4.html#a6c0c2003b8cb0f5c5c272448f9a945c4

change lwgeom_needs_bbox so it always returns true if geodetic bit is set to true?

[pramsey]

At r14359 I've put in a test for geodetic pretty deep in the serialization logic. Basically all geographies get boxes always. I'm not 100% sure of this, but it makes a fair amount of sense, since you cannot just read the boxes of geographies off the raw coordinates, so there's always calculation involved and the information should be cached.

[pramsey]

And now I've taken it back out. There is a price to pay for not having those boxes at index build time, but after that, mostly we can afford to pay the cost of calculating them on the fly and the payback in storage space is worth it.

[robe]

(In [14362]) back to using geography_gserialized, but don't add box (gserialized does that) (but we must still set geodetic for some ungodly reason) references #3355 references #3356

[pramsey]

I would like to close this as I cannot see the action needed or defect to be resolved anymore.