Skip to content

Commit

Permalink
crud: do not change input tuple object
Browse files Browse the repository at this point in the history
If crud request uses tuple as input argument (insert, upsert and replace
operations) and its bucket_id is empty, the module will fill this field
and damage input argument tuple. This patch fixes this behavior.

After this patch, performance of insert, upsert and replace has
decreased by 5%.

Part of #212
  • Loading branch information
DifferentialOrange committed Apr 20, 2022
1 parent 2454862 commit c8f0146
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Fixed
* Fix processing storage error for tuple-merger implementation of
select/pairs (#271).
* Do not change input tuple object in requests.

## [0.10.0] - 01-12-21

Expand Down
4 changes: 3 additions & 1 deletion crud/insert.lua
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ end
-- returns result, err, need_reload
-- need_reload indicates if reloading schema could help
-- see crud.common.schema.wrap_func_reload()
local function call_insert_on_router(space_name, tuple, opts)
local function call_insert_on_router(space_name, original_tuple, opts)
dev_checks('string', 'table', {
timeout = '?number',
bucket_id = '?number|cdata',
Expand All @@ -58,6 +58,8 @@ local function call_insert_on_router(space_name, tuple, opts)
return nil, InsertError:new("Space %q doesn't exist", space_name), true
end

local tuple = table.deepcopy(original_tuple)

local bucket_id, err = sharding.tuple_set_and_return_bucket_id(tuple, space, opts.bucket_id)
if err ~= nil then
return nil, InsertError:new("Failed to get bucket ID: %s", err), true
Expand Down
4 changes: 3 additions & 1 deletion crud/replace.lua
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ end
-- returns result, err, need_reload
-- need_reload indicates if reloading schema could help
-- see crud.common.schema.wrap_func_reload()
local function call_replace_on_router(space_name, tuple, opts)
local function call_replace_on_router(space_name, original_tuple, opts)
dev_checks('string', 'table', {
timeout = '?number',
bucket_id = '?number|cdata',
Expand All @@ -62,6 +62,8 @@ local function call_replace_on_router(space_name, tuple, opts)
return nil, ReplaceError:new("Space %q doesn't exist", space_name), true
end

local tuple = table.deepcopy(original_tuple)

local bucket_id, err = sharding.tuple_set_and_return_bucket_id(tuple, space, opts.bucket_id)
if err ~= nil then
return nil, ReplaceError:new("Failed to get bucket ID: %s", err), true
Expand Down
4 changes: 3 additions & 1 deletion crud/upsert.lua
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ end
-- returns result, err, need_reload
-- need_reload indicates if reloading schema could help
-- see crud.common.schema.wrap_func_reload()
local function call_upsert_on_router(space_name, tuple, user_operations, opts)
local function call_upsert_on_router(space_name, original_tuple, user_operations, opts)
dev_checks('string', '?', 'table', {
timeout = '?number',
bucket_id = '?number|cdata',
Expand Down Expand Up @@ -69,6 +69,8 @@ local function call_upsert_on_router(space_name, tuple, user_operations, opts)
end
end

local tuple = table.deepcopy(original_tuple)

local bucket_id, err = sharding.tuple_set_and_return_bucket_id(tuple, space, opts.bucket_id)
if err ~= nil then
return nil, UpsertError:new("Failed to get bucket ID: %s", err), true
Expand Down
47 changes: 47 additions & 0 deletions test/integration/simple_operations_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,53 @@ pgroup.test_partial_result_bad_input = function(g)
t.assert_str_contains(err.err, 'Space format doesn\'t contain field named "lastname"')
end

pgroup.test_tuple_not_damaged = function(g)
-- insert
local insert_tuple = {22, box.NULL, 'Elizabeth', 24}
local new_insert_tuple, err = g.cluster.main_server:eval([[
local crud = require('crud')
local insert_tuple = ...
local _, err = crud.insert('customers', insert_tuple)
return insert_tuple, err
]], {insert_tuple})

t.assert_equals(err, nil)
t.assert_equals(new_insert_tuple, insert_tuple)

-- upsert
local upsert_tuple = {33, box.NULL, 'Peter', 35}
local new_upsert_tuple, err = g.cluster.main_server:eval([[
local crud = require('crud')
local upsert_tuple = ...
local _, err = crud.upsert('customers', upsert_tuple, {{'+', 'age', 1}})
return upsert_tuple, err
]], {upsert_tuple})

t.assert_equals(err, nil)
t.assert_equals(new_upsert_tuple, upsert_tuple)

-- replace
local replace_tuple = {22, box.NULL, 'Elizabeth', 24}
local new_replace_tuple, err = g.cluster.main_server:eval([[
local crud = require('crud')
local replace_tuple = ...
local _, err = crud.replace('customers', replace_tuple)
return replace_tuple, err
]], {replace_tuple})

t.assert_equals(err, nil)
t.assert_equals(new_replace_tuple, replace_tuple)
end

pgroup.test_opts_not_damaged = function(g)
-- insert
local insert_opts = {timeout = 1, bucket_id = 655, fields = {'name', 'age'}}
Expand Down

0 comments on commit c8f0146

Please sign in to comment.