-
Notifications
You must be signed in to change notification settings - Fork 20
Convert to C-API based extension #20
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
Conversation
samansmink
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is really great!!
| # Set to 1 to enable Unstable API (binaries will only work on TARGET_DUCKDB_VERSION, forwards compatibility will be broken) | ||
| # WARNING: When set to 1, the duckdb_extension.h from the TARGET_DUCKDB_VERSION must be used, using any other version of | ||
| # the header is unsafe. | ||
| USE_UNSTABLE_C_API=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to open an issue that lists all the C API functions that we need to stabilize to be able to switch inet to the stable C API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we can do that - In this case I don't think inet actually needs any unstable changes, but there is technically a behavior change related to registering overloads (thats only available on upstream main), so I kept it as unstable just to signal that this won't work with any "stable" c-api build. Or rather, it won't work with all duckdb versions that provides the stable api, only those post v1.4.
This PR converts the
inetextension to be fully based on DuckDB's extension C-API. It also converts all code to c, from c++.This makes maintenance much easier as it no longer depends on any (frequently changing) DuckDB-internal code. Additionally it significantly improves the build-time and the size of the extension. A release build is about
325Kon my machine, compared to the6.7Mbinary we distribute for DuckDB v1.3.2.While most of the code has been relatively straightforward to port c (e.g.
ipaddress.cpp/hpp), I've reworked thehtml_unescapefunction significantly by making use of a static pre-generated perfect-hash-table to decode html entities, which should be faster, and avoids dynamic initialization and copying of the map in the function bind data. But the behavior should be the same.It also includes the changes in #19