Skip to content

Conversation

tbbuck
Copy link
Contributor

@tbbuck tbbuck commented Sep 19, 2025

Hi there! I hope you've been keeping well since I last dropped a PR on you out of the blue :) Amazing work on the recent Spatial updates lately btw, I've been constantly blown away by the table join join performance we now have at our fingertips thanks to you :-D

This PR implements 3 functions for the CoverageClean functionality recently added to GEOS:

  • GEOMETRY ST_CoverageClean (geoms GEOMETRY[], snapDistance DOUBLE, maxWidth DOUBLE)
  • GEOMETRY ST_CoverageClean (geoms GEOMETRY[], snapDistance DOUBLE)
  • GEOMETRY ST_CoverageClean (geoms GEOMETRY[])

Somewhat bizarrely, I've been unable to replicate the CoverageCleanerTest.cpp tests from libgeos with either this of extension or the geosop tool. The extension code does align with output from geosop though.

Future nice-to-haves:

  • Support OverlapMergeStrategy: I couldn't work out how to implement a 4-parameter function call in the same style as the other GEOS functions. I also got a bit concerned about unintended consequences of using enums from other packages.
  • Improve my VCPKG version knowledge. I've hacked in the version change as an override to vcpkg.json - feels like a nasty bodge job.
  • Implement aggregate ST_CoverageClean_Agg() versions of the above.

I'm very happy to work on any or all of the above with you btw, if you can share any guidance with me I'm all ears on making this function fly.

Thanks again!

@tbbuck tbbuck force-pushed the coverage-cleaner-geos-v3.14.0 branch from ea09e61 to 3311421 Compare September 19, 2025 23:41
@Maxxen
Copy link
Member

Maxxen commented Sep 24, 2025

Hello! Thanks for the PR!

This is currently failing because the VCPKG baseline DuckDB's CI build system uses doesn't include the latest GEOS version. I'll see if we can have a look at bumping it soon at which point this PR should be able to pass CI.

@tbbuck
Copy link
Contributor Author

tbbuck commented Sep 25, 2025

Hello! Ahhhh that makes sense - I was struggling to figure out what was going wrong where, thanks for signposting it :) Is there anything I can do on that side that means less work for you?

For accepting an additional 4th parameter, I've just realised that I could call SenaryExecutor and ignore the extra columns (i.e. no need to find or implement a QuateraryExecutor). Do you think that's a good way for me to add an extra parameter here?

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants