Add logical_type() to vtab::Value #657
Draft
Conversation
This file contains hidden or 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
Member
|
Do you need type introspection, or just extraction methods for non-integer types? |
mlafeldt
reviewed
Jan 20, 2026
|
|
||
| /// Returns a LogicalTypeHandle representing the type of this value. | ||
| pub fn logical_type(&self) -> LogicalTypeHandle { | ||
| // SAFETY: The returned LogicalTypeHandle frees allocated memory when dropped. |
Member
There was a problem hiding this comment.
Not true. duckdb_get_value_type returns a borrowed pointer to internal memory owned by the Value, not a new allocation. The C API docs say: "The type itself must not be destroyed". But LogicalTypeHandle::Drop calls duckdb_destroy_logical_type, causing double-free/corruption when dropped. I suggest returning an owned copy.
Alternatively, if sufficient, implement logical_type_id and return LogicalTypeId, which doesn't have this ownership problem.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Intending this to be a toe-in-the-water for building out
vtab::Value(maybe should be renamedValueHandlefor consistency withLogicalTypeHandle?).I was trying to create an extension (parsing fixed width binary files...) that needs non-integer parameters so looking to put the plumbing in to make that possible.