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

structured data support #1446

Closed
wants to merge 2 commits into from
Closed

Conversation

Walker-00
Copy link

Structured data type like struct, enum and others support added under the future "json" tested for window and linux I'm not sure about mac It's complicated for me to cross compile for it.

Copy link

@dariusc93 dariusc93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

Although in my case, I would generally do this as a wrapper around sled for storing serializable objects (which i would suggest instead of having it in sled directly), but what could be done to make it more uniform for other implementations around serde (eg bincode) would probably be to have 2 function pointer stored in Db that would call eg serde_json::to_vec and serde_json::from_slice (or in bincode case,bincode::serialize and bincode::deserialize) and have the functions be something like Db::insert_ser and Db::get_de (not the exact naming, but just throwing something out there) for anything related to serde under say a serde feature.

With that said, it might be best to look at #1266 and #1230

EDIT: Added links to issue and previous pr

key: K,
value: &V,
) -> Result<Option<IVec>> {
let value = serde_json::to_string(value).unwrap();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be best not to unwrapping as that would result in a panic if it fails to serialize (you could probably use io::Error or just one of the other errors in result::Error). Also, wouldnt it be better to use serde_json::to_vec instead?

) -> Result<Option<T>> {
match self.db.get(key.as_ref())? {
Some(v) => {
let x = String::from_utf8(v.to_vec()).unwrap();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could grab the bytes and pass it to serde_json::from_slice instead

@spacejam
Copy link
Owner

Hello - thank you for your interest in this, but I am going to close this PR because I would like sled to be completely ignorant of higher level formatting concerns for the forseeable future.

@spacejam spacejam closed this Aug 26, 2023
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.

None yet

3 participants