Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internal Rework #494

Merged
merged 74 commits into from
Feb 11, 2025
Merged

Internal Rework #494

merged 74 commits into from
Feb 11, 2025

Conversation

Maxxen
Copy link
Member

@Maxxen Maxxen commented Feb 10, 2025

This is a pretty massive rework of the spatial internals.
Im going to write about this more in the future, but heres the highlights:

  • Simplify source layout and includes - instead of having a ton of separate file for each function, we now bundle them into files by module/type. We also remove all the extra spatial namespacing and "#include spatial/common.h" helper headers. All in all this significantly reduces compile times and makes it easier to find stuff (not as much nesting). The final layout is not entirely set in stone yet. Im not sure if we want to have a separate include directory or not, we'll see, but this is already much clearer IMO.

  • Completely reworked internal geometry representation/engine. We now have a separate SGL (spatial geometry library) subproject implementing simple features without any explicit dependencies on the DuckDB code base. Additionally It uses a pretty (I think) clever representation based on circular linked lists and parent-pointers, which allows for very flexible allocation patterns and the implementation of a lot of geometry operations without requiring recursion or allocation. By separating it from the DuckDB code base its also much easier to test, and the goal is to eventually reach full test code coverage on it, which is much more feasible if its not tied to DuckDB. Its currently written in very "low level" C++, because I want to see if we can lower it down into C for extra portability. Its still a bit of a mess though, but ill clean it up eventually.

There are also a lot of misc changes and fixes here and there, heres some at the top of my head

  • WKB parsing is now non-recursive
  • WKT parsing is now non-recursive
  • WKB parsing failures will report better error messages, and will try to guess the type name for unsupported geometry types.
  • WKB_BLOB is no longer auto-cast to WKT.
  • Geographiclib is no longer a required dependency, the _spheroid operations are now implemented by using the c version reexported from proj.
  • Remove allocations from ST_Extent_Agg during serialization
  • GEOS-based functions no longer require a memory arena to be initialized.

@Maxxen Maxxen merged commit ba425b7 into duckdb:main Feb 11, 2025
22 checks passed
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.

1 participant