ST_ConcaveHull 2.1 regress failure compared to 2.0 (underlying issue st_dumppoints)

[robe]

This worked in 2.0.3

SELECT ST_ConcaveHull('SRID=4326;MULTIPOINTM(-71.0821 42.3036 1,-71.0821 67.3036 1,-56.0821 42.3036 1,-56.0821 67.3036 1,-41.0821 42.3036 1,-41.0821 67.3036 1,-26.0821 42.3036 1,-26.0821 67.3036 1,-11.0821 42.3036 1,-11.0821 67.3036 1,3.9179 42.3036 1,3.9179 67.3036 1,18.9179 42.3036 1,18.9179 67.3036 1,33.9179 42.3036 1,33.9179 67.3036 1,48.9179 42.3036 1,48.9179 67.3036 1,-71.0821 42.3036 2,-71.0821 67.3036 2,-56.0821 42.3036 2,-56.0821 67.3036 2,-41.0821 42.3036 2,-41.0821 67.3036 2,-26.0821 42.3036 2,-26.0821 67.3036 2,-11.0821 42.3036 2,-11.0821 67.3036 2,3.9179 42.3036 2,3.9179 67.3036 2,18.9179 42.3036 2,18.9179 67.3036 2,33.9179 42.3036 2,33.9179 67.3036 2,48.9179 42.3036 2,48.9179 67.3036 2)'::geometry
, 0.5,false)

and gave answer:

-- The ST_AsEWKT output 
-- PostgreSQL 9.2.4, compiled by Visual C++ build 1600, 32-bit POSTGIS="2.0.3 r11132" GEOS="3.5.0dev-CAPI-1.9.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.10.0, released 2013/04/24" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

SRID=4326;POLYGON((-71.0821 42.3036,-71.0821 67.3036,48.9179 67.3036,48.9179 42.3036,-71.0821 42.3036))

Which makes sense if we drop the M as I would expect it to.

In 2.1.0 branch

-- PostgreSQL 9.2.4, compiled by Visual C++ build 1600, 32-bit POSTGIS="2.1.0rc3 r11766" GEOS="3.5.0dev-CAPI-1.9.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.10.0, released 2013/04/24" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER


ERROR:  Operation on mixed SRID geometries
CONTEXT:  PL/pgSQL function st_concavehull(geometry,double precision,boolean) line 63 at assignment
********** Error **********

This could be a bug in my code I suppose that it might be missing resetting the SRID but why it worked in 2.0.3 and not in 2.1.0 is a bit troubling.

[nicklas]

In trunk version something happens when there is more than 3 points.

SELECT st_astext(ST_ConcaveHull(
'SRID=1;MULTIPOINT(1 1, 2 2, 2 1)'::geometry,
 0.5,false));

gives the same answer in both 2.0 and trunk:

POLYGON((1.004 1.004,1.996 1.004,1.996 1.996,2 2,2 1,1 1,1.004 1.004))

but

SELECT st_astext(ST_ConcaveHull(
'SRID=1;MULTIPOINT(1 1, 2 2, 2 1, 2 0)'::geometry,
 0.5,false))

gives the "mixed SRID" error in trunk. In 2.0 it returns:

POLYGON((2 0,1 1,2 2,2 0))

which doesn't seem to "shrink" the polygon like the 3-point answer does.

[pramsey]

Sorry, no help from me. Clearly some function somewhere is dropping SRID, but I don't see any obvious candidates in the PL/PgSQL and unrolling that amount of Pl/PgSQL is beyond me. The fact that it's only failing for npoints > 3 might be useful in finding the issue, though who knows: why do you get differences between 2.0 and 2.1 on the result for 3 point lines?

Maybe this requires the Mother of All bisections to find the change point wherein the result of this function changed.

[pramsey]

I ran the mother of all bisections and the problem stems from the C revision of ST_DumpPoints, at r10595. Prior that that revision it worked, after it, it failed with the SRID problem.

[pramsey]

And there's the answer, compare this query between 2.0 and 2.1:

select st_srid((st_dumppoints('SRID=4326;MULTIPOINT(0 0,1 1,2 2,3 3)'::geometry)).geom);

2.0 returns 4326's, and 2.1 returns 0's.

[pramsey]

I have committed a fix at r12049.

Please test this well, it's a very deep change that makes practical sense (ensuring that collections have their SRID set all the way down) but was not implemented in the 2.0 transition (by mistake, maybe?) because in 1.5 doing so would have had deleterious effects on the WKB/WKT deserializer. It might have been forgotten.

In any event, making the change breaks no regressions except the meaningless one in ST_Summary, so I'm OK with it. If it lasts w/o ill effect in trunk for a while, I'll put it back into 2.1

[robe]

doing a garden regress between 2.1.1 dev and 2.2.0 dev I see 2 key differences --

all the ST_DumpPoints outputs for multitypes have changed to include SRID in, dump which is expected and desired.

I'm also seeing a change in ST_ExteriorRing for curvepolygon. That in 2.1.1dev does not output the SRID but does in 2.2. Is that expected by this change. I would assume we'd want the same in 2.1.

Output for this query

SELECT ST_AsEWKT(ST_ExteriorRing(foo1.the_geom)) As result
  FROM ((SELECT ST_GeomFromEWKT('SRID=4326;CURVEPOLYGON(CIRCULARSTRING(-71.0821 42.3036, -71.4821 42.3036, -71.7821 42.7036, -71.0821 42.7036, -71.0821 42.3036),(-71.1821 42.4036, -71.3821 42.6036, -71.3821 42.4036, -71.1821 42.4036) ) ') As the_geom)) As foo1 LIMIT 3;

2.2 yields:

SRID=4326;CIRCULARSTRING(-71.0821 42.3036,-71.4821 42.3036,-71.7821 42.7036,-71.0821 42.7036,-71.0821 42.3036)

But 2.1 yields

CIRCULARSTRING(-71.0821 42.3036,-71.4821 42.3036,-71.7821 42.7036,-71.0821 42.7036,-71.0821 42.3036)

[robe]

I checked my postgis 2.0.4 install, and 2.0 returns the same answer as 2.1 (no srid), but that seems more like a bug that should be fixed than anything so I would say go for the commit to 2.1. My concave regress tests that were failing in garden are now passing.

[pramsey]

Everything you describe seems like a "good side effect" of propagating SRID down into the sub-geometries of collections, and you haven't found any bad side effects yet, so that's great. Putting this into 2.1 and maybe 2.0 is indicated if the trunk experience continues to be positive.

[robe]

So you gonna go ahead and backport to 2.1 so we can close this out?

[pramsey]

Ok, ported back to 2.1 at r12085

We could push all the way back to 2.0, but since nobody has reported any issues w/ the current state, probably best to just leave well enough alone.

[robe]

yah I think leave well enough alone especially since 2.0 behaved right for everything except exterior ring as far as I could tell.