Closed erikleitch closed 9 years ago
Looks like my previous comment got mangled a bit by the parser. Should read:
Per group discussion last Friday it was decided to change the riak_object content encoding format for TS, from:
<< EncBin/binary >>
to
<<?ENC_MAGIC/integer, EncBin/binary>>
+1
Per group discussion last Friday it was decided to change the riak_object content encoding format for TS, from:
<<EncBin/binary>>
to
<<?ENC_MAGIC/integer, EncBin/binary>>
This branch implements that encoding change in eleveldb, and a couple additional fixes along the way. Changes are:
1) The encoding change necessitates a change in the way that eleveldb decodes the resulting binary. This is the purpose of the changes to Extractor:getToRiakObjectContents() and Extractor:riakObjectContentsCanBeParsed(), and the addition of MSGPACK_MAGIC define to indicate that the encoding is msgpack.
2) unit-testing code sut.erl was modified to test these changes
3) I have also taken the opportunity to add a check for the magic number (53) at the head of the encoded riak object. We were previously only checking the version, but should really be checking both numbers.
4) The change to Extractor::cTypeOf() is an additional protection against mis-use of the filtering API documented here: https://docs.google.com/document/d/1PNaiiBP-28Szljpi5owLMb2jKXOGpepOrsFQIbF4QcU/edit#heading=h.qb0fnpizuhxh. Field values are specified with a type, like: {field, <<"f1">>, integer}, but constants shouldn't contain a type, which could conflict with the field specification, ie,they should be specified like {const, 2} instead of {const, 2, float}. The previous code assumed that if arity == 3, the tuple was a field specification. The modified code now checks if a 3-tuple is an invalid const specification, and quietly ignores ignores the type if it is.
5) Lastly, I noticed that some members of ExpressionNode<unsigned char*>(type) weren't initialized in the constructor. This has produced no errors in testing, because hasval is always checked before value_ is ever accessed, but needed to be fixed nonetheless.