Unexpected behavior when setting a doc body to nil

[tleyden]

When I set a doc body to nil and the retrieve the doc, it's returning an empty slice (and I was expecting it to return a nil doc body)

Steps to reproduce:

• Create new doc
• Update doc to set doc body to nil
• Get doc via kvstore.GetKV()
• Expected: should return nil. Actual: empty slice is returned

Test case

func TestSetDocBodyToNil(t *testing.T) {

    rawBucket, tempDir := GetTestBucket()

    defer os.RemoveAll(tempDir)
    defer CloseBucket(rawBucket)

    bucket := rawBucket.(*forestdbBucket)

    // set "foo" key to value "bar"
    err := bucket.kvstore.SetKV([]byte("foo"), []byte("bar"))
    assert.True(t, err == nil)

    err = bucket.db.Commit(forestdb.COMMIT_NORMAL)
    assert.True(t, err == nil)

    // now set "foo" key to nil
    doc, err := forestdb.NewDoc([]byte("foo"), nil, nil)
    assert.True(t, err == nil)

    defer doc.Close()
    err = bucket.kvstore.Set(doc)
    assert.True(t, err == nil)

    err = bucket.db.Commit(forestdb.COMMIT_NORMAL)
    assert.True(t, err == nil)

    // get the value of "foo" key
    docBody, err := bucket.kvstore.GetKV([]byte("foo"))
    assert.True(t, err == nil)
    assert.True(t, docBody == nil) // fails, because it's an empty slice

}

[mschoch]

I think the problem is that KV stores store []byte. You can't store nil, you can only store an empty byte array.

One option is that we throw an error if you try to store nil. But, before that I think we should just look at what the C library does. It looks to me like we're just passing through to C, so the same issue is either a bug, or not an issue at the C level.

[hisundar]

Setting body to null is the equivalent of issuing a delete. (That is how
fdb_del() is internally implemented.) So it is not a bug at C level.

On Mon, Apr 13, 2015 at 9:47 AM, Marty Schoch [email protected]
wrote:

I think the problem is that KV stores store []byte. You can't store nil,
you can only store an empty byte array.

One option is that we throw an error if you try to store nil. But, before
that I think we should just look at what the C library does. It looks to me
like we're just passing through to C, so the same issue is either a bug, or
not an issue at the C level.

—
Reply to this email directly or view it on GitHub
#15 (comment).

[mschoch]

Welcome to the joy of goforestdb, time to go back and write some C code...

[tleyden]

@hisundar can the behavior you just mentioned be added to the api docs? https://github.com/couchbase/forestdb/wiki/Public-APIs#changing-a-documents-contents + the header file (if not already, I didn't check)

[hisundar]

Sorry for the miscommunication. Setting the doc body to NULL is NOT the
same deleting it anymore. The api documentation is correct.

On Mon, Apr 13, 2015 at 10:26 AM, Traun Leyden [email protected]
wrote:

@hisundar https://github.com/hisundar can the behavior you just
mentioned be added to the api docs?
https://github.com/couchbase/forestdb/wiki/Public-APIs#changing-a-documents-contents

• the header file (if not already, I didn't check)

—
Reply to this email directly or view it on GitHub
#15 (comment).

[mschoch]

Yeah I've just gone through the C version and it behaves as expected. The issue is we cannot really store nil, currently when requested to do this, we store an empty byte slice. You notice the problem when you get the value back, because we return you the empty byte slice.

Option 1: leave it the way it is, storing nil is a shortcut for saving an empty byte slice
Option 2: interpret setting the value to nil means deleting (DISLIKE)
Option 3: return an error at the Go API level because we cannot actually set the value to nil