basho / eleveldb

Erlang LevelDB API
262 stars 177 forks source link

Modified msgpack extraction path to check for empty erlang list (null… #161

Closed erikleitch closed 9 years ago

erikleitch commented 9 years ago

… indicator from riak_kv). Modified filter to return false when evaluated against a missing field value, instead of throw. Modified unit tests to reflect the change in behavior.

Note:

The filtering code was originally developed under the direction that missing fields should be an error. This PR changes that behavior so that filters that reference missing fields evaluate to false, instead of throwing an error.

Summary of changes:

1) added code in CmpUtil to detect empty list encoding in msgpack 2) added code in extractor.cc to check for empty fields while decoding msgpack 3) modified all binary filter operators in filter.h to return false in evaluate() rather than throw if a field has no value 4) modified unit tests in sut.erl to reflect the new behavior

matthewvon commented 9 years ago

Is .rebar/erlcinfo an intentional check-in ? I do not know its function.

matthewvon commented 9 years ago

+1 once .rebar/erlcinfo explained or removed.

erikleitch commented 9 years ago

.rebar/erclinfo not intentional -- got checked in by accident at some point & I removed it from a later branch. Will make sure it's removed from the merge

matthewvon commented 9 years ago

When you commit the change to delete .rebar/erclinfo, would you include an edit to .gitignore that explicitly lists that file. Might as well protect against the next time ...

Thank you.

erikleitch commented 9 years ago

Yup. Good idea.

On Tue, Oct 20, 2015 at 10:06 AM, Matthew Von-Maszewski < notifications@github.com> wrote:

When you commit the change to delete .rebar/erclinfo, would you include an edit to .gitignore that explicitly lists that file. Might as well protect against the next time ...

Thank you.

— Reply to this email directly or view it on GitHub https://github.com/basho/eleveldb/pull/161#issuecomment-149634931.