feat: initial MVT ST_TileEnvelope implementation

[bradh]

This is similar to the PostGIS ST_TileEnvelope function, but only supports the standard (EPSG:3857) geometry, and does not support additional margins. See https://postgis.net/docs/ST_TileEnvelope.html

There is extensive testing of zoom levels and tile indices, mainly generated by a script that uses PostGIS as a test oracle. The generator script is included.

There are a lot of tests. On my machine, those tests take about 3 minutes. Possibly those could be _slow.test.

[bradh]

Note that this isn't very useful for MVT without https://postgis.net/docs/ST_AsMVTGeom.html and https://postgis.net/docs/ST_AsMVT.html equivalent implementations, but I'll get to those next.

[Maxxen]

Hello! Thanks for the PR!

Code looks good, but Im not sure about the testing script. We can't include code from PostGIS as it's licensed under GPL-V2. I also don't think that including 40k+ lines of tests is proportional for this function - perhaps just writing a couple of unitt-tests that check e.g. min/max zoom levels, invalid parameter errors, or that e.g. a tile at zoom 5 st_contans a tile at zoom 6, would be enough?

It's cool that you're interested in adding the other function required for creating vector tiles. I've made an attempt at this previously in this old branch. Ive been thinking of revisiting it sometime soon but it might provide more useful to you.

The hard part is going to be clipping the geometries to the tile boundaries. You can use GEOS for it, but as a result of clipping you will end up with degenerate cases that GEOS will reject (e.g. line-strings with a single point), or other inconsistencies that is going to make you want to support extra buffering in the ST_AsMVTGeom function. Additionally, while somewhat less important, round-tripping everything back and forth through GEOS will be slow, and the contention on memory allocation that GEOS introduces will limit the effectiveness of DuckDBs parallelism. Therefore my plan was to first try to implement a clipping routine natively in SGL, probably some extension of Greiner-Hormann and then give ST_AsMVT and friends another shot.

[bradh]

Code looks good, but Im not sure about the testing script. We can't include code from PostGIS as it's licensed under GPL-V2.

The testing script isn't from PostGIS. It just calls psql to get the answers that PostGIS would return for the same query.

I also don't think that including 40k+ lines of tests is proportional for this function - perhaps just writing a couple of unitt-tests that check e.g. min/max zoom levels, invalid parameter errors, or that e.g. a tile at zoom 5 st_contans a tile at zoom 6, would be enough?

OK, I like tests...

Will trim the tests.

It's cool that you're interested in adding the other function required for creating vector tiles. I've made an attempt at this previously in this old branch. Ive been thinking of revisiting it sometime soon but it might provide more useful to you.

Thanks. Looks very useful.

The hard part is going to be clipping the geometries to the tile boundaries. You can use GEOS for it, but as a result of clipping you will end up with degenerate cases that GEOS will reject (e.g. line-strings with a single point), or other inconsistencies that is going to make you want to support extra buffering in the ST_AsMVTGeom function. Additionally, while somewhat less important, round-tripping everything back and forth through GEOS will be slow, and the contention on memory allocation that GEOS introduces will limit the effectiveness of DuckDBs parallelism. Therefore my plan was to first try to implement a clipping routine natively in SGL, probably some extension of Greiner-Hormann and then give ST_AsMVT and friends another shot.

For my use, points are essential, linestrings are important, and anything else doesn't really matter. So maybe the implementation could check points, clip linestrings, and bail to GEOS for anything more complicated. When a native SGL clipping routine is available, that could be swapped out.

There is already an extra buffer in the ST_AsMVTGeom API used by PostGIS, but degenerate geometry may still be a problem. Possibly that could be a check in SGL.

[bradh]

Also it looks like the tests are failing for something unrelated to the change I'm proposing here.

[yutannihilation]

I hit the same failure. It seems it's better to set the base branch to v1.3.0 instead of main.

#593 (comment)

[bradh]

I've cut down the tests, and incorporated some of the implementation from your version of ST_TileEnvelope on that branch you pointed to, since that seemed cleaner.

[Maxxen]

Cool! Im retargeting this to v1.3.0, there's been some changes in SGL, e.g. box_xy is now called extent_xy, but otherwise this looks good to go.

[bradh]

Updated to reflect v1.3.0. Will see if the CI is any happier...

[bradh]

The windows test failure looks unrelated to the change I'm making here.

[Maxxen]

Thanks! The windows fail is indeed unrelated, it should hopefully go away on its own soon once we reenable vcpkg caching