box: fix _func id overflow#12004
Conversation
f3d9e90 to
65f39fb
Compare
ImeevMA
left a comment
There was a problem hiding this comment.
Hi! Thanks for the patch. I have a few comments regarding the SQL part, please review them. As for the patch as a whole, I think it's a bit overcomplicated. Do we even need a universal function for generating space and function identifiers? Introducing a function to generate the function identifier seems like a simpler solution. Just a thought, though.
2d52f6f to
fe23e34
Compare
There was a problem hiding this comment.
Thank you for the fixes! I have just one comment for SQL part.
I have a few comments on the non-SQL part:
- I still think that there is no need to complicate the code due to function ID generation.
- Name of the test-file usually starts with
gh-...for non-general tests. Also, you can just add the test tobox-luatest/func_test.lua.
But I leave this part to @sergepetrenko
fe23e34 to
a32dd13
Compare
sergepetrenko
left a comment
There was a problem hiding this comment.
Thanks for the patch!
81bce09 to
f554899
Compare
98ba970 to
302b904
Compare
test/box-luatest/gh_11849_11851_box_func_no_id_overflow_test.lua
Outdated
Show resolved
Hide resolved
test/box-luatest/gh_11849_11851_box_func_no_id_overflow_test.lua
Outdated
Show resolved
Hide resolved
test/box-luatest/gh_11849_11851_box_func_no_id_overflow_test.lua
Outdated
Show resolved
Hide resolved
80edb44 to
0541e08
Compare
517b840 to
85aae89
Compare
drewdzzz
left a comment
There was a problem hiding this comment.
LGTM, thank you for the patch and for your fixes!
test/box-luatest/gh_11849_11851_box_func_no_id_overflow_test.lua
Outdated
Show resolved
Hide resolved
Introduce the `box_generate_unique_id` function, which is useful for spaces with limited primary key range such as `_func`. Part of tarantool#11849 Part of tarantool#11851 NO_DOC=refactoring NO_TEST=refactoring NO_CHANGELOG=refactoring
Each time one used the `box.schema.func.create`, the primary key of the resulted function increased because of the `_func:auto_increment`. Now the new key is chosen by the `generate_func_id` in Lua and `OP_GenFuncid` SQL opcode, both implemented using the new `box_generate_func_id` function that shares logic with `box_generate_space_id`. Hence we are scanning the entire range of possible ids `[0, BOX_FUNCTION_MAX)` before we are returning an error. Fixes tarantool#11849 Fixes tarantool#11851 NO_DOC=bugfix
85aae89 to
5d695f8
Compare
|
I'm OK with the latest fixes, all the comments are resolved and |
sergepetrenko
left a comment
There was a problem hiding this comment.
Lev, thanks for the patch!
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release/3.2
git worktree add -d .worktree/backport/release/3.2/12004 origin/release/3.2
cd .worktree/backport/release/3.2/12004
git switch --create backport/release/3.2/12004
git cherry-pick -x 80e580e04494c96d98c925603582354c951889bd 95d35505f5784c0d45bc404ef6f0c85471b3ec02 |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release/3.3
git worktree add -d .worktree/backport/release/3.3/12004 origin/release/3.3
cd .worktree/backport/release/3.3/12004
git switch --create backport/release/3.3/12004
git cherry-pick -x 80e580e04494c96d98c925603582354c951889bd 95d35505f5784c0d45bc404ef6f0c85471b3ec02 |
|
Backport failed for |
|
Successfully created backport PR for |
Backport summary
|
Each time one used the
box.schema.func.create, the primary key ofthe resulted function increased because of the
_func:auto_increment. Now the new key is chosen by thegenerate_func_idin Lua andOP_GenFuncidSQL opcode, both implementedusing the new
box_generate_func_idfunction that shares logic withbox_generate_space_id. Hence we are scanning the entire range ofpossible ids
[0, BOX_FUNCTION_MAX)before we are returning anerror.
Fixes #11849
Fixes #11851
NO_DOC=bugfix