Add blob support

[macintux]

See basho/riak_kv#1540 for details.

[andytill]

I did talk to @lukebakken a while back about putting the blob data into the varchar column to avoid an extra record field per row, did that have problems?

[andytill]

This will definitely need an upgrade/downgrade test, with the added flag with queries, we might be in a position where the clients can't handle upgraded messages from TS because of this change and TS can't handle upgraded client messages because of the new flag on the query message.

[macintux]

That's a typical situation with the clients. I wasn't aware of the discussion to overload the existing varchar field, will investigate.

Sent from my iPad

On Nov 21, 2016, at 6:23 AM, Andy Till notifications@github.com wrote:

@andytill commented on this pull request.

In src/riak_ts.proto:

@@ -111,6 +112,7 @@ message TsCell {
optional sint64 timestamp_value = 3;
optional bool boolean_value = 4;
optional double double_value = 5;

• optional bytes blob_value = 6;
  This will definitely need an upgrade/downgrade test, with the added flag with queries, we might be in a position where the clients can't handle upgraded messages from TS because of this change and TS can't handle upgraded client messages because of the new flag on the query message.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[andytill]

@macintux ah damn, I thought I'd sent you the email chain, will resend.

[macintux]

You did send it to me, just forgot about it.

One significant downside to removing the blob option in PB: riak-shell uses the column type to decide to display the data as hex instead of a string. Unclear there's a good alternative.

/cc @alexmoore who's starting the client side of all this

[andytill]

My initial plan was to have a new enum value for blob like there is here and if the type is varchar or blob, pull the value from the varchar field of the TsCell message.

[macintux]

Gotcha, will look again, thanks.

[macintux]

Reviewing the code, I've realized I completely missed making important changes to riak_pb_ts_codec.erl, yet riak-shell works fine. Curious.

[andytill]

It could be because it uses the ttb encoder which doesn't require any changes because it just runs term_to_binary/1 on whatever it is given.

[alexmoore]

riak_pb_ts_codec.erl would need a change to add the blob the column types definition, and then that would trickle down to encode_cell, or anywhere else tscolumntype() is referenced.

https://github.com/basho/riak_pb/blob/develop/src/riak_pb_ts_codec.erl#L55

The response wouldn't change much for a TTB message though, it'd just now have blob included in the columnDescriptions list:

{ tsqueryresp,
   {
      [<<"geohash">>,<<"user">>,<<"time">>,<<"weather">>,<<"temperature">>,<<"uv_index">>,<<"observed">>,<<"sensor_data">>],
      [varchar,varchar,timestamp,varchar,double,sint64,boolean,blob],
      [
         {<<"hash1">>,<<"user2">>,1443806600000,<<"cloudy">>,[],[],true,<<0,1,2,3,4,5,6,7>>}
      ]
   }
}

[lukebakken]

@macintux re-using varchar means the existing client libs won't have to change.

[andytill]

+1 7e6260d

[macintux]

Ok, Bors has left the building. YOLOmerging