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

Allow put/write of empty values #223

Closed
ggreer opened this issue Dec 16, 2013 · 13 comments
Closed

Allow put/write of empty values #223

ggreer opened this issue Dec 16, 2013 · 13 comments

Comments

@ggreer
Copy link

ggreer commented Dec 16, 2013

As silly as it sounds, I'd like to be able to save empty values to leveldb. Right now, I get "WriteError: value cannot be an empty String" if I try.

I traced the code back to the LD_STRING_OR_BUFFER_TO_SLICE macro in leveldown: https://github.com/rvagg/node-leveldown/blob/master/src/leveldown.h#L45

This macro is correct for validating keys, but leveldb allows for empty values. If this is not intentional behavior, I think I'll try to fix it and submit a pull request.

@heapwolf
Copy link
Contributor

What is a use case for wanting to save an empty values to a database?

@mcollina
Copy link
Member

You can use empty values to store events that happened.
It is frustrating to call db.put(mykey, 'dummy').
Also, leveldb can handle them just fine, so why not?

@juliangruber
Copy link
Member

same, i've used ' ' often and that's really a hack

@aeosynth
Copy link

https://github.com/rvagg/node-levelup#batch

any 'type': 'put' entry with a 'value' of null or undefined will return an error

the empty string is neither null nor undefined

my use case: i simply want to avoid NotFoundError

@heapwolf
Copy link
Contributor

Not saying it's not a good idea or not worth having. This is just food for thought. But two reasons I avoid doing this are...

  1. Poor context, the next person who has to look at it may not understand the context of the value. I prefer to do something like { type: 'put', key: 'some_key', value: '{ some_things: [] }' }.
  2. Its easy to accumulate a lot of trash. For instance, lets say you add an empty value for every user who might do something. Then you need to clean it up later either way you have superfluous cycles against your store.

@dominictarr
Copy link
Contributor

a not found error, and an empty record are totally different things. for example, even if you don't know the exact location of a key you can seek past it and find it that way. you have still stored the key, which is useful. maybe you want to have the key store values... for example, in an index you might want to keep a tuple of {from, to, relation}... if you put all the information in the key, then you don't need to worry about collisions, because it will effectively sort by the value.

I think you should make this into a pull request.

@michaelnisi
Copy link

Yes, there are valid use cases where keys contain sufficient data making the actual values obsolete.

@heapwolf
Copy link
Contributor

hmm, then maybe you need a key store and not a key-value store ;)

@juliangruber
Copy link
Member

@hij1nx that's super useful for sets, like reserved slugs. And key-value stores are a superset of key stores, so I think the concern is valid.

@heapwolf
Copy link
Contributor

@juliangruber i'm not disagreeing, im simply saying that there are down sides to storing empty values.

@ggoodman
Copy link

@hij1nx I am trying to save git objects to leveldb. Git objects are content-addressable by their SHA1. Since a Blob object for an empty file is perfectly reasonable, this check is preventing me from storing empty git files.

In theory, I could add in custom code everywhere a read and write to leveldb to short-circuit on the sha1 of an empty buffer, but that seems extreme.

Perhaps this fits the description of a legitimate use-case.

@kesla
Copy link
Contributor

kesla commented Sep 11, 2014

@ggoodman in leveldown 1.0.0 we added support for empty values, so now we just need to update levelup to support it. So, if you want empty values you can for now use the more raw leveldown instead.

@ralphtheninja
Copy link
Member

@ggreer What @kesla mentioned before is now in the 1.0.0-wip branch (36658a2) so you'll have this functionality once 1.0.0 of levelup has been released.

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 a pull request may close this issue.

10 participants