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

hyperdb.del() should not add null value for already deleted or non-existent keys #94

Open
e-e-e opened this issue Apr 2, 2018 · 3 comments

Comments

@e-e-e
Copy link
Contributor

e-e-e commented Apr 2, 2018

Currently deleting keys adds a null value to the db regardless of if the key exists or not. It would be great to check if the key exists or not before deleting to avoid unnecessary data in the db.

I wrote some tests for this the first and last are currently failing.

tape('does not delete non existent keys', function (t) {
  var db = create.one()
  db.del('hello', function (err, node) {
    t.error(err, 'no error')
    t.same(db._writers[0].length(), 0)
    t.end()
  })
})

tape('does not delete non existent keys', function (t) {
  var db = create.one()
  db.put('hello', 'world', function (err, node) {
    t.error(err, 'no error')
    db.del('hello', function (err, node) {
      t.error(err, 'no error')
      t.same(db._writers[0].length(), 2)
      t.end()
    })
  })
})


tape('does not delete non existent keys', function (t) {
  var db = create.one()
  db.put('hello', 'world', function (err, node) {
    t.error(err, 'no error')
    db.del('hello', function (err, node) {
      t.error(err, 'no error')
      t.same(db._writers[0].length(), 2)
      db.del('hello', function (err, node) {
        t.error(err, 'no error')
        t.same(db._writers[0].length(), 2)
        t.end()
      })
    })
  })
})
@mafintosh
Copy link
Owner

I think we can efficiently to this while walking the trie and see if we end up visiting the same key if someone wants to attempt a pr

@mafintosh
Copy link
Owner

@e-e-e i think extending this with an option that makes it not overwrite a put with the same value seems relevant as well

@e-e-e
Copy link
Contributor Author

e-e-e commented Aug 8, 2018

@mafintosh just playing around with writing a staging area for hyperdb's and realised that recording a del for a non-existent key is a really key feature. It helps to easily see if the staged db has deleted a key without having to add that key.

I think doing just as you suggest, not overwriting puts is a better idea than not adding the del for non-existent items.

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

No branches or pull requests

2 participants