Opened 20 months ago
Closed 9 months ago
#5361 closed defect (fixed)
Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, ST_Dump, and ST_DumpSegments
Reported by: | robe | Owned by: | pramsey |
---|---|---|---|
Priority: | low | Milestone: | PostGIS 3.5.0 |
Component: | postgis | Version: | master |
Keywords: | Cc: |
Description
Someone asked a question on IRC, which caused me some concern.
How do you get the child types of a compound curve?
I was bothered because of the dichotomy of functions that should agree with each other that don't
SELECT ST_Numgeometries(geom), ST_GeometryType(ST_GeometryN(geom,1)), ST_GeometryN(geom,2) FROM ST_GeomFromText( 'COMPOUNDCURVE( (2 2, 2.5 2.5), CIRCULARSTRING(2.5 2.5, 4.5 2.5, 3.5 3.5), (3.5 3.5, 2.5 4.5, 3 5) )') AS f(geom);
Output is:
st_numgeometries | st_geometrytype | st_geometryn ------------------+------------------+-------------- 3 | ST_CompoundCurve | (1 row)
One can argue both sides, that a compoundcurve is no different from a LINESTRING in that it defines a contiguous line of sorts.
But yet both ST_Numgeometries and ST_Dump do not see it that way
SELECT gd.path, ST_GeometryType(gd.geom) FROM ST_GeomFromText( 'COMPOUNDCURVE( (2 2, 2.5 2.5), CIRCULARSTRING(2.5 2.5, 4.5 2.5, 3.5 3.5), (3.5 3.5, 2.5 4.5, 3 5) )') AS f(geom), ST_Dump(f.geom) AS gd;
path | st_geometrytype ------+------------------- {1} | ST_LineString {2} | ST_CircularString {3} | ST_LineString (3 rows)
But then if ST_Dump were not flawed in this one, one could not arrive at the subtypes of a compound curve. What of ST_NumGeometries? Do the specs say anything about how compound curve sub-elements should be counted.
Change History (11)
comment:1 by , 19 months ago
comment:2 by , 19 months ago
I suspect trying to get the others to do less is more likely to break people's workflow than having ST_GeometryN do more or maybe we should do nothing :)
Out of curiosity I tried this on a SQL Server 2017 server.
WITH a AS (SELECT Geometry::STGeomFromText( 'COMPOUNDCURVE( (2 2, 2.5 2.5), CIRCULARSTRING(2.5 2.5, 4.5 2.5, 3.5 3.5), (3.5 3.5, 2.5 4.5, 3 5) )',0) AS geom ) SELECT a.geom.STGeometryN(2), a.geom.STNumGeometries(), a.geom.STNumCurves(), a.geom.STCurveN(2).STAsText() FROM a;
and it returned
NULL, 1, 4 , CIRCULARSTRING (2.5 2.5, 4.5 2.5, 3.5 3.5)
which in my mind looks like the correct way to handle this. But is it worth it?
What I'm mad about is we don't have an ST_NumCurves and ST_CurveN function. How did we live so long without these?
I trashed my Oracle server so not sure what answer that would give. Maybe we just leave it alone since no one has complained except the IRC guy. If they do complain we could introduced a ST_CurveN and ST_NumCurves functions that counts the components in the curve.
comment:3 by , 17 months ago
Milestone: | PostGIS 3.4.0 → PostGIS 3.5.0 |
---|
I'm tableing this for now. Not sure it's worth doing anything about.
comment:4 by , 16 months ago
Summary: | Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, and ST_Dump → Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, and ST_Dump, add support for SQL MM ST_NumCurves and ST_CurveN |
---|
comment:5 by , 15 months ago
So, the offending piece of code is this one
/* call is valid on multi* geoms only */ if (type==POINTTYPE || type==LINETYPE || type==CIRCSTRINGTYPE || type==COMPOUNDTYPE || type==POLYGONTYPE || type==CURVEPOLYTYPE || type==TRIANGLETYPE) { if ( idx == 0 ) PG_RETURN_POINTER(geom); PG_RETURN_NULL(); }
Both COMPOUNDTYPE
and CURVEPOLYTYPE
are, structurally, multi-geometries, so it would be easy to cast them to COLLECTION
hand them off... then a CURVEPOLYGON
would be handled like a nested GEOMETRYCOLLECTION
... the rings would each be a COMPOUND
and each of those would in turn be a set of CURVESTRING
and/or LINESTRING
.
This seems... not entirely bad? And it does match the ST_Dump
behaviour?
comment:6 by , 15 months ago
Can I have a ST_NumCurves and ST_CurveN too.
I think at the very least ST_NumGeometries and ST_GeometryN should at least agree with each other which is what your proposed change would fix at least.
If ST_NumGeometries says there are 3 geometries, then I should be able to do ST_GeometryN(geom, 3) and get something back. That does seem like it would cause less friction than changing the behavior to 1 geometry.
Then again, how many people would be using curves to care how that changes unless everyone gets into the habit now of SVGing their curves now that we support that.
That said, I feel SQL Server is more the expected behavior and their having a ST_NumCurves, ST_CurveN to handle the subelements.
I'm afraid to ask what it does with PolyhedralSurfaces and TINS. I'm pretty sure ST_Dump expands those to polygons and triangles. In theory one can argue are those really any different from multipolygons? To me they are.
What I was thinking of was an extract argument expand_complex=true
SO SELECT ST_NumGeometries(geom) would do as it does now
SELECT ST_GeometryN(geom) -- would actually agree with ST_NumGeometries
But if I did SELECT ST_NumGeometries(geom, expand_complex => false);
SELECT ST_GeometryN(geom, expand_complex => false);
My curved stuff and TIN stuff and PolyhedralSurface stuff would be treated as one unit.
So that way it's backward compatible except for the case where it was broken anyway.
We could add the same arg to ST_Dump(geom, expand_complex => false)
cause yes I have on occasion wanted to expand my TINS and polyhedralsurfaces and curved stuff into the subelements, but many times I want to treat them as a single unit.
comment:7 by , 9 months ago
Summary: | Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, and ST_Dump, add support for SQL MM ST_NumCurves and ST_CurveN → Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, and ST_Dump |
---|
comment:8 by , 9 months ago
Another instance ST_DumpSegments should either say it doesn't work with compound curves, or do something useful
SELECT dp.path, ST_AsText(dp.geom) FROM ST_DumpSegments('COMPOUNDCURVE( (2 2, 2.5 2.5), CIRCULARSTRING(2.5 2.5, 4.5 2.5, 3.5 3.5), (3.5 3.5, 2.5 4.5, 3 5) )'::geometry) AS dp; path | st_astext ------+----------- (0 rows)
Doesn't give an error but returns no rows.
comment:9 by , 9 months ago
Summary: | Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, and ST_Dump → Compound Curve inconsistent behavior with ST_GeometryN, ST_NumGeometries, ST_Dump, and ST_DumpSegments |
---|
comment:10 by , 9 months ago
Some notes from a consistency check. The main question philosophically is whether CompoundCurve and CurvePolygon should be treated as unitary objects (like LineString and Polygon) or as collections. Either interpretation can result in consistent behaviour, I think.
https://gist.github.com/pramsey/cbf6db1edb8762854925f6b6ad6d6b35
Perhaps it is ST_GeometryN that needs the fix...
Yes, a compoundcurve is "one thing", but it could also be considered "three things with some rules" (namely the endpoint-to-endpoint coordinate matching). Linear ring is just a linestring with an extra rule.
So I guess it should be possible to deconstruct a CompoundCurve with ST_GeometryN? Certainly since we can dump it already.