Add duckdb patch signature_capi #1918
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signature of capi extensions changed from returning void to returning a boolean, mismatch on native targets is fine, since result is just ignored, but flagged problems in wasm validation. Given capi extensions are still experimental, and more so in duckdb-wasm, I would propose taking this liberty from DuckDB source code to allow more simpler exploration on this matter.
samansmink
approved these changes
Nov 12, 2024
|
Also to be noted, this changed in duckdb main branch already, so that would allow to test extensions either:
and see similar results. Otherwise the third option would have been left behind, and that would mean that errors will be only noticed with delays. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signature of capi extensions changed from returning void to returning a boolean, mismatch on native targets is fine, since result is just ignored, but flagged problems in wasm validation due to signature changing in an incompatible way from
(int,int)->()to(int,int)->(int).Given the error is on validation of indirect call, I am not aware of a workaround Wasm side, and given capi extensions are still experimental, and more so in duckdb-wasm, I would propose taking this liberty from DuckDB source code to allow more simpler exploration on this matter.
Discussed with @samansmink.