leveldb_get: can't return NotFound err

[GoogleCodeExporter]

leveldb_get: can't return NotFound err

Original issue reported on code.google.com by chaishus...@gmail.com on 8 Oct 2013 at 6:26

Attachments:

• leveldb_get_not_found_err.patch

[GoogleCodeExporter]

I think this is working as intended, so I am closing this as WontFix.  
Explanation below.  Let me know if I missed something.

To check for not-found with the current code, the caller can do this:
   char* errptr = NULL;
   char* result = leveldb_get(..., &errptr);
   if (errptr != NULL) {
     // Handle error
   } else if (result == NULL) {
     // Not found
   } else {
     // Handle result
   }

In more detail, there are three possible outcomes:
(1) Error
(2a) Not found
(2b) Found

leveldb_get() treats 2a,2b as success and 1 as error.  Following the error 
reporting rules for this module, it leaves *errptr unchanged (as NULL in my 
suggested code) for 2a,2b and sets it to an error message for 1.  The return 
value of NULL can be used to distinguish between 2a and 2b.

Original comment by san...@google.com on 8 Oct 2013 at 4:09

• Changed state: WontFix

[GoogleCodeExporter]

I thought leveldb_get and leveldb::DB::Get has different behavior for not found 
scene.

// C++
std::string value;
leveldb::Status s = db->Get(leveldb::ReadOptions(), key1, &value);
if (s.ok()) {
    // Handle result
} else if (s.NotFound()) {
    // Not found
} else {
    // other error
}

// C
char* errptr = NULL;
char* result = leveldb_get(..., &errptr);
if (errptr != NULL) {
    // Handle error
} else if (result == NULL) {
    // Not found
} else {
    // Handle result
}

And for other language binds with c api, the caller often need to check err 
type.

// Go
func newError(e string) error {
    if e == "" || e == "OK" {
        return nil
    }
    if strings.HasPrefix(e, "NotFound:") {
        return ErrNotFound
    }
    if strings.HasPrefix(e, "Corruption:") {
        return ErrCorruption
    }
    if strings.HasPrefix(e, "Not implemented:") {
        return ErrNotImplemented
    }
    if strings.HasPrefix(e, "Invalid argument:") {
        return ErrInvalidArgument
    }
    if strings.HasPrefix(e, "IO error:") {
        return ErrIO
    }
    return ErrUnknown
}

In current leveldb_get, the caller should distinguish between
`(result == NULL)` and `(vallen == 0)`, and the caller can't
know the `NotFound` status just from the errptr.

Thanks.

Original comment by chaishus...@gmail.com on 9 Oct 2013 at 12:11