Coverage Simplification, Validation, Union

[pramsey]

Adds three new functions,

• ST_CoverageIsValid(geom geometry winset, tolerance float8)
• ST_CoverageSimplify(geom geometry winset, tolerance float8, simplifyBoundary integer)
• ST_CoverageUnion(geom geometry set)

For the purpose of these functions a "coverage" is a set of polygons with perfectly shared edges (no overlaps or gaps and exact vertex matching).

[robe2]

@pramsey your documentation is missing mention of minimum required GEOS.

[robe2]

Garden crash:

SELECT ST_CoverageIsValid( ST_GeomFromText('MULTIPOLYGON (((9 9, 9 1, 1 1, 2 4, 7 7, 9 9)), EMPTY)', 4326)) OVER(ORDER BY random());

[dr-jts]

Garden crash:

SELECT ST_CoverageIsValid( ST_GeomFromText('MULTIPOLYGON (((9 9, 9 1, 1 1, 2 4, 7 7, 9 9)), EMPTY)', 4326)) OVER(ORDER BY random());

@robe2 yes, that's a known issue. I'm in the process of fixing it.

[robe2]

@pramsey if you want to commit, I'm fine with that. GHA latest is passing now after the build.

The berries will fail at their garden test step until the crash issue is fixed in GEOS.

[robe2]

Am I allowed to be petty @pramsey ?

The ST_CoverageIsValid doesn't sound like it should return a geometry. The function seems misnamed.

Maybe it should be named ST_CoverageInvalidElements

[dr-jts]

The ST_CoverageIsValid doesn't sound like it should return a geometry. The function seems misnamed.

Agreed, this sounds like it should return a boolean.

How about ST_CoverageInvalidLocation(s)? I tend towards the plural form, but either is good.

Aside: we have kicked around the idea of an aggregate function returning a boolean, to report whether a set of polygonal geometries forms a valid coverage. ST_CoverageIsValid would be a good name for this function. We haven't proceeded on this because it is really just a wrapper over ST_CoverageInvalidLocations, and doesn't return as much information. But maybe it will become apparent that it is useful in the future.

[robe2]

The ST_CoverageIsValid doesn't sound like it should return a geometry. The function seems misnamed.

Agreed, this sounds like it should return a boolean.

How about ST_CoverageInvalidLocation(s)? I tend towards the plural form, but either is good.

Aside: we have kicked around the idea of an aggregate function returning a boolean, to report whether a set of polygonal geometries forms a valid coverage. ST_CoverageIsValid would be a good name for this function. We haven't proceeded on this because it is really just a wrapper over ST_CoverageInvalidLocations, and doesn't return as much information. But maybe it will become apparent that it is useful in the future.

I'm 0.52 for non-plural and 0.48 for plural based on a total weighting of 1

[pramsey]

Changed to ST_CoverageInvalidLocations

[robe2]

@pramsey I rebuilt the docker image and reran latest. All tests are passing now. Anything else you are waiting on before you commit?

[dbaston]

Would this make more sense in lwgeom_geos.c ?

[pramsey]

I dunno that the extra layer of indirection is a big win? What's the argument for?

[dbaston]

To be clear, I meant postgis/lwgeom_geos.c. I just noticed that it was in lwgeom_window.c but is not a window function.

[pramsey]

Potato / potato. Could move it but then it would be separate from its coverage friends...