ST_CollectionHomogenize sometimes returns reference to argument

[dbaston]

I noticed this situation when digging into a question posted on Stack Exchange: ​http://gis.stackexchange.com/questions/195915/weird-behavior-with-st-clusterwithin

That question itself isn't relevant here, but the some of the test data can be used to make a reproducible (for me) test case of the problem:

CREATE TABLE cluster_inputs (
    name text,
    geom geometry
);

CREATE TABLE cluster (
    geom geometry
);

INSERT INTO cluster (geom) VALUES ('010700002031BF0D000100000001030000000100000005000000713D0AD76D4D11C1A4703D9AE5955941713D0AD76D4D11C1A4703D9A94965941713D0AD77D4211C1A4703D9A94965941713D0AD77D4211C1A4703D9AE5955941713D0AD76D4D11C1A4703D9AE5955941');
INSERT INTO cluster (geom) VALUES ('010700002031BF0D00010000000103000000010000000D0000009A9999998B3211C114AE47F13D965941295C8FC2863211C1A4703D5A569659418FC2F528D33011C1EC51B84E5A9659417B14AE47B62F11C1B81E851B5A965941295C8FC2BC2F11C18FC2F5F85D96594114AE47E10D2C11C1295C8FE26F965941EC51B81EF32B11C1666666D6699659417B14AE47D12C11C1F6285CFF139659418FC2F528BA2F11C1F6285C7F14965941B81E85EBBE2F11C11F85EB01279659415C8FC2F5DB2F11C1D7A3702D2C965941F6285C8F2F3011C15C8FC235309659419A9999998B3211C114AE47F13D965941');


INSERT INTO cluster_inputs (name, geom) VALUES ('a', '010300002031BF0D000100000005000000713D0AD76D4D11C1A4703D9AE5955941713D0AD76D4D11C1A4703D9A94965941713D0AD77D4211C1A4703D9A94965941713D0AD77D4211C1A4703D9AE5955941713D0AD76D4D11C1A4703D9AE5955941');
INSERT INTO cluster_inputs (name, geom) VALUES ('b', '010300002031BF0D00010000000D0000009A9999998B3211C114AE47F13D965941295C8FC2863211C1A4703D5A569659418FC2F528D33011C1EC51B84E5A9659417B14AE47B62F11C1B81E851B5A965941295C8FC2BC2F11C18FC2F5F85D96594114AE47E10D2C11C1295C8FE26F965941EC51B81EF32B11C1666666D6699659417B14AE47D12C11C1F6285CFF139659418FC2F528BA2F11C1F6285C7F14965941B81E85EBBE2F11C11F85EB01279659415C8FC2F5DB2F11C1D7A3702D2C965941F6285C8F2F3011C15C8FC235309659419A9999998B3211C114AE47F13D965941');

For me, running the following query then produces an error:

SELECT
  cluster_inputs.name, 
  ST_Intersects(ST_CollectionHomogenize(cluster.geom), cluster_inputs.geom)
FROM cluster_inputs, cluster
ORDER BY name;

ERROR:  First argument geometry could not be converted to GEOS: IllegalArgumentException: Points of LinearRing do not form a closed linestring

which suggests a memory issue. The problem can be resolved by wrapping the return of lwgeom_homogenize with an lwgeom_clone_deep, which seems to confirm this. Since the return of lwgeom_homogenize references portions of its argument, and ST_CollectionHomogenize releases the serialized input before serializing the output, we seem to have a problem.

[mwtoews]

Here's a simplified version of the issue:

WITH cluster AS (
  SELECT ST_Polygonize(ST_MakeEnvelope(1, 2, 3, 4)) AS geom
)
SELECT ST_GeometryType(geom),
  ST_CollectionHomogenize(geom)::box2d,
  ST_CollectionHomogenize(geom::text)::box2d
FROM cluster;

-[ RECORD 1 ]-----------+----------------------
st_geometrytype         | ST_GeometryCollection
st_collectionhomogenize | BOX(0 0,3 4)
st_collectionhomogenize | BOX(1 2,3 4)

The error is shown from the second field, where the X and Y coords are 0. A workaround is shown in the last example, where the geometry is recreated.

[robe]

Okay this is a problem in both 2.2 and 2.3.

Does anyone have issue with applying dbaston's suggestion. If not I'm going to go ahead and do that.

dbaston,

I'd feel better if you did it now that you have commit rights .

[pramsey]

Fixing homogenize to return a full copy seems preferable to deep cloning the return from homogenize, ISTM.

[dbaston]

Replying to pramsey:

Fixing homogenize to return a full copy seems preferable to deep cloning the return from homogenize, ISTM.

Agreed. I don't think we actually need a clone though, I think it's sufficient to just avoid freeing our input GSERIALIZED before we've serialized our output. If there are no concerns with the attached patch that does this, I'll commit it.

[pramsey]

Sure. Maybe add a TODO comment in the code indicating we're guarding against this problem and should maybe change the liblwgeom contract.

[dbaston]

Fixed in trunk at r15105 and 2.2 at r15107.