Skip to content

Conversation

@JoelJaeschke
Copy link

This commit addresses #583 and adds ST_Subdivide, similar to how PostGIS does it. The algorithm does not reproduce PostGIS exactly, however. Every type of input geometry is accepted, except for GeometryCollections. The result is a GeometryCollection of subdivided geometry parts. The user can specify the maximum number of vertices that every part should have.

Slight additions to GeosGeometry were necessary, namely:

  • Initialize new geometry from bounding rectangle
  • Expose some more of the GEOS geometry's attributes
  • Add a function that counts number of vertices of a geometry

---
This commit adds ST_Subdivide, similar to how PostGIS does it. The
algorithm does not reproduce PostGIS exactly, however. Every type
of input geometry is accepted, except for GeometryCollections. The
result is a GeometryCollection of subdivided geometry parts. The
user can specify the maximum number of vertices that every part
should have.
@JoelJaeschke
Copy link
Author

Hey @Maxxen could you give me a pointer on why CI failed? This does not look like anything I changed, so I am a bit confused whether I caused that. Also, should I merge into main or the v1.4 branch? Thanks!

@Maxxen
Copy link
Member

Maxxen commented Jan 9, 2026

Hey, sorry this got buried in my backlog. Im gonna try to review this soon in the coming days.

The main branch sometimes lags behind upstream duckdb main which is used in the CI workflow and so these sort of unrelated errors can surface. Generally v1.4-andium branch is what will be used to build spatial for the v1.4-series of patch releases, but main is used for the next upcoming minor release (v1.5 in this case), so I guess it depends if your PR needs to make use of upcoming features or if you want it in the next patch version.

But now v1.4 is so close to release anyway I would just keep it on main. I think it should be enough to rebase otherwise I can have a look and try to fix-up this PR later so it can pass CI.

The algorithm does not reproduce PostGIS exactly, however.

Exact PostGIS behavior is not strictly a requirement, but I somehow have a feeling I'm the one thats going to have to fix this once a user opens an issue about getting different results. Perhaps it's better if we can limit/explicitly throw an error in the harder-to-mimic cases if the semantics differ?

@JoelJaeschke
Copy link
Author

JoelJaeschke commented Jan 10, 2026

No worries, and thanks for taking the time!

I think it makes sense to keep it on main. I was just confused as main already used the native geometry type whereas v1.4 still used the geometry type from duckdb-spatial I believe.

Regarding the difference versus PostGIS, PostGIS chooses the node closest to center to do the split, whereas the version I implemented just splits the geometry along the geometric center, irrespective of whether there is a node there or not. The advantage of the PostGIS approach is that it does not (possibly) introduce two new nodes per split and thus may finish faster, but the algorithm becomes more complex. I fully understand your argument, so if you would prefer to have the PR use the same approach as PostGIS, I would happily implement that.

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