[tarantool-patches] Re: [PATCH 4/8] netbox: extend codec with 'decode' methods
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Apr 23 21:59:13 MSK 2018
Hello. Thanks for review! See the diff at the end of letter.
>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>> index 4ed2b375d..3868cdf1c 100644
>> --- a/src/box/lua/net_box.lua
>> +++ b/src/box/lua/net_box.lua
>> @@ -50,7 +50,34 @@ local E_PROC_LUA = box.error.PROC_LUA
>>
>> -- utility tables
>> local is_final_state = {closed = 1, error = 1}
>> -local method_codec = {
>> +
>> +local function decode_nil(...) end
>
>> +local function decode_nothing(...) return ... end
>
> decode_nop, may be?
Done.
>
>> +local function decode_one_tuple(response)
>> + if response[1] then
>> + return box.tuple.new(response[1])
>> + end
>> +end
>> +local function decode_single_tuple(response)
>> + if response[2] then
>> + return nil, box.error.MORE_THAN_ONE_TUPLE
>> + end
>> + if response[1] then
>> + return box.tuple.new(response[1])
>> + end
>> +end
>
> Do we really need this MORE_THAN_ONE_TUPLE error? Can't we use
> decode_one_tuple in 'get' as well? I ask, because having two methods
> called decode_one_tuple and decode_single_tuple is confusing IMO -
> it's unclear what's the difference between them.
I agree with you, it looks ugly, but IProto has no 'get' method, so
we must emulate it via select(limit = 2) + check that a tuple is
only one. I renamed decode_single to decode_unique. Maybe it looks
not so bad.
>
>> +local function decode_select(response)
>> + setmetatable(response, sequence_mt)
>> + for i, v in pairs(response) do
>> + response[i] = box.tuple.new(v)
>> + end
>> + return response
>> +end
>> +local function decode_count(response)
>> + return response[1]
>> +end
>> +
>> +local method_encoder = {
>> ping = internal.encode_ping,
>> call_16 = internal.encode_call_16,
>> call_17 = internal.encode_call,
>> @@ -61,6 +88,10 @@ local method_codec = {
>> update = internal.encode_update,
>> upsert = internal.encode_upsert,
>> select = internal.encode_select,
>> + get = internal.encode_select,
>> + min = internal.encode_select,
>> + max = internal.encode_select,
>> + count = internal.encode_call,
>> -- inject raw data into connection, used by console and tests
>> inject = function(buf, id, schema_version, bytes)
>> local ptr = buf:reserve(#bytes)
>> @@ -69,6 +100,24 @@ local method_codec = {
>> end
>> }
>>
>> +local method_decoder = {
>> + ping = decode_nil,
>> + call_16 = decode_select,
>> + call_17 = decode_nothing,
>> + eval = decode_nothing,
>> + insert = decode_one_tuple,
>> + replace = decode_one_tuple,
>> + delete = decode_one_tuple,
>> + update = decode_one_tuple,
>> + upsert = decode_nil,
>> + select = decode_select,
>> + get = decode_single_tuple,
>> + min = decode_single_tuple,
>> + max = decode_single_tuple,
>> + count = decode_count,
>> + inject = decode_nothing,
>> +}
>> +
>> local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
>>
>> --
>
>> @@ -1144,9 +1175,8 @@ index_metatable = function(remote)
>> if opts and opts.buffer then
>> error("index:min() doesn't support `buffer` argument")
>> end
>> - local res = remote:_request('select', opts, self.space.id, self.id,
>> - box.index.GE, 0, 1, key)
>> - return one_tuple(res)
>> + return remote:_request('get', opts, self.space.id, self.id,
>
> Should be 'min'?
Yes, did not notice that. Fixed on the branch.
>
>> + box.index.GE, 0, 1, key)
>> end
>>
>> function methods:max(key, opts)
>> @@ -1154,9 +1184,8 @@ index_metatable = function(remote)
>> if opts and opts.buffer then
>> error("index:max() doesn't support `buffer` argument")
>> end
>> - local res = remote:_request('select', opts, self.space.id, self.id,
>> - box.index.LE, 0, 1, key)
>> - return one_tuple(res)
>> + return remote:_request('get', opts, self.space.id, self.id,
>
> Should be 'max'?
Same, you are right. Fixed.
See the diff:
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 3868cdf1c..53f301356 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -52,13 +52,13 @@ local E_PROC_LUA = box.error.PROC_LUA
local is_final_state = {closed = 1, error = 1}
local function decode_nil(...) end
-local function decode_nothing(...) return ... end
+local function decode_nop(...) return ... end
local function decode_one_tuple(response)
if response[1] then
return box.tuple.new(response[1])
end
end
-local function decode_single_tuple(response)
+local function decode_unique_tuple(response)
if response[2] then
return nil, box.error.MORE_THAN_ONE_TUPLE
end
@@ -103,19 +103,19 @@ local method_encoder = {
local method_decoder = {
ping = decode_nil,
call_16 = decode_select,
- call_17 = decode_nothing,
- eval = decode_nothing,
+ call_17 = decode_nop,
+ eval = decode_nop,
insert = decode_one_tuple,
replace = decode_one_tuple,
delete = decode_one_tuple,
update = decode_one_tuple,
upsert = decode_nil,
select = decode_select,
- get = decode_single_tuple,
- min = decode_single_tuple,
- max = decode_single_tuple,
+ get = decode_unique_tuple,
+ min = decode_unique_tuple,
+ max = decode_unique_tuple,
count = decode_count,
- inject = decode_nothing,
+ inject = decode_nop,
}
local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
@@ -1175,7 +1175,7 @@ index_metatable = function(remote)
if opts and opts.buffer then
error("index:min() doesn't support `buffer` argument")
end
- return remote:_request('get', opts, self.space.id, self.id,
+ return remote:_request('min', opts, self.space.id, self.id,
box.index.GE, 0, 1, key)
end
@@ -1184,7 +1184,7 @@ index_metatable = function(remote)
if opts and opts.buffer then
error("index:max() doesn't support `buffer` argument")
end
- return remote:_request('get', opts, self.space.id, self.id,
+ return remote:_request('max', opts, self.space.id, self.id,
box.index.LE, 0, 1, key)
end
More information about the Tarantool-patches
mailing list