box: populate `index_object.parts` with `key_def` module methods by Gumix · Pull Request #8356 · tarantool/tarantool

Gumix · 2023-02-19T22:22:49Z

box: populate index_object.parts with key_def module methods

This patch adds 4 methods of the key_def module instance to the
index_object.parts, see the docbot request for details. The rest
methods (new() and totable()) are not applicable here, because the
instance is already created and pushed to the stack as a table.

@TarantoolBot document
Title: index_object.parts methods
Product: Tarantool
Since: 3.0
Root document: https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_index/parts/

index_object.parts has the following methods: extract_key(), compare(), compare_with_key(), merge().
For their description and usage examples, refer to Module key_def.

Root document: https://www.tarantool.io/en/doc/latest/reference/reference_lua/key_def/

index_object.parts can be used as a key_def module instance for calling the corresponding methods.

Example:

box.schema.space.create('T')
i = box.space.T:create_index('I', {parts={3, 'string', 1, 'unsigned'}})
box.space.T:insert{1, 99.5, 'X', nil, 99.5}
i.parts:extract_key(box.space.T:get({'X', 1}))

The code above is equivalent to:

key_def = require('key_def')
box.schema.space.create('T')
i = box.space.T:create_index('I', {parts={3, 'string', 1, 'unsigned'}})
box.space.T:insert{1, 99.5, 'X', nil, 99.5}
k = key_def.new(i.parts)
k:extract_key(box.space.T:get({'X', 1}))

Closes #7356

src/box/lua/key_def.c

coveralls · 2023-02-19T22:57:03Z

Coverage: 85.651% (+0.009%) from 85.642% when pulling 27f698c on Gumix:iverbin/gh-7356-populate-index-with-keydef into 19b53ac
on tarantool:master.

src/box/lua/key_def.c

src/box/lua/space.cc

olegrok · 2023-02-23T20:28:49Z

box.schema.space.create('test')
box.space.test:create_index('pk')
box.space.test:create_index('mk', {parts = {{type = 'string', path = '[*]', field = 2}}})
box.space.test:replace({1, {"a", "b"}})
tuple = box.space.test.index.mk:select()[1]

tarantool> box.space.test.index[0].parts:extract_key(tuple) -- Ok
---
- [1]
...

tarantool> box.space.test.index[1].parts:extract_key(tuple) -- Not Ok
Assertion failed: (!key_def->is_multikey), function tuple_validate_key_parts, file tuple_extract_key.cc, line 513.
[1]    10902 abort      ./src/tarantool

Totktonada · 2023-02-28T17:07:48Z

While thinking on it I tried to create the captures in Lua to reduce a complexity of the code. It seems to work.

Patch (collapsed).

diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index e5fc370e49..6245bba005 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -40,6 +40,9 @@
 
 static uint32_t CTID_STRUCT_KEY_DEF_REF = 0;
 
+static int
+lbox_key_def_gc(struct lua_State *L);
+
 void
 luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
 {
@@ -75,6 +78,35 @@ luaT_push_key_def(struct lua_State *L, const struct key_def *key_def)
 		}
 		lua_rawseti(L, -2, i + 1);
 	}
