Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org,
	Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx
Date: Tue, 7 May 2019 11:28:46 +0300	[thread overview]
Message-ID: <1bedf880-3e47-4662-258c-dbc9326e0b44@tarantool.org> (raw)
In-Reply-To: <20190507081103.fnzqguaeev4hxrlt@esperanza>


>> +	assert(field_map_get_size((uint32_t *)raw,
>> +				  format->field_map_size) == field_map_size);
> 
> Could you please move this assertion to field_map_build so that it works
> for all engines, not only for memtx?

Partially this new condition is less reliable: we use the same components
that were used a bit before on build. Now it is a check that _build action
and _get_size are compatible; I don't mind.

diff --git a/src/box/field_map.c b/src/box/field_map.c
index a917d4a25..fd696c198 100644
--- a/src/box/field_map.c
+++ b/src/box/field_map.c
@@ -114,6 +114,9 @@ field_map_build(struct field_map_builder *builder, char *buffer)
 		extent_wptr += sizeof(uint32_t) + extent_offset_sz;
 	}
 	assert(extent_wptr == buffer + builder->extents_size);
+	assert(field_map_get_size(field_map,
+				  builder->slot_count * sizeof(uint32_t)) ==
+	       field_map_build_size(builder));
 }
 
 uint32_t
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index ab4913035..806a79c5a 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -1180,8 +1180,6 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
 	char *raw = (char *) tuple + tuple->data_offset;
 	field_map_build(&builder, raw - field_map_size);
 	memcpy(raw, data, tuple_len);
-	assert(field_map_get_size((uint32_t *)raw,
-				  format->field_map_size) == field_map_size);
 	say_debug("%s(%zu) = %p", __func__, tuple_len, memtx_tuple);
 end:
 	region_truncate(region, region_svp);


> 
> Also, we need to make sure that the key_def module doesn't allow to
> create multikey key definitions, as its API doesn't allow to compare
> multikey indexes. Please do it in the scope of this patch as well.
> May be, we will extend the API later to allow that, but I think we
> can live without it for now.

diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index 0e1236093..b4bd64f59 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -179,6 +179,11 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part,
 			diag_set(IllegalParams, "invalid path");
 			return -1;
 		}
+		if ((size_t)json_path_multikey_offset(path, path_len,
+					      TUPLE_INDEX_BASE) != path_len) {
+			diag_set(IllegalParams, "multikey path is unsupported");
+			return -1;
+		}
 		char *tmp = region_alloc(region, path_len + 1);
 		if (tmp == NULL) {
 			diag_set(OutOfMemory, path_len + 1, "region", "path");

diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
index c52ff48fe..4b468a696 100755
--- a/test/box-tap/key_def.test.lua
+++ b/test/box-tap/key_def.test.lua
@@ -128,6 +128,15 @@ local key_def_new_cases = {
         }},
         exp_err = 'invalid path',
     },
+    {
+        'Multikey JSON path',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            path = '[*]',
+        }},
+        exp_err = 'multikey path is unsupported',
+    },
     {
         'Success case; one part',
         parts = {{

  reply	other threads:[~2019-05-07  8:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06 11:57 [PATCH v5 0/4] " Kirill Shcherbatov
2019-05-06 11:57 ` [PATCH v5 1/4] box: introduce tuple_format_iterator class Kirill Shcherbatov
2019-05-06 13:55   ` Vladimir Davydov
2019-05-06 14:32     ` [tarantool-patches] " Kirill Shcherbatov
2019-05-06 11:57 ` [PATCH v5 2/4] box: introduce field_map_builder class Kirill Shcherbatov
2019-05-06 14:22   ` Vladimir Davydov
2019-05-06 11:57 ` [PATCH v5 3/4] salad: introduce bps_tree_delete_identical routine Kirill Shcherbatov
2019-05-06 14:34   ` Vladimir Davydov
2019-05-06 14:55     ` [tarantool-patches] " Kirill Shcherbatov
2019-05-06 11:57 ` [PATCH v5 4/4] box: introduce multikey indexes in memtx Kirill Shcherbatov
2019-05-06 15:46   ` Vladimir Davydov
2019-05-06 16:35     ` [tarantool-patches] " Kirill Shcherbatov
2019-05-07  8:11       ` Vladimir Davydov
2019-05-07  8:28         ` Kirill Shcherbatov [this message]
2019-05-07 11:30           ` Vladimir Davydov
2019-05-07 13:13     ` Vladimir Davydov
2019-05-06 15:52 ` [PATCH v5 0/4] " Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1bedf880-3e47-4662-258c-dbc9326e0b44@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox