Skip to content

Commit

Permalink
api: return error if no tuples/objects
Browse files Browse the repository at this point in the history
Before this patch, the behavior of `*_many` requests if empty array of
tuples/objects were rather confusing. For example, due to format
processing all `*_object_many` operations resulted in `nil, {}` --
error request with zero errors. `insert_many` and `replace_many` calls
result in `nil, nil` -- no result, no error. `upsert_many` results in
`{metadata = metadata}, nil` with no `rows` in response. Thus, for all
six `*_many` calls trying to execute the request with empty array on
input result in malformed response.

After this patch, trying to run `*_many` request with empty array of
tuples/objects will result in `nil, {err}`, similar to existing
`*_many` API. Single tuple crud API already does not allow to run with
no tuples/objects.

Closes #377
  • Loading branch information
DifferentialOrange committed Oct 10, 2023
1 parent 53ba2c6 commit 6b20c8c
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
* `deps.sh` installs the `vshard` instead of the `cartridge` by default (#364).
You could to specify an environment variable `CARTIRDGE_VERSION` to install
the `cartridge` and run tests cases with it.
* Return explicit error for `*_many` call with no tuples/objects (#377).

### Fixed
* `crud.readview` resource cleanup on garbage collect (#379).
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ local result, err = crud.insert_object_many(space_name, objects, opts)
where:

* `space_name` (`string`) - name of the space to insert an object
* `tuples` / `objects` (`table`) - array of tuples/objects to insert
* `tuples` / `objects` (`table`) - array of tuples/objects to insert (at least one)
* `opts`:
* `timeout` (`?number`) - `vshard.call` timeout and vshard master
discovery timeout (in seconds), default value is 2
Expand Down Expand Up @@ -639,7 +639,7 @@ local result, err = crud.replace_object_many(space_name, objects, opts)
where:

* `space_name` (`string`) - name of the space to insert/replace an object
* `tuples` / `objects` (`table`) - array of tuples/objects to replace
* `tuples` / `objects` (`table`) - array of tuples/objects to replace (at least one)
* `opts`:
* `timeout` (`?number`) - `vshard.call` timeout and vshard master
discovery timeout (in seconds), default value is 2
Expand Down Expand Up @@ -855,7 +855,7 @@ where:
* `tuples_operation_data` / `objects_operation_data` (`table`) - array of
tuples/objects to insert
and update [operations](https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_space/#box-space-update)
in format {{tuple_1, operation_1}, ..., {tuple_n, operation_n}},
in format {{tuple_1, operation_1}, ..., {tuple_n, operation_n}} (at least one),
if there is tuple with duplicate key then existing tuple will
be updated with update operations
* `opts`:
Expand Down
8 changes: 8 additions & 0 deletions crud/insert_many.lua
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ function insert_many.tuples(space_name, tuples, opts)
fetch_latest_metadata = '?boolean',
})

if next(tuples) == nil then
return nil, {InsertManyError:new("At least one tuple expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down Expand Up @@ -284,6 +288,10 @@ function insert_many.objects(space_name, objs, opts)
fetch_latest_metadata = '?boolean',
})

if next(objs) == nil then
return nil, {InsertManyError:new("At least one object expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down
8 changes: 8 additions & 0 deletions crud/replace_many.lua
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ function replace_many.tuples(space_name, tuples, opts)
fetch_latest_metadata = '?boolean',
})

if next(tuples) == nil then
return nil, {ReplaceManyError:new("At least one tuple expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down Expand Up @@ -287,6 +291,10 @@ function replace_many.objects(space_name, objs, opts)
fetch_latest_metadata = '?boolean',
})

if next(objs) == nil then
return nil, {ReplaceManyError:new("At least one object expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down
8 changes: 8 additions & 0 deletions crud/upsert_many.lua
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ function upsert_many.tuples(space_name, tuples_operation_data, opts)
fetch_latest_metadata = '?boolean',
})

if next(tuples_operation_data) == nil then
return nil, {UpsertManyError:new("At least one tuple expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down Expand Up @@ -303,6 +307,10 @@ function upsert_many.objects(space_name, objs_operation_data, opts)
fetch_latest_metadata = '?boolean',
})

if next(objs_operation_data) == nil then
return nil, {UpsertManyError:new("At least one object expected")}
end

opts = opts or {}

local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
Expand Down
20 changes: 20 additions & 0 deletions test/integration/insert_many_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2032,3 +2032,23 @@ pgroup.test_noreturn_opt = function(g)
t.assert_equals(#errs, 3)
t.assert_equals(result, nil)
end

pgroup.test_zero_tuples = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.insert_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one tuple expected")
t.assert_equals(result, nil)
end

pgroup.test_zero_objects = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.insert_object_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end
20 changes: 20 additions & 0 deletions test/integration/replace_many_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2043,3 +2043,23 @@ pgroup.test_noreturn_opt = function(g)
t.assert_equals(#errs, 3)
t.assert_equals(result, nil)
end

pgroup.test_zero_tuples = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.replace_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one tuple expected")
t.assert_equals(result, nil)
end

pgroup.test_zero_objects = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.replace_object_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end
20 changes: 20 additions & 0 deletions test/integration/upsert_many_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2044,3 +2044,23 @@ pgroup.test_noreturn_opt = function(g)
t.assert_equals(#errs, 3)
t.assert_equals(result, nil)
end

pgroup.test_zero_tuples = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.upsert_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one tuple expected")
t.assert_equals(result, nil)
end

pgroup.test_zero_objects = function(g)
local result, errs = g.cluster.main_server.net_box:call(
'crud.upsert_object_many', {'customers', {}})

t.assert_not_equals(errs, nil)
t.assert_equals(#errs, 1)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end

0 comments on commit 6b20c8c

Please sign in to comment.