+
+	luaL_loadstring(L, "do"
+			"    local key_def = ..."
+			"    return {"
+			"        __index = {"
+			"            extract_key = function(_self, ...)"
+			"                return key_def:extract_key(...)"
+			"            end,"
+			"            compare = function(_self, ...)"
+			"                return key_def:compare(...)"
+			"            end,"
+			"            compare_with_key = function(_self, ...)"
+			"                return key_def:compare_with_key(...)"
+			"            end,"
+			"            merge = function(_self, ...)"
+			"                return key_def:merge(...)"
+			"            end,"
+			"        }"
+			"    }"
+			"end");
+
+	*(struct key_def **) luaL_pushcdata(L,
+				CTID_STRUCT_KEY_DEF_REF) = key_def_dup(key_def);
+	lua_pushcfunction(L, lbox_key_def_gc);
+	luaL_setcdatagc(L, -2);
+
+	lua_call(L, 1, 1);
+
+	lua_setmetatable(L, -2);
 }
 
 /**

NB: We can do some optimizations here: at least do luaL_loadstring() once, save a reference in the Lua registry and reuse the function.

NB: key_def_dup() is because keydef's are only copying and not refcounted. We have no control over the original keydef lifetime, so we should copy it and free when unneeded.

This code passes the test except error messages:

Patch on the test (collapsed).

--- a/test/box-luatest/gh_7356_index_parts_methods_test.lua	2023-02-28 19:50:56.320584381 +0300
+++ b/test/box-luatest/gh_7356_index_parts_methods_test.lua	2023-02-28 19:51:04.320584448 +0300
@@ -29,10 +29,10 @@
         t.assert_equals(k, {'X', 1})
 
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:extract_key(tuple)',
+            'Usage: key_def:extract_key(tuple)',
             sk.parts.extract_key)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:extract_key(tuple)',
+            'Usage: key_def:extract_key(tuple)',
             sk.parts.extract_key, sk.parts)
         t.assert_error_msg_content_equals(
             'A tuple or a table expected, got number',
@@ -60,10 +60,10 @@
         t.assert_equals(i.parts.compare(nil, tuple_c, tuple_a), 1)
 
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:compare(tuple_a, tuple_b)',
+            'Usage: key_def:compare(tuple_a, tuple_b)',
             i.parts.compare)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:compare(tuple_a, tuple_b)',
+            'Usage: key_def:compare(tuple_a, tuple_b)',
             i.parts.compare, i.parts, tuple_a)
         t.assert_error_msg_content_equals(
             'A tuple or a table expected, got cdata',
@@ -89,13 +89,13 @@
         t.assert_equals(i.parts:compare_with_key(tuple, {'X', 2}), -1)
 
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:compare_with_key(tuple, key)',
+            'Usage: key_def:compare_with_key(tuple, key)',
             i.parts.compare_with_key)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:compare_with_key(tuple, key)',
+            'Usage: key_def:compare_with_key(tuple, key)',
             i.parts.compare_with_key, box.NULL)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:compare_with_key(tuple, key)',
+            'Usage: key_def:compare_with_key(tuple, key)',
             i.parts.compare_with_key, box.NULL, {0, nil, ''})
         t.assert_error_msg_content_equals(
             'Supplied key type of part 1 does not match index part type: ' ..
@@ -120,13 +120,13 @@
         t.assert_equals(i.parts:merge(kd2):totable(), exp)
 
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:merge(second_key_def)',
+            'Usage: key_def:merge(second_key_def)',
             i.parts.merge)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:merge(second_key_def)',
+            'Usage: key_def:merge(second_key_def)',
             i.parts.merge, i.parts)
         t.assert_error_msg_content_equals(
-            'Usage: index.parts:merge(second_key_def)',
+            'Usage: key_def:merge(second_key_def)',
             i.parts.merge, i.parts, kd2, box.NULL)
     end)
 end

It is not proposal to do it this way, just some stuff to think around.

Another possible way is to just add a field _key_def and use it in the functions instead of a capture. We'll need to hide it from serialization using __serialize (just like we do in src/lua/init.lua). And it is not so clean as captures. But it may be also acceptable.

src/box/lua/key_def.c

src/box/lua/space.cc

test/box-luatest/gh_7356_index_parts_methods_test.lua

src/box/lua/key_def.h

@TarantoolBot

This patch adds 4 methods of the key_def module instance to the `index_object.parts`, see the docbot request for details. The rest methods (new() and totable()) are not applicable here, because the instance is already created and pushed to the stack as a table. Closes #7356 @TarantoolBot document Title: `index_object.parts` methods Product: Tarantool Since: 3.0 Root document: https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_index/parts/ `index_object.parts` has the following methods: `extract_key()`, `compare()`, `compare_with_key()`, `merge()`. For their description and usage examples, refer to [Module key_def](https://www.tarantool.io/en/doc/latest/reference/reference_lua/key_def/#lua-object.key_def.key_def_object). --- Root document: https://www.tarantool.io/en/doc/latest/reference/reference_lua/key_def/ `index_object.parts` can be used like a key_def module instance for calling the corresponding methods. Example: ```lua box.schema.space.create('T') i = box.space.T:create_index('I', {parts={3, 'string', 1, 'unsigned'}}) box.space.T:insert{1, 99.5, 'X', nil, 99.5} i.parts:extract_key(box.space.T:get({'X', 1})) ``` The code above is equivalent to: ```lua key_def = require('key_def') box.schema.space.create('T') i = box.space.T:create_index('I', {parts={3, 'string', 1, 'unsigned'}}) box.space.T:insert{1, 99.5, 'X', nil, 99.5} k = key_def.new(i.parts) k:extract_key(box.space.T:get({'X', 1})) ```

Totktonada reviewed Feb 19, 2023

View reviewed changes

src/box/lua/key_def.c Outdated Show resolved Hide resolved

Gumix assigned Totktonada Feb 19, 2023

Gumix requested a review from Totktonada February 19, 2023 22:56

Totktonada reviewed Feb 19, 2023

View reviewed changes

src/box/lua/key_def.c Outdated Show resolved Hide resolved

Totktonada reviewed Feb 19, 2023

View reviewed changes

src/box/lua/space.cc Show resolved Hide resolved

Gumix assigned Gumix and unassigned Totktonada Feb 23, 2023

Gumix marked this pull request as draft February 23, 2023 23:36

Gumix marked this pull request as ready for review March 4, 2023 12:40

Gumix requested a review from Totktonada March 4, 2023 12:52

Gumix assigned Totktonada and unassigned Gumix Mar 4, 2023

Totktonada reviewed Mar 22, 2023

View reviewed changes

Totktonada assigned Gumix and unassigned Totktonada Mar 22, 2023

Totktonada reviewed Mar 22, 2023

View reviewed changes

test/box-luatest/gh_7356_index_parts_methods_test.lua Show resolved Hide resolved

Gumix requested a review from a team as a code owner April 7, 2023 20:17

Gumix requested a review from Totktonada April 7, 2023 20:29

Gumix assigned Totktonada and unassigned Gumix Apr 7, 2023

Totktonada reviewed Apr 7, 2023

View reviewed changes

src/box/lua/key_def.h Outdated Show resolved Hide resolved

Gumix requested a review from Totktonada April 8, 2023 10:51

p7nov approved these changes Apr 10, 2023

View reviewed changes

Totktonada approved these changes Apr 14, 2023

View reviewed changes

Totktonada assigned Gumix and unassigned Totktonada Apr 14, 2023

Gumix requested a review from locker April 14, 2023 11:20

Gumix assigned locker and unassigned Gumix Apr 14, 2023

locker reviewed Apr 14, 2023

View reviewed changes

src/box/lua/key_def.h Outdated Show resolved Hide resolved

locker reviewed Apr 14, 2023

View reviewed changes

src/box/lua/key_def.h Show resolved Hide resolved

locker assigned Gumix and unassigned locker Apr 17, 2023

Gumix requested a review from locker April 17, 2023 12:06

Gumix assigned locker and unassigned Gumix Apr 17, 2023

locker approved these changes Apr 18, 2023

View reviewed changes

locker added the full-ci Enables all tests for a pull request label Apr 18, 2023

locker merged commit 55295f5 into tarantool:master Apr 18, 2023

Gumix mentioned this pull request Sep 7, 2023

lua/fiber: do not raise on printing a dead fiber #9099

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

box: populate `index_object.parts` with `key_def` module methods#8356

box: populate `index_object.parts` with `key_def` module methods#8356
locker merged 1 commit intotarantool:masterfrom
Gumix:iverbin/gh-7356-populate-index-with-keydef

Gumix commented Feb 19, 2023 •

edited

Loading

Uh oh!

Uh oh!

coveralls commented Feb 19, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

olegrok commented Feb 23, 2023

Uh oh!

Totktonada commented Feb 28, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

Gumix commented Feb 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

coveralls commented Feb 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olegrok commented Feb 23, 2023

Uh oh!

Totktonada commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Gumix commented Feb 19, 2023 •

edited

Loading

coveralls commented Feb 19, 2023 •

edited

Loading

Totktonada commented Feb 28, 2023 •

edited

Loading