From c8f01464839ea47e9632a9cfa047b0b0715ae9d8 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Wed, 13 Apr 2022 12:09:46 +0300 Subject: [PATCH] crud: do not change input tuple object 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 --- CHANGELOG.md | 1 + crud/insert.lua | 4 +- crud/replace.lua | 4 +- crud/upsert.lua | 4 +- test/integration/simple_operations_test.lua | 47 +++++++++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d309bc7e..cdf48b59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crud/insert.lua b/crud/insert.lua index fbb0e462..1ad90665 100644 --- a/crud/insert.lua +++ b/crud/insert.lua @@ -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', @@ -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 diff --git a/crud/replace.lua b/crud/replace.lua index 2069398e..27e775d2 100644 --- a/crud/replace.lua +++ b/crud/replace.lua @@ -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', @@ -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 diff --git a/crud/upsert.lua b/crud/upsert.lua index 2ce3add9..58dec218 100644 --- a/crud/upsert.lua +++ b/crud/upsert.lua @@ -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', @@ -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 diff --git a/test/integration/simple_operations_test.lua b/test/integration/simple_operations_test.lua index 9ab7196d..a7be4200 100644 --- a/test/integration/simple_operations_test.lua +++ b/test/integration/simple_operations_test.lua @@ -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'}}