* [PATCH 1/7] Make tuple comparison hints mandatory
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-09 5:58 ` [tarantool-patches] " Konstantin Osipov
2019-05-08 17:22 ` [PATCH 2/7] Get rid of tuple_field_by_part_multikey Vladimir Davydov
` (7 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
There isn't much point in having separate versions of tuple comparators
that don't take tuple comparison hints anymore, because all hot paths
have been patched to use the hinted versions. Besides un-hinted versions
don't make sense for multikey indexes, which use hints to store offsets
in multikey arrays. Let's strip the _hinted suffix from all hinted
comparators and zap un-hinted versions. In a few remaining places in the
code that still use un-hinted versions, let's simply pass HINT_NONE.
---
src/box/key_def.c | 5 +-
src/box/key_def.h | 99 ++++----------------
src/box/lua/key_def.c | 5 +-
src/box/memtx_hash.c | 7 +-
src/box/memtx_space.c | 4 +-
src/box/memtx_tree.c | 41 ++++-----
src/box/space.c | 3 +-
src/box/sql/analyze.c | 5 +-
src/box/tuple_compare.cc | 231 ++++++++++-------------------------------------
src/box/vinyl.c | 9 +-
src/box/vy_range.c | 3 +-
src/box/vy_stmt.h | 81 ++++-------------
src/box/vy_upsert.c | 3 +-
13 files changed, 133 insertions(+), 363 deletions(-)
diff --git a/src/box/key_def.c b/src/box/key_def.c
index d9e8e7a1..fc547570 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -350,7 +350,7 @@ int
box_tuple_compare(box_tuple_t *tuple_a, box_tuple_t *tuple_b,
box_key_def_t *key_def)
{
- return tuple_compare(tuple_a, tuple_b, key_def);
+ return tuple_compare(tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def);
}
int
@@ -358,7 +358,8 @@ box_tuple_compare_with_key(box_tuple_t *tuple_a, const char *key_b,
box_key_def_t *key_def)
{
uint32_t part_count = mp_decode_array(&key_b);
- return tuple_compare_with_key(tuple_a, key_b, part_count, key_def);
+ return tuple_compare_with_key(tuple_a, HINT_NONE, key_b,
+ part_count, HINT_NONE, key_def);
}
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 7b3d03c2..5f65f501 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -130,27 +130,18 @@ key_part_is_nullable(const struct key_part *part)
}
/** @copydoc tuple_compare_with_key() */
-typedef int (*tuple_compare_with_key_t)(struct tuple *tuple_a,
+typedef int (*tuple_compare_with_key_t)(struct tuple *tuple,
+ hint_t tuple_hint,
const char *key,
uint32_t part_count,
+ hint_t key_hint,
struct key_def *key_def);
-/** @copydoc tuple_compare_with_key_hinted() */
-typedef int (*tuple_compare_with_key_hinted_t)(struct tuple *tuple,
- hint_t tuple_hint,
- const char *key,
- uint32_t part_count,
- hint_t key_hint,
- struct key_def *key_def);
/** @copydoc tuple_compare() */
typedef int (*tuple_compare_t)(struct tuple *tuple_a,
+ hint_t tuple_a_hint,
struct tuple *tuple_b,
+ hint_t tuple_b_hint,
struct key_def *key_def);
-/** @copydoc tuple_compare_hinted() */
-typedef int (*tuple_compare_hinted_t)(struct tuple *tuple_a,
- hint_t tuple_a_hint,
- struct tuple *tuple_b,
- hint_t tuple_b_hint,
- struct key_def *key_def);
/** @copydoc tuple_extract_key() */
typedef char *(*tuple_extract_key_t)(struct tuple *tuple,
struct key_def *key_def,
@@ -177,12 +168,8 @@ typedef hint_t (*key_hint_t)(const char *key, uint32_t part_count,
struct key_def {
/** @see tuple_compare() */
tuple_compare_t tuple_compare;
- /** @see tuple_compare_hinted() */
- tuple_compare_hinted_t tuple_compare_hinted;
/** @see tuple_compare_with_key() */
tuple_compare_with_key_t tuple_compare_with_key;
- /** @see tuple_compare_with_key_hinted() */
- tuple_compare_with_key_hinted_t tuple_compare_with_key_hinted;
/** @see tuple_extract_key() */
tuple_extract_key_t tuple_extract_key;
/** @see tuple_extract_key_raw() */
@@ -601,21 +588,6 @@ tuple_extract_key_raw(const char *data, const char *data_end,
}
/**
- * Compare keys using the key definition.
- * @param key_a key parts with MessagePack array header
- * @param part_count_a the number of parts in the key_a
- * @param key_b key_parts with MessagePack array header
- * @param part_count_b the number of parts in the key_b
- * @param key_def key definition
- *
- * @retval 0 if key_a == key_b
- * @retval <0 if key_a < key_b
- * @retval >0 if key_a > key_b
- */
-int
-key_compare(const char *key_a, const char *key_b, struct key_def *key_def);
-
-/**
* Compare keys using the key definition and comparison hints.
* @param key_a key parts with MessagePack array header
* @param key_a_hint comparison hint of @a key_a
@@ -628,25 +600,9 @@ key_compare(const char *key_a, const char *key_b, struct key_def *key_def);
* @retval >0 if key_a > key_b
*/
int
-key_compare_hinted(const char *key_a, hint_t key_a_hint,
- const char *key_b, hint_t key_b_hint,
- struct key_def *key_def);
-
-/**
- * Compare tuples using the key definition.
- * @param tuple_a first tuple
- * @param tuple_b second tuple
- * @param key_def key definition
- * @retval 0 if key_fields(tuple_a) == key_fields(tuple_b)
- * @retval <0 if key_fields(tuple_a) < key_fields(tuple_b)
- * @retval >0 if key_fields(tuple_a) > key_fields(tuple_b)
- */
-static inline int
-tuple_compare(struct tuple *tuple_a, struct tuple *tuple_b,
- struct key_def *key_def)
-{
- return key_def->tuple_compare(tuple_a, tuple_b, key_def);
-}
+key_compare(const char *key_a, hint_t key_a_hint,
+ const char *key_b, hint_t key_b_hint,
+ struct key_def *key_def);
/**
* Compare tuples using the key definition and comparison hints.
@@ -660,30 +616,12 @@ tuple_compare(struct tuple *tuple_a, struct tuple *tuple_b,
* @retval >0 if key_fields(tuple_a) > key_fields(tuple_b)
*/
static inline int
-tuple_compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
- struct tuple *tuple_b, hint_t tuple_b_hint,
- struct key_def *key_def)
+tuple_compare(struct tuple *tuple_a, hint_t tuple_a_hint,
+ struct tuple *tuple_b, hint_t tuple_b_hint,
+ struct key_def *key_def)
{
- return key_def->tuple_compare_hinted(tuple_a, tuple_a_hint, tuple_b,
- tuple_b_hint, key_def);
-}
-
-/**
- * @brief Compare tuple with key using the key definition.
- * @param tuple tuple
- * @param key key parts without MessagePack array header
- * @param part_count the number of parts in @a key
- * @param key_def key definition
- *
- * @retval 0 if key_fields(tuple) == parts(key)
- * @retval <0 if key_fields(tuple) < parts(key)
- * @retval >0 if key_fields(tuple) > parts(key)
- */
-static inline int
-tuple_compare_with_key(struct tuple *tuple, const char *key,
- uint32_t part_count, struct key_def *key_def)
-{
- return key_def->tuple_compare_with_key(tuple, key, part_count, key_def);
+ return key_def->tuple_compare(tuple_a, tuple_a_hint,
+ tuple_b, tuple_b_hint, key_def);
}
/**
@@ -700,13 +638,12 @@ tuple_compare_with_key(struct tuple *tuple, const char *key,
* @retval >0 if key_fields(tuple) > parts(key)
*/
static inline int
-tuple_compare_with_key_hinted(struct tuple *tuple, hint_t tuple_hint,
- const char *key, uint32_t part_count,
- hint_t key_hint, struct key_def *key_def)
+tuple_compare_with_key(struct tuple *tuple, hint_t tuple_hint,
+ const char *key, uint32_t part_count,
+ hint_t key_hint, struct key_def *key_def)
{
- return key_def->tuple_compare_with_key_hinted(tuple, tuple_hint, key,
- part_count, key_hint,
- key_def);
+ return key_def->tuple_compare_with_key(tuple, tuple_hint, key,
+ part_count, key_hint, key_def);
}
/**
diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index b4bd64f5..72bb05c4 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -305,7 +305,7 @@ lbox_key_def_compare(struct lua_State *L)
return luaT_error(L);
}
- int rc = tuple_compare(tuple_a, tuple_b, key_def);
+ int rc = tuple_compare(tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def);
tuple_unref(tuple_a);
tuple_unref(tuple_b);
lua_pushinteger(L, rc);
@@ -345,7 +345,8 @@ lbox_key_def_compare_with_key(struct lua_State *L)
return luaT_error(L);
}
- int rc = tuple_compare_with_key(tuple, key, part_count, key_def);
+ int rc = tuple_compare_with_key(tuple, HINT_NONE, key,
+ part_count, HINT_NONE, key_def);
region_truncate(region, region_svp);
tuple_unref(tuple);
lua_pushinteger(L, rc);
diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
index ff478bdc..3174ae2c 100644
--- a/src/box/memtx_hash.c
+++ b/src/box/memtx_hash.c
@@ -44,15 +44,16 @@ static inline bool
memtx_hash_equal(struct tuple *tuple_a, struct tuple *tuple_b,
struct key_def *key_def)
{
- return tuple_compare(tuple_a, tuple_b, key_def) == 0;
+ return tuple_compare(tuple_a, HINT_NONE,
+ tuple_b, HINT_NONE, key_def) == 0;
}
static inline bool
memtx_hash_equal_key(struct tuple *tuple, const char *key,
struct key_def *key_def)
{
- return tuple_compare_with_key(tuple, key, key_def->part_count,
- key_def) == 0;
+ return tuple_compare_with_key(tuple, HINT_NONE, key, key_def->part_count,
+ HINT_NONE, key_def) == 0;
}
#define LIGHT_NAME _index
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index c3c9ac79..4d5e7991 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -513,8 +513,8 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
struct index *pk = space->index[0];
if (!key_update_can_be_skipped(pk->def->key_def->column_mask,
column_mask) &&
- tuple_compare(old_tuple, stmt->new_tuple,
- pk->def->key_def) != 0) {
+ tuple_compare(old_tuple, HINT_NONE, stmt->new_tuple,
+ HINT_NONE, pk->def->key_def) != 0) {
/* Primary key is changed: log error and do nothing. */
diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
pk->def->name, space_name(space));
diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c
index a01aed48..268ce405 100644
--- a/src/box/memtx_tree.c
+++ b/src/box/memtx_tree.c
@@ -80,11 +80,10 @@ memtx_tree_data_identical(const struct memtx_tree_data *a,
#define BPS_TREE_BLOCK_SIZE (512)
#define BPS_TREE_EXTENT_SIZE MEMTX_EXTENT_SIZE
#define BPS_TREE_COMPARE(a, b, arg)\
- tuple_compare_hinted((&a)->tuple, (&a)->hint, (&b)->tuple,\
- (&b)->hint, arg)
+ tuple_compare((&a)->tuple, (&a)->hint, (&b)->tuple, (&b)->hint, arg)
#define BPS_TREE_COMPARE_KEY(a, b, arg)\
- tuple_compare_with_key_hinted((&a)->tuple, (&a)->hint, (b)->key,\
- (b)->part_count, (b)->hint, arg)
+ tuple_compare_with_key((&a)->tuple, (&a)->hint, (b)->key,\
+ (b)->part_count, (b)->hint, arg)
#define BPS_TREE_IDENTICAL(a, b) memtx_tree_data_identical(&a, &b)
#define bps_tree_elem_t struct memtx_tree_data
#define bps_tree_key_t struct memtx_tree_key_data *
@@ -125,8 +124,8 @@ memtx_tree_qcompare(const void* a, const void *b, void *c)
const struct memtx_tree_data *data_a = a;
const struct memtx_tree_data *data_b = b;
struct key_def *key_def = c;
- return tuple_compare_hinted(data_a->tuple, data_a->hint, data_b->tuple,
- data_b->hint, key_def);
+ return tuple_compare(data_a->tuple, data_a->hint, data_b->tuple,
+ data_b->hint, key_def);
}
/* {{{ MemtxTree Iterators ****************************************/
@@ -248,11 +247,11 @@ tree_iterator_next_equal(struct iterator *iterator, struct tuple **ret)
memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator);
/* Use user key def to save a few loops. */
if (res == NULL ||
- tuple_compare_with_key_hinted(res->tuple, res->hint,
- it->key_data.key,
- it->key_data.part_count,
- it->key_data.hint,
- it->index_def->key_def) != 0) {
+ tuple_compare_with_key(res->tuple, res->hint,
+ it->key_data.key,
+ it->key_data.part_count,
+ it->key_data.hint,
+ it->index_def->key_def) != 0) {
iterator->next = tree_iterator_dummie;
it->current.tuple = NULL;
*ret = NULL;
@@ -281,11 +280,11 @@ tree_iterator_prev_equal(struct iterator *iterator, struct tuple **ret)
memtx_tree_iterator_get_elem(it->tree, &it->tree_iterator);
/* Use user key def to save a few loops. */
if (res == NULL ||
- tuple_compare_with_key_hinted(res->tuple, res->hint,
- it->key_data.key,
- it->key_data.part_count,
- it->key_data.hint,
- it->index_def->key_def) != 0) {
+ tuple_compare_with_key(res->tuple, res->hint,
+ it->key_data.key,
+ it->key_data.part_count,
+ it->key_data.hint,
+ it->index_def->key_def) != 0) {
iterator->next = tree_iterator_dummie;
it->current.tuple = NULL;
*ret = NULL;
@@ -861,11 +860,11 @@ memtx_tree_index_build_array_deduplicate(struct memtx_tree_index *index)
while (r_idx < index->build_array_size) {
if (index->build_array[w_idx].tuple !=
index->build_array[r_idx].tuple ||
- tuple_compare_hinted(index->build_array[w_idx].tuple,
- index->build_array[w_idx].hint,
- index->build_array[r_idx].tuple,
- index->build_array[r_idx].hint,
- cmp_def) != 0) {
+ tuple_compare(index->build_array[w_idx].tuple,
+ index->build_array[w_idx].hint,
+ index->build_array[r_idx].tuple,
+ index->build_array[r_idx].hint,
+ cmp_def) != 0) {
/* Do not override the element itself. */
if (++w_idx == r_idx)
continue;
diff --git a/src/box/space.c b/src/box/space.c
index 54ba97fb..edfc5d60 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -455,7 +455,8 @@ space_before_replace(struct space *space, struct txn *txn,
*/
if (pk != NULL && request_changed &&
old_tuple != NULL && new_tuple != NULL &&
- tuple_compare(old_tuple, new_tuple, pk->def->key_def) != 0) {
+ tuple_compare(old_tuple, HINT_NONE, new_tuple, HINT_NONE,
+ pk->def->key_def) != 0) {
diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
pk->def->name, space->def->name);
rc = -1;
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 0c02050a..d2d29c5f 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1346,8 +1346,9 @@ static int
sample_compare(const void *a, const void *b, void *arg)
{
struct key_def *def = (struct key_def *)arg;
- return key_compare(((struct index_sample *) a)->sample_key,
- ((struct index_sample *) b)->sample_key, def);
+ return key_compare(((struct index_sample *) a)->sample_key, HINT_NONE,
+ ((struct index_sample *) b)->sample_key, HINT_NONE,
+ def);
}
/**
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index af9aa7b6..9a3780c6 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -375,29 +375,6 @@ mp_compare_scalar_coll(const char *field_a, const char *field_b,
return mp_compare_scalar_with_type(field_a, type_a, field_b, type_b);
}
-static inline int
-tuple_compare_multikey(struct tuple *tuple_a, struct tuple *tuple_b,
- struct key_def *key_def)
-{
- (void)tuple_a;
- (void)tuple_b;
- (void)key_def;
- unreachable();
- return 0;
-}
-
-static inline int
-tuple_compare_with_key_multikey(struct tuple *tuple, const char *key,
- uint32_t part_count, struct key_def *key_def)
-{
- (void) tuple;
- (void) key;
- (void) part_count;
- (void) key_def;
- unreachable();
- return 0;
-}
-
/**
* @brief Compare two fields parts using a type definition
* @param field_a field
@@ -471,9 +448,9 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
template<bool is_nullable, bool has_optional_parts, bool has_json_paths,
bool is_multikey>
static inline int
-tuple_compare_slowpath_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
- struct tuple *tuple_b, hint_t tuple_b_hint,
- struct key_def *key_def)
+tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
+ struct tuple *tuple_b, hint_t tuple_b_hint,
+ struct key_def *key_def)
{
assert(has_json_paths == key_def->has_json_paths);
assert(!has_optional_parts || is_nullable);
@@ -621,22 +598,12 @@ tuple_compare_slowpath_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
return 0;
}
-template<bool is_nullable, bool has_optional_parts, bool has_json_paths>
-static inline int
-tuple_compare_slowpath(struct tuple *tuple_a, struct tuple *tuple_b,
- struct key_def *key_def)
-{
- return tuple_compare_slowpath_hinted
- <is_nullable, has_optional_parts, has_json_paths, false>
- (tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def);
-}
-
template<bool is_nullable, bool has_optional_parts, bool has_json_paths,
bool is_multikey>
static inline int
-tuple_compare_with_key_slowpath_hinted(struct tuple *tuple,
- hint_t tuple_hint, const char *key, uint32_t part_count,
- hint_t key_hint, struct key_def *key_def)
+tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint,
+ const char *key, uint32_t part_count,
+ hint_t key_hint, struct key_def *key_def)
{
assert(has_json_paths == key_def->has_json_paths);
assert(!has_optional_parts || is_nullable);
@@ -731,16 +698,6 @@ tuple_compare_with_key_slowpath_hinted(struct tuple *tuple,
return 0;
}
-template<bool is_nullable, bool has_optional_parts, bool has_json_paths>
-static inline int
-tuple_compare_with_key_slowpath(struct tuple *tuple, const char *key,
- uint32_t part_count, struct key_def *key_def)
-{
- return tuple_compare_with_key_slowpath_hinted
- <is_nullable, has_optional_parts, has_json_paths, false>
- (tuple, HINT_NONE, key, part_count, HINT_NONE, key_def);
-}
-
template<bool is_nullable>
static inline int
key_compare_parts(const char *key_a, const char *key_b, uint32_t part_count,
@@ -799,9 +756,9 @@ key_compare_parts(const char *key_a, const char *key_b, uint32_t part_count,
template<bool is_nullable, bool has_optional_parts>
static inline int
-tuple_compare_with_key_sequential_hinted(struct tuple *tuple,
- hint_t tuple_hint, const char *key, uint32_t part_count,
- hint_t key_hint, struct key_def *key_def)
+tuple_compare_with_key_sequential(struct tuple *tuple, hint_t tuple_hint,
+ const char *key, uint32_t part_count,
+ hint_t key_hint, struct key_def *key_def)
{
assert(!has_optional_parts || is_nullable);
assert(key_def_is_sequential(key_def));
@@ -842,19 +799,13 @@ tuple_compare_with_key_sequential_hinted(struct tuple *tuple,
return 0;
}
-template<bool is_nullable, bool has_optional_parts>
-static inline int
-tuple_compare_with_key_sequential(struct tuple *tuple, const char *key,
- uint32_t part_count, struct key_def *key_def)
-{
- return tuple_compare_with_key_sequential_hinted
- <is_nullable, has_optional_parts>
- (tuple, HINT_NONE, key, part_count, HINT_NONE, key_def);
-}
-
int
-key_compare(const char *key_a, const char *key_b, struct key_def *key_def)
+key_compare(const char *key_a, hint_t key_a_hint,
+ const char *key_b, hint_t key_b_hint, struct key_def *key_def)
{
+ int rc = hint_cmp(key_a_hint, key_b_hint);
+ if (rc != 0)
+ return rc;
uint32_t part_count_a = mp_decode_array(&key_a);
uint32_t part_count_b = mp_decode_array(&key_b);
assert(part_count_a <= key_def->part_count);
@@ -870,22 +821,11 @@ key_compare(const char *key_a, const char *key_b, struct key_def *key_def)
}
}
-int
-key_compare_hinted(const char *key_a, hint_t key_a_hint,
- const char *key_b, hint_t key_b_hint,
- struct key_def *key_def)
-{
- int rc = hint_cmp(key_a_hint, key_b_hint);
- if (rc != 0)
- return rc;
- return key_compare(key_a, key_b, key_def);
-}
-
template <bool is_nullable, bool has_optional_parts>
static int
-tuple_compare_sequential_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
- struct tuple *tuple_b, hint_t tuple_b_hint,
- struct key_def *key_def)
+tuple_compare_sequential(struct tuple *tuple_a, hint_t tuple_a_hint,
+ struct tuple *tuple_b, hint_t tuple_b_hint,
+ struct key_def *key_def)
{
assert(!has_optional_parts || is_nullable);
assert(has_optional_parts == key_def->has_optional_parts);
@@ -953,16 +893,6 @@ tuple_compare_sequential_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
return 0;
}
-template <bool is_nullable, bool has_optional_parts>
-static int
-tuple_compare_sequential(struct tuple *tuple_a, struct tuple *tuple_b,
- struct key_def *key_def)
-{
- return tuple_compare_sequential_hinted
- <is_nullable, has_optional_parts>
- (tuple_a, HINT_NONE, tuple_b, HINT_NONE, key_def);
-}
-
template <int TYPE>
static inline int
field_compare(const char **field_a, const char **field_b);
@@ -1078,9 +1008,13 @@ struct FieldCompare<IDX, TYPE>
template <int IDX, int TYPE, int ...MORE_TYPES>
struct TupleCompare
{
- static int compare(struct tuple *tuple_a, struct tuple *tuple_b,
+ static int compare(struct tuple *tuple_a, hint_t tuple_a_hint,
+ struct tuple *tuple_b, hint_t tuple_b_hint,
struct key_def *)
{
+ int rc = hint_cmp(tuple_a_hint, tuple_b_hint);
+ if (rc != 0)
+ return rc;
struct tuple_format *format_a = tuple_format(tuple_a);
struct tuple_format *format_b = tuple_format(tuple_b);
const char *field_a, *field_b;
@@ -1092,23 +1026,17 @@ struct TupleCompare
compare(tuple_a, tuple_b, format_a,
format_b, field_a, field_b);
}
-
- static int compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
- struct tuple *tuple_b, hint_t tuple_b_hint,
- struct key_def *key_def)
- {
- int rc = hint_cmp(tuple_a_hint, tuple_b_hint);
- if (rc != 0)
- return rc;
- return compare(tuple_a, tuple_b, key_def);
- }
};
template <int TYPE, int ...MORE_TYPES>
struct TupleCompare<0, TYPE, MORE_TYPES...> {
- static int compare(struct tuple *tuple_a, struct tuple *tuple_b,
+ static int compare(struct tuple *tuple_a, hint_t tuple_a_hint,
+ struct tuple *tuple_b, hint_t tuple_b_hint,
struct key_def *)
{
+ int rc = hint_cmp(tuple_a_hint, tuple_b_hint);
+ if (rc != 0)
+ return rc;
struct tuple_format *format_a = tuple_format(tuple_a);
struct tuple_format *format_b = tuple_format(tuple_b);
const char *field_a = tuple_data(tuple_a);
@@ -1118,28 +1046,15 @@ struct TupleCompare<0, TYPE, MORE_TYPES...> {
return FieldCompare<0, TYPE, MORE_TYPES...>::compare(tuple_a, tuple_b,
format_a, format_b, field_a, field_b);
}
-
- static int compare_hinted(struct tuple *tuple_a, hint_t tuple_a_hint,
- struct tuple *tuple_b, hint_t tuple_b_hint,
- struct key_def *key_def)
- {
- int rc = hint_cmp(tuple_a_hint, tuple_b_hint);
- if (rc != 0)
- return rc;
- return compare(tuple_a, tuple_b, key_def);
- }
};
} /* end of anonymous namespace */
struct comparator_signature {
tuple_compare_t f;
- tuple_compare_hinted_t f_hinted;
uint32_t p[64];
};
#define COMPARATOR(...) \
- { TupleCompare<__VA_ARGS__>::compare, \
- TupleCompare<__VA_ARGS__>::compare_hinted, \
- { __VA_ARGS__, UINT32_MAX } },
+ { TupleCompare<__VA_ARGS__>::compare, { __VA_ARGS__, UINT32_MAX } },
/**
* field1 no, field1 type, field2 no, field2 type, ...
@@ -1277,12 +1192,16 @@ template <int FLD_ID, int IDX, int TYPE, int ...MORE_TYPES>
struct TupleCompareWithKey
{
static int
- compare(struct tuple *tuple, const char *key, uint32_t part_count,
- struct key_def *key_def)
+ compare(struct tuple *tuple, hint_t tuple_hint,
+ const char *key, uint32_t part_count,
+ hint_t key_hint, struct key_def *key_def)
{
/* Part count can be 0 in wildcard searches. */
if (part_count == 0)
return 0;
+ int rc = hint_cmp(tuple_hint, key_hint);
+ if (rc != 0)
+ return rc;
struct tuple_format *format = tuple_format(tuple);
const char *field = tuple_field_raw(format, tuple_data(tuple),
tuple_field_map(tuple),
@@ -1291,29 +1210,21 @@ struct TupleCompareWithKey
compare(tuple, key, part_count,
key_def, format, field);
}
-
- static int
- compare_hinted(struct tuple *tuple, hint_t tuple_hint,
- const char *key, uint32_t part_count, hint_t key_hint,
- struct key_def *key_def)
- {
- int rc = hint_cmp(tuple_hint, key_hint);
- if (rc != 0)
- return rc;
- return compare(tuple, key, part_count, key_def);
- }
};
template <int TYPE, int ...MORE_TYPES>
struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...>
{
- static int compare(struct tuple *tuple,
+ static int compare(struct tuple *tuple, hint_t tuple_hint,
const char *key, uint32_t part_count,
- struct key_def *key_def)
+ hint_t key_hint, struct key_def *key_def)
{
/* Part count can be 0 in wildcard searches. */
if (part_count == 0)
return 0;
+ int rc = hint_cmp(tuple_hint, key_hint);
+ if (rc != 0)
+ return rc;
struct tuple_format *format = tuple_format(tuple);
const char *field = tuple_data(tuple);
mp_decode_array(&field);
@@ -1321,17 +1232,6 @@ struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...>
compare(tuple, key, part_count,
key_def, format, field);
}
-
- static int
- compare_hinted(struct tuple *tuple, hint_t tuple_hint,
- const char *key, uint32_t part_count, hint_t key_hint,
- struct key_def *key_def)
- {
- int rc = hint_cmp(tuple_hint, key_hint);
- if (rc != 0)
- return rc;
- return compare(tuple, key, part_count, key_def);
- }
};
} /* end of anonymous namespace */
@@ -1339,14 +1239,11 @@ struct TupleCompareWithKey<0, 0, TYPE, MORE_TYPES...>
struct comparator_with_key_signature
{
tuple_compare_with_key_t f;
- tuple_compare_with_key_hinted_t f_hinted;
uint32_t p[64];
};
#define KEY_COMPARATOR(...) \
- { TupleCompareWithKey<0, __VA_ARGS__>::compare, \
- TupleCompareWithKey<0, __VA_ARGS__>::compare_hinted, \
- { __VA_ARGS__ } },
+ { TupleCompareWithKey<0, __VA_ARGS__>::compare, { __VA_ARGS__ } },
static const comparator_with_key_signature cmp_wk_arr[] = {
KEY_COMPARATOR(0, FIELD_TYPE_UNSIGNED, 1, FIELD_TYPE_UNSIGNED, 2, FIELD_TYPE_UNSIGNED)
@@ -1680,9 +1577,8 @@ key_hint_multikey(const char *key, uint32_t part_count, struct key_def *key_def)
/*
* Multikey hint for tuple is an index of the key in
* array, it always must be defined. While
- * tuple_hint_multikey, tuple_compare_multikey and
- * tuple_compare_with_key_multikey assume that it must
- * be initialized manually(so they mustn't be called),
+ * tuple_hint_multikey assumes that it must be
+ * initialized manually (so it mustn't be called),
* the virtual method for a key makes sense. Overriding
* this method such way, we extend existend code to
* do nothing on key hint calculation an it is valid
@@ -1764,9 +1660,7 @@ key_def_set_compare_func_fast(struct key_def *def)
assert(!key_def_has_collation(def));
tuple_compare_t cmp = NULL;
- tuple_compare_hinted_t cmp_hinted = NULL;
tuple_compare_with_key_t cmp_wk = NULL;
- tuple_compare_with_key_hinted_t cmp_wk_hinted = NULL;
bool is_sequential = key_def_is_sequential(def);
/*
@@ -1781,7 +1675,6 @@ key_def_set_compare_func_fast(struct key_def *def)
break;
if (i == def->part_count && cmp_arr[k].p[i * 2] == UINT32_MAX) {
cmp = cmp_arr[k].f;
- cmp_hinted = cmp_arr[k].f_hinted;
break;
}
}
@@ -1794,32 +1687,23 @@ key_def_set_compare_func_fast(struct key_def *def)
}
if (i == def->part_count) {
cmp_wk = cmp_wk_arr[k].f;
- cmp_wk_hinted = cmp_wk_arr[k].f_hinted;
break;
}
}
if (cmp == NULL) {
cmp = is_sequential ?
tuple_compare_sequential<false, false> :
- tuple_compare_slowpath<false, false, false>;
- cmp_hinted = is_sequential ?
- tuple_compare_sequential_hinted<false, false> :
- tuple_compare_slowpath_hinted<false, false, false, false>;
+ tuple_compare_slowpath<false, false, false, false>;
}
if (cmp_wk == NULL) {
cmp_wk = is_sequential ?
tuple_compare_with_key_sequential<false, false> :
- tuple_compare_with_key_slowpath<false, false, false>;
- cmp_wk_hinted = is_sequential ?
- tuple_compare_with_key_sequential_hinted<false, false> :
- tuple_compare_with_key_slowpath_hinted<false, false,
- false, false>;
+ tuple_compare_with_key_slowpath<false, false,
+ false, false>;
}
def->tuple_compare = cmp;
- def->tuple_compare_hinted = cmp_hinted;
def->tuple_compare_with_key = cmp_wk;
- def->tuple_compare_with_key_hinted = cmp_wk_hinted;
}
template<bool is_nullable, bool has_optional_parts>
@@ -1830,24 +1714,13 @@ key_def_set_compare_func_plain(struct key_def *def)
if (key_def_is_sequential(def)) {
def->tuple_compare = tuple_compare_sequential
<is_nullable, has_optional_parts>;
- def->tuple_compare_hinted = tuple_compare_sequential_hinted
- <is_nullable, has_optional_parts>;
def->tuple_compare_with_key = tuple_compare_with_key_sequential
<is_nullable, has_optional_parts>;
- def->tuple_compare_with_key_hinted =
- tuple_compare_with_key_sequential_hinted
- <is_nullable, has_optional_parts>;
} else {
def->tuple_compare = tuple_compare_slowpath
- <is_nullable, has_optional_parts, false>;
- def->tuple_compare_hinted = tuple_compare_slowpath_hinted
<is_nullable, has_optional_parts, false, false>;
def->tuple_compare_with_key = tuple_compare_with_key_slowpath
- <is_nullable, has_optional_parts, false>;
- def->tuple_compare_with_key_hinted =
- tuple_compare_with_key_slowpath_hinted
- <is_nullable, has_optional_parts,
- false, false>;
+ <is_nullable, has_optional_parts, false, false>;
}
}
@@ -1857,22 +1730,14 @@ key_def_set_compare_func_json(struct key_def *def)
{
assert(def->has_json_paths);
if (key_def_is_multikey(def)) {
- def->tuple_compare = tuple_compare_multikey;
- def->tuple_compare_hinted = tuple_compare_slowpath_hinted
+ def->tuple_compare = tuple_compare_slowpath
<is_nullable, has_optional_parts, true, true>;
- def->tuple_compare_with_key = tuple_compare_with_key_multikey;
- def->tuple_compare_with_key_hinted =
- tuple_compare_with_key_slowpath_hinted
+ def->tuple_compare_with_key = tuple_compare_with_key_slowpath
<is_nullable, has_optional_parts, true, true>;
} else {
def->tuple_compare = tuple_compare_slowpath
- <is_nullable, has_optional_parts, true>;
- def->tuple_compare_hinted = tuple_compare_slowpath_hinted
<is_nullable, has_optional_parts, true, false>;
def->tuple_compare_with_key = tuple_compare_with_key_slowpath
- <is_nullable, has_optional_parts, true>;
- def->tuple_compare_with_key_hinted =
- tuple_compare_with_key_slowpath_hinted
<is_nullable, has_optional_parts, true, false>;
}
}
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index f63383f8..6ffbecd9 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1366,7 +1366,8 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
* we must not use vy_entry_compare() here.
*/
if (result->stmt == NULL ||
- vy_stmt_compare(result->stmt, entry.stmt, lsm->cmp_def) != 0) {
+ vy_stmt_compare(result->stmt, HINT_NONE, entry.stmt,
+ HINT_NONE, lsm->cmp_def) != 0) {
/*
* If a tuple read from a secondary index doesn't
* match the tuple corresponding to it in the
@@ -1622,7 +1623,8 @@ vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
* fail here.
*/
if (found != NULL && vy_stmt_type(stmt) == IPROTO_REPLACE &&
- vy_stmt_compare(stmt, found, lsm->pk->key_def) == 0) {
+ vy_stmt_compare(stmt, HINT_NONE, found, HINT_NONE,
+ lsm->pk->key_def) == 0) {
tuple_unref(found);
return 0;
}
@@ -1827,7 +1829,8 @@ vy_check_update(struct space *space, const struct vy_lsm *pk,
uint64_t column_mask)
{
if (!key_update_can_be_skipped(pk->key_def->column_mask, column_mask) &&
- vy_stmt_compare(old_tuple, new_tuple, pk->key_def) != 0) {
+ vy_stmt_compare(old_tuple, HINT_NONE, new_tuple,
+ HINT_NONE, pk->key_def) != 0) {
diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
index_name_by_id(space, pk->index_id),
space_name(space));
diff --git a/src/box/vy_range.c b/src/box/vy_range.c
index 3c36d345..395639dc 100644
--- a/src/box/vy_range.c
+++ b/src/box/vy_range.c
@@ -478,7 +478,8 @@ vy_range_needs_split(struct vy_range *range, int64_t range_size,
slice->first_page_no);
/* No point in splitting if a new range is going to be empty. */
- if (key_compare(first_page->min_key, mid_page->min_key,
+ if (key_compare(first_page->min_key, first_page->min_key_hint,
+ mid_page->min_key, mid_page->min_key_hint,
range->cmp_def) == 0)
return false;
/*
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index cc8a9c5d..49fa1602 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -380,91 +380,50 @@ vy_stmt_hint(struct tuple *stmt, struct key_def *key_def)
/**
* Compare two vinyl statements taking into account their
- * formats (key or tuple).
- */
-static inline int
-vy_stmt_compare(struct tuple *a, struct tuple *b, struct key_def *key_def)
-{
- bool a_is_tuple = !vy_stmt_is_key(a);
- bool b_is_tuple = !vy_stmt_is_key(b);
- if (a_is_tuple && b_is_tuple) {
- return tuple_compare(a, b, key_def);
- } else if (a_is_tuple && !b_is_tuple) {
- const char *key = tuple_data(b);
- uint32_t part_count = mp_decode_array(&key);
- return tuple_compare_with_key(a, key, part_count, key_def);
- } else if (!a_is_tuple && b_is_tuple) {
- const char *key = tuple_data(a);
- uint32_t part_count = mp_decode_array(&key);
- return -tuple_compare_with_key(b, key, part_count, key_def);
- } else {
- assert(!a_is_tuple && !b_is_tuple);
- return key_compare(tuple_data(a), tuple_data(b), key_def);
- }
-}
-
-/**
- * Compare two vinyl statements taking into account their
* formats (key or tuple) and using comparison hints.
*/
static inline int
-vy_stmt_compare_hinted(struct tuple *a, hint_t a_hint,
- struct tuple *b, hint_t b_hint,
- struct key_def *key_def)
+vy_stmt_compare(struct tuple *a, hint_t a_hint,
+ struct tuple *b, hint_t b_hint,
+ struct key_def *key_def)
{
bool a_is_tuple = !vy_stmt_is_key(a);
bool b_is_tuple = !vy_stmt_is_key(b);
if (a_is_tuple && b_is_tuple) {
- return tuple_compare_hinted(a, a_hint, b, b_hint, key_def);
+ return tuple_compare(a, a_hint, b, b_hint, key_def);
} else if (a_is_tuple && !b_is_tuple) {
const char *key = tuple_data(b);
uint32_t part_count = mp_decode_array(&key);
- return tuple_compare_with_key_hinted(a, a_hint, key, part_count,
- b_hint, key_def);
+ return tuple_compare_with_key(a, a_hint, key, part_count,
+ b_hint, key_def);
} else if (!a_is_tuple && b_is_tuple) {
const char *key = tuple_data(a);
uint32_t part_count = mp_decode_array(&key);
- return -tuple_compare_with_key_hinted(b, b_hint, key, part_count,
- a_hint, key_def);
+ return -tuple_compare_with_key(b, b_hint, key, part_count,
+ a_hint, key_def);
} else {
assert(!a_is_tuple && !b_is_tuple);
- return key_compare_hinted(tuple_data(a), a_hint,
- tuple_data(b), b_hint, key_def);
+ return key_compare(tuple_data(a), a_hint,
+ tuple_data(b), b_hint, key_def);
}
}
/**
* Compare a vinyl statement (key or tuple) with a raw key
- * (msgpack array).
- */
-static inline int
-vy_stmt_compare_with_raw_key(struct tuple *stmt, const char *key,
- struct key_def *key_def)
-{
- if (!vy_stmt_is_key(stmt)) {
- uint32_t part_count = mp_decode_array(&key);
- return tuple_compare_with_key(stmt, key, part_count, key_def);
- }
- return key_compare(tuple_data(stmt), key, key_def);
-}
-
-/**
- * Compare a vinyl statement (key or tuple) with a raw key
* (msgpack array) using comparison hints.
*/
static inline int
-vy_stmt_compare_with_raw_key_hinted(struct tuple *stmt, hint_t stmt_hint,
- const char *key, hint_t key_hint,
- struct key_def *key_def)
+vy_stmt_compare_with_raw_key(struct tuple *stmt, hint_t stmt_hint,
+ const char *key, hint_t key_hint,
+ struct key_def *key_def)
{
if (!vy_stmt_is_key(stmt)) {
uint32_t part_count = mp_decode_array(&key);
- return tuple_compare_with_key_hinted(stmt, stmt_hint,
- key, part_count, key_hint,
- key_def);
+ return tuple_compare_with_key(stmt, stmt_hint, key,
+ part_count, key_hint,
+ key_def);
}
- return key_compare_hinted(tuple_data(stmt), stmt_hint,
- key, key_hint, key_def);
+ return key_compare(tuple_data(stmt), stmt_hint, key, key_hint, key_def);
}
/**
@@ -761,7 +720,7 @@ vy_entry_key_from_msgpack(struct tuple_format *format, struct key_def *key_def,
static inline int
vy_entry_compare(struct vy_entry a, struct vy_entry b, struct key_def *key_def)
{
- return vy_stmt_compare_hinted(a.stmt, a.hint, b.stmt, b.hint, key_def);
+ return vy_stmt_compare(a.stmt, a.hint, b.stmt, b.hint, key_def);
}
/**
@@ -773,8 +732,8 @@ vy_entry_compare_with_raw_key(struct vy_entry entry,
const char *key, hint_t key_hint,
struct key_def *key_def)
{
- return vy_stmt_compare_with_raw_key_hinted(entry.stmt, entry.hint,
- key, key_hint, key_def);
+ return vy_stmt_compare_with_raw_key(entry.stmt, entry.hint,
+ key, key_hint, key_def);
}
#if defined(__cplusplus)
diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c
index 2c693706..e5e53be7 100644
--- a/src/box/vy_upsert.c
+++ b/src/box/vy_upsert.c
@@ -206,7 +206,8 @@ check_key:
* Check that key hasn't been changed after applying operations.
*/
if (!key_update_can_be_skipped(cmp_def->column_mask, column_mask) &&
- vy_stmt_compare(old_stmt, result_stmt, cmp_def) != 0) {
+ vy_stmt_compare(old_stmt, HINT_NONE, result_stmt,
+ HINT_NONE, cmp_def) != 0) {
/*
* Key has been changed: ignore this UPSERT and
* @retval the old stmt.
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/7] Get rid of tuple_field_by_part_multikey
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
2019-05-08 17:22 ` [PATCH 1/7] Make tuple comparison hints mandatory Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-08 17:22 ` [PATCH 3/7] Make tuple_extract_key support multikey indexes Vladimir Davydov
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
Always require multikey_idx to be passed to tuple_field_by_part.
If the key definition isn't multikey, pass -1. Rationale: having
separate functions for multikey and unikey indexes doesn't have
any performance benefits (as all those functions are inline), but
makes the code inconsistent with other functions (e.g. tuple_compare)
which don't have a separate multikey variant. After all, passing -1
when the key definition is known to be unikey doesn't blow the code.
While we are at it, let's also add a few assertions ensuring that
the key definition isn't multikey to functions that don't support
multikey yet.
---
src/box/memtx_bitset.c | 6 ++---
src/box/memtx_rtree.c | 3 ++-
src/box/tuple.h | 29 +++++++-----------------
src/box/tuple_compare.cc | 53 +++++++++++++++++++++++---------------------
| 12 +++++-----
src/box/tuple_hash.cc | 17 ++++++++------
6 files changed, 58 insertions(+), 62 deletions(-)
diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
index b6f8f7ac..275e736d 100644
--- a/src/box/memtx_bitset.c
+++ b/src/box/memtx_bitset.c
@@ -276,6 +276,7 @@ memtx_bitset_index_replace(struct index *base, struct tuple *old_tuple,
struct memtx_bitset_index *index = (struct memtx_bitset_index *)base;
assert(!base->def->opts.is_unique);
+ assert(!key_def_is_multikey(base->def->key_def));
assert(old_tuple != NULL || new_tuple != NULL);
(void) mode;
@@ -300,9 +301,8 @@ memtx_bitset_index_replace(struct index *base, struct tuple *old_tuple,
}
if (new_tuple != NULL) {
- const char *field =
- tuple_field_by_part(new_tuple,
- base->def->key_def->parts);
+ const char *field = tuple_field_by_part(new_tuple,
+ base->def->key_def->parts, -1);
uint32_t key_len;
const void *key = make_key(field, &key_len);
#ifndef OLD_GOOD_BITSET
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index 0a903517..19cb69a1 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -120,8 +120,9 @@ extract_rectangle(struct rtree_rect *rect, struct tuple *tuple,
struct index_def *index_def)
{
assert(index_def->key_def->part_count == 1);
+ assert(!key_def_is_multikey(index_def->key_def));
const char *elems = tuple_field_by_part(tuple,
- index_def->key_def->parts);
+ index_def->key_def->parts, -1);
unsigned dimension = index_def->opts.dimension;
uint32_t count = mp_decode_array(&elems);
return mp_decode_rect(rect, dimension, elems, count, "Field");
diff --git a/src/box/tuple.h b/src/box/tuple.h
index c0d06dbe..2a46f371 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -653,9 +653,9 @@ tuple_field_raw_by_full_path(struct tuple_format *format, const char *tuple,
* @retval Field data if the field exists or NULL.
*/
static inline const char *
-tuple_field_raw_by_part_multikey(struct tuple_format *format, const char *data,
- const uint32_t *field_map,
- struct key_part *part, int multikey_idx)
+tuple_field_raw_by_part(struct tuple_format *format, const char *data,
+ const uint32_t *field_map,
+ struct key_part *part, int multikey_idx)
{
if (unlikely(part->format_epoch != format->epoch)) {
assert(format->epoch != 0);
@@ -672,32 +672,19 @@ tuple_field_raw_by_part_multikey(struct tuple_format *format, const char *data,
}
/**
- * Get a tuple field pointed to by an index part.
- * @param format Tuple format.
- * @param data A pointer to MessagePack array.
- * @param field_map A pointer to the LAST element of field map.
- * @param part Index part to use.
- * @retval Field data if the field exists or NULL.
- */
-static inline const char *
-tuple_field_raw_by_part(struct tuple_format *format, const char *data,
- const uint32_t *field_map, struct key_part *part)
-{
- return tuple_field_raw_by_part_multikey(format, data, field_map,
- part, -1);
-}
-
-/**
* Get a field refereed by index @part in tuple.
* @param tuple Tuple to get the field from.
* @param part Index part to use.
+ * @param multikey_idx A multikey index hint.
* @retval Field data if the field exists or NULL.
*/
static inline const char *
-tuple_field_by_part(struct tuple *tuple, struct key_part *part)
+tuple_field_by_part(struct tuple *tuple, struct key_part *part,
+ int multikey_idx)
{
return tuple_field_raw_by_part(tuple_format(tuple), tuple_data(tuple),
- tuple_field_map(tuple), part);
+ tuple_field_map(tuple), part,
+ multikey_idx);
}
/**
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 9a3780c6..0aa032af 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -504,17 +504,17 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
for (; part < end; part++) {
if (is_multikey) {
- field_a = tuple_field_raw_by_part_multikey(format_a,
- tuple_a_raw, field_map_a, part,
- (int)tuple_a_hint);
- field_b = tuple_field_raw_by_part_multikey(format_b,
- tuple_b_raw, field_map_b, part,
- (int)tuple_b_hint);
+ field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
+ field_map_a, part,
+ (int)tuple_a_hint);
+ field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
+ field_map_b, part,
+ (int)tuple_b_hint);
} else if (has_json_paths) {
field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
- field_map_a, part);
+ field_map_a, part, -1);
field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
- field_map_b, part);
+ field_map_b, part, -1);
} else {
field_a = tuple_field_raw(format_a, tuple_a_raw,
field_map_a, part->fieldno);
@@ -568,17 +568,17 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
end = key_def->parts + key_def->part_count;
for (; part < end; ++part) {
if (is_multikey) {
- field_a = tuple_field_raw_by_part_multikey(format_a,
- tuple_a_raw, field_map_a, part,
- (int)tuple_a_hint);
- field_b = tuple_field_raw_by_part_multikey(format_b,
- tuple_b_raw, field_map_b, part,
- (int)tuple_b_hint);
+ field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
+ field_map_a, part,
+ (int)tuple_a_hint);
+ field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
+ field_map_b, part,
+ (int)tuple_b_hint);
} else if (has_json_paths) {
field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
- field_map_a, part);
+ field_map_a, part, -1);
field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
- field_map_b, part);
+ field_map_b, part, -1);
} else {
field_a = tuple_field_raw(format_a, tuple_a_raw,
field_map_a, part->fieldno);
@@ -625,12 +625,12 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint,
if (likely(part_count == 1)) {
const char *field;
if (is_multikey) {
- field = tuple_field_raw_by_part_multikey(format,
- tuple_raw, field_map, part,
- (int)tuple_hint);
+ field = tuple_field_raw_by_part(format, tuple_raw,
+ field_map, part,
+ (int)tuple_hint);
} else if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part);
+ field_map, part, -1);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
@@ -659,12 +659,12 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint,
for (; part < end; ++part, mp_next(&key)) {
const char *field;
if (is_multikey) {
- field = tuple_field_raw_by_part_multikey(format,
- tuple_raw, field_map, part,
- (int)tuple_hint);
+ field = tuple_field_raw_by_part(format, tuple_raw,
+ field_map, part,
+ (int)tuple_hint);
} else if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part);
+ field_map, part, -1);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
@@ -1553,6 +1553,7 @@ template <enum field_type type, bool is_nullable>
static hint_t
key_hint(const char *key, uint32_t part_count, struct key_def *key_def)
{
+ assert(!key_def_is_multikey(key_def));
if (part_count == 0)
return HINT_NONE;
return field_hint<type, is_nullable>(key, key_def->parts->coll);
@@ -1562,7 +1563,8 @@ template <enum field_type type, bool is_nullable>
static hint_t
tuple_hint(struct tuple *tuple, struct key_def *key_def)
{
- const char *field = tuple_field_by_part(tuple, key_def->parts);
+ assert(!key_def_is_multikey(key_def));
+ const char *field = tuple_field_by_part(tuple, key_def->parts, -1);
if (is_nullable && field == NULL)
return hint_nil();
return field_hint<type, is_nullable>(field, key_def->parts->coll);
@@ -1584,6 +1586,7 @@ key_hint_multikey(const char *key, uint32_t part_count, struct key_def *key_def)
* do nothing on key hint calculation an it is valid
* because it is never used(unlike tuple hint).
*/
+ assert(key_def_is_multikey(key_def));
return HINT_NONE;
}
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 7fded038..7868378d 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -132,7 +132,7 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
key_def->parts[i].fieldno);
} else {
field = tuple_field_raw_by_part(format, data, field_map,
- &key_def->parts[i]);
+ &key_def->parts[i], -1);
}
if (has_optional_parts && field == NULL) {
bsize += mp_sizeof_nil();
@@ -179,7 +179,7 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
key_def->parts[i].fieldno);
} else {
field = tuple_field_raw_by_part(format, data, field_map,
- &key_def->parts[i]);
+ &key_def->parts[i], -1);
}
if (has_optional_parts && field == NULL) {
key_buf = mp_encode_nil(key_buf);
@@ -413,13 +413,14 @@ key_def_set_extract_func(struct key_def *key_def)
bool
tuple_key_contains_null(struct tuple *tuple, struct key_def *def)
{
+ assert(!key_def_is_multikey(def));
struct tuple_format *format = tuple_format(tuple);
const char *data = tuple_data(tuple);
const uint32_t *field_map = tuple_field_map(tuple);
for (struct key_part *part = def->parts, *end = part + def->part_count;
part < end; ++part) {
- const char *field =
- tuple_field_raw_by_part(format, data, field_map, part);
+ const char *field = tuple_field_raw_by_part(format, data,
+ field_map, part, -1);
if (field == NULL || mp_typeof(*field) == MP_NIL)
return true;
}
@@ -429,9 +430,10 @@ tuple_key_contains_null(struct tuple *tuple, struct key_def *def)
int
tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple)
{
+ assert(!key_def_is_multikey(key_def));
for (uint32_t idx = 0; idx < key_def->part_count; idx++) {
struct key_part *part = &key_def->parts[idx];
- const char *field = tuple_field_by_part(tuple, part);
+ const char *field = tuple_field_by_part(tuple, part, -1);
if (field == NULL) {
if (key_part_is_nullable(part))
continue;
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index 85095eb8..63de2bae 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -155,11 +155,12 @@ struct TupleHash
{
static uint32_t hash(struct tuple *tuple, struct key_def *key_def)
{
+ assert(!key_def_is_multikey(key_def));
uint32_t h = HASH_SEED;
uint32_t carry = 0;
uint32_t total_size = 0;
- const char *field =
- tuple_field_by_part(tuple, key_def->parts);
+ const char *field = tuple_field_by_part(tuple,
+ key_def->parts, -1);
TupleFieldHash<TYPE, MORE_TYPES...>::
hash(&field, &h, &carry, &total_size);
return PMurHash32_Result(h, carry, total_size);
@@ -170,8 +171,9 @@ template <>
struct TupleHash<FIELD_TYPE_UNSIGNED> {
static uint32_t hash(struct tuple *tuple, struct key_def *key_def)
{
- const char *field =
- tuple_field_by_part(tuple, key_def->parts);
+ assert(!key_def_is_multikey(key_def));
+ const char *field = tuple_field_by_part(tuple,
+ key_def->parts, -1);
uint64_t val = mp_decode_uint(&field);
if (likely(val <= UINT32_MAX))
return val;
@@ -348,7 +350,7 @@ uint32_t
tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, struct tuple *tuple,
struct key_part *part)
{
- const char *field = tuple_field_by_part(tuple, part);
+ const char *field = tuple_field_by_part(tuple, part, -1);
if (field == NULL)
return tuple_hash_null(ph1, pcarry);
return tuple_hash_field(ph1, pcarry, &field, part->coll);
@@ -360,6 +362,7 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def)
{
assert(has_json_paths == key_def->has_json_paths);
assert(has_optional_parts == key_def->has_optional_parts);
+ assert(!key_def_is_multikey(key_def));
uint32_t h = HASH_SEED;
uint32_t carry = 0;
uint32_t total_size = 0;
@@ -370,7 +373,7 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def)
const char *field;
if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw, field_map,
- key_def->parts);
+ key_def->parts, -1);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
prev_fieldno);
@@ -391,7 +394,7 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def)
struct key_part *part = &key_def->parts[part_id];
if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part);
+ field_map, part, -1);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 3/7] Make tuple_extract_key support multikey indexes
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
2019-05-08 17:22 ` [PATCH 1/7] Make tuple comparison hints mandatory Vladimir Davydov
2019-05-08 17:22 ` [PATCH 2/7] Get rid of tuple_field_by_part_multikey Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-08 17:22 ` [PATCH 4/7] Make tuple_bloom " Vladimir Davydov
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
Add multikey_idx argument to tuple_extract_key and forward it to
tuple_field_by_part in the method implementation. For unikey indexes
pass -1. We need this to support multikey indexes in Vinyl.
We could of course introduce a separate set of methods for multikey
indexes (something like tuple_extract_key_multikey), but that would
look cumbersome and hardly result in any performance benefits, because
passing -1 to a relatively cold function, such as key extractor, isn't
a big deal. Besides, passing multikey_idx unconditionally is consistent
with tuple_compare.
---
src/box/index.cc | 2 +-
src/box/key_def.h | 18 +++++++++++----
src/box/lua/key_def.c | 2 +-
src/box/memtx_space.c | 2 +-
src/box/request.c | 5 ++--
src/box/space.c | 2 +-
src/box/sql.c | 10 ++++----
| 54 ++++++++++++++++++++++++++++++--------------
src/box/vinyl.c | 12 +++++-----
src/box/vy_run.c | 6 ++---
src/box/vy_stmt.c | 17 +++++++-------
src/box/vy_stmt.h | 6 ++---
12 files changed, 83 insertions(+), 53 deletions(-)
diff --git a/src/box/index.cc b/src/box/index.cc
index 2817d076..39b0ee46 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -156,7 +156,7 @@ box_tuple_extract_key(box_tuple_t *tuple, uint32_t space_id, uint32_t index_id,
struct index *index = index_find(space, index_id);
if (index == NULL)
return NULL;
- return tuple_extract_key(tuple, index->def->key_def, key_size);
+ return tuple_extract_key(tuple, index->def->key_def, -1, key_size);
}
static inline int
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 5f65f501..8b94b3b6 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -145,11 +145,13 @@ typedef int (*tuple_compare_t)(struct tuple *tuple_a,
/** @copydoc tuple_extract_key() */
typedef char *(*tuple_extract_key_t)(struct tuple *tuple,
struct key_def *key_def,
+ int multikey_idx,
uint32_t *key_size);
/** @copydoc tuple_extract_key_raw() */
typedef char *(*tuple_extract_key_raw_t)(const char *data,
const char *data_end,
struct key_def *key_def,
+ int multikey_idx,
uint32_t *key_size);
/** @copydoc tuple_hash() */
typedef uint32_t (*tuple_hash_t)(struct tuple *tuple,
@@ -531,10 +533,12 @@ key_part_cmp(const struct key_part *parts1, uint32_t part_count1,
* Check if a key of @a tuple contains NULL.
* @param tuple Tuple to check.
* @param def Key def to check by.
+ * @param multikey_idx Multikey index hint.
* @retval Does the key contain NULL or not?
*/
bool
-tuple_key_contains_null(struct tuple *tuple, struct key_def *def);
+tuple_key_contains_null(struct tuple *tuple, struct key_def *def,
+ int multikey_idx);
/**
* Check that tuple fields match with given key definition
@@ -554,6 +558,7 @@ tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple);
* has O(n) complexity, where n is the number of key parts.
* @param tuple - tuple from which need to extract key
* @param key_def - definition of key that need to extract
+ * @param multikey_idx - multikey index hint
* @param key_size - here will be size of extracted key
*
* @retval not NULL Success
@@ -561,9 +566,10 @@ tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple);
*/
static inline char *
tuple_extract_key(struct tuple *tuple, struct key_def *key_def,
- uint32_t *key_size)
+ int multikey_idx, uint32_t *key_size)
{
- return key_def->tuple_extract_key(tuple, key_def, key_size);
+ return key_def->tuple_extract_key(tuple, key_def, multikey_idx,
+ key_size);
}
/**
@@ -574,6 +580,7 @@ tuple_extract_key(struct tuple *tuple, struct key_def *key_def,
* @param data - msgpuck data from which need to extract key
* @param data_end - pointer at the end of data
* @param key_def - definition of key that need to extract
+ * @param multikey_idx - multikey index hint
* @param key_size - here will be size of extracted key
*
* @retval not NULL Success
@@ -581,10 +588,11 @@ tuple_extract_key(struct tuple *tuple, struct key_def *key_def,
*/
static inline char *
tuple_extract_key_raw(const char *data, const char *data_end,
- struct key_def *key_def, uint32_t *key_size)
+ struct key_def *key_def, int multikey_idx,
+ uint32_t *key_size)
{
return key_def->tuple_extract_key_raw(data, data_end, key_def,
- key_size);
+ multikey_idx, key_size);
}
/**
diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index 72bb05c4..28d40478 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -265,7 +265,7 @@ lbox_key_def_extract_key(struct lua_State *L)
struct region *region = &fiber()->gc;
size_t region_svp = region_used(region);
uint32_t key_size;
- char *key = tuple_extract_key(tuple, key_def, &key_size);
+ char *key = tuple_extract_key(tuple, key_def, -1, &key_size);
tuple_unref(tuple);
if (key == NULL)
return luaT_error(L);
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 4d5e7991..265bf923 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -445,7 +445,7 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
/* Extract the primary key from tuple. */
const char *key = tuple_extract_key_raw(request->tuple,
request->tuple_end,
- index->def->key_def, NULL);
+ index->def->key_def, -1, NULL);
if (key == NULL)
return -1;
/* Cut array header */
diff --git a/src/box/request.c b/src/box/request.c
index 9c684af7..8b84260a 100644
--- a/src/box/request.c
+++ b/src/box/request.c
@@ -92,7 +92,7 @@ request_create_from_tuple(struct request *request, struct space *space,
uint32_t size, key_size;
const char *data = tuple_data_range(old_tuple, &size);
request->key = tuple_extract_key_raw(data, data + size,
- space->index[0]->def->key_def, &key_size);
+ space->index[0]->def->key_def, -1, &key_size);
if (request->key == NULL)
return -1;
request->key_end = request->key + key_size;
@@ -125,7 +125,8 @@ request_rebind_to_primary_key(struct request *request, struct space *space,
struct index *pk = space_index(space, 0);
assert(pk != NULL);
uint32_t key_len;
- char *key = tuple_extract_key(found_tuple, pk->def->key_def, &key_len);
+ char *key = tuple_extract_key(found_tuple, pk->def->key_def, -1,
+ &key_len);
assert(key != NULL);
request->key = key;
request->key_end = key + key_len;
diff --git a/src/box/space.c b/src/box/space.c
index edfc5d60..9a266604 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -320,7 +320,7 @@ space_before_replace(struct space *space, struct txn *txn,
break;
index = pk;
key = tuple_extract_key_raw(request->tuple, request->tuple_end,
- index->def->key_def, NULL);
+ index->def->key_def, -1, NULL);
if (key == NULL)
return -1;
part_count = mp_decode_array(&key);
diff --git a/src/box/sql.c b/src/box/sql.c
index 7a20d86e..c3f404ee 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -480,7 +480,7 @@ int tarantoolsqlEphemeralDelete(BtCursor *pCur)
char *key;
uint32_t key_size;
key = tuple_extract_key(pCur->last_tuple,
- pCur->iter->index->def->key_def,
+ pCur->iter->index->def->key_def, -1,
&key_size);
if (key == NULL)
return SQL_TARANTOOL_DELETE_FAIL;
@@ -506,7 +506,7 @@ int tarantoolsqlDelete(BtCursor *pCur, u8 flags)
int rc;
key = tuple_extract_key(pCur->last_tuple,
- pCur->iter->index->def->key_def,
+ pCur->iter->index->def->key_def, -1,
&key_size);
if (key == NULL)
return SQL_TARANTOOL_DELETE_FAIL;
@@ -561,7 +561,7 @@ int tarantoolsqlEphemeralClearTable(BtCursor *pCur)
uint32_t key_size;
while (iterator_next(it, &tuple) == 0 && tuple != NULL) {
- key = tuple_extract_key(tuple, it->index->def->key_def,
+ key = tuple_extract_key(tuple, it->index->def->key_def, -1,
&key_size);
if (space_ephemeral_delete(pCur->space, key) != 0) {
iterator_delete(it);
@@ -594,7 +594,7 @@ int tarantoolsqlClearTable(struct space *space, uint32_t *tuple_count)
if (iter == NULL)
return SQL_TARANTOOL_ITERATOR_FAIL;
while (iterator_next(iter, &tuple) == 0 && tuple != NULL) {
- request.key = tuple_extract_key(tuple, pk->def->key_def,
+ request.key = tuple_extract_key(tuple, pk->def->key_def, -1,
&key_size);
request.key_end = request.key + key_size;
rc = box_process_rw(&request, space, &unused);
@@ -801,7 +801,7 @@ out:
#ifndef NDEBUG
/* Sanity check. */
original_size = region_used(&fiber()->gc);
- key = tuple_extract_key(tuple, key_def, &key_size);
+ key = tuple_extract_key(tuple, key_def, -1, &key_size);
if (key != NULL) {
int new_rc = sqlVdbeRecordCompareMsgpack(key, unpacked);
region_truncate(&fiber()->gc, original_size);
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 7868378d..15e43aae 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -37,8 +37,10 @@ key_def_contains_sequential_parts(const struct key_def *def)
template <bool has_optional_parts>
static char *
tuple_extract_key_sequential_raw(const char *data, const char *data_end,
- struct key_def *key_def, uint32_t *key_size)
+ struct key_def *key_def, int multikey_idx,
+ uint32_t *key_size)
{
+ (void)multikey_idx;
assert(!has_optional_parts || key_def->is_nullable);
assert(key_def_is_sequential(key_def));
assert(has_optional_parts == key_def->has_optional_parts);
@@ -87,7 +89,7 @@ tuple_extract_key_sequential_raw(const char *data, const char *data_end,
template <bool has_optional_parts>
static inline char *
tuple_extract_key_sequential(struct tuple *tuple, struct key_def *key_def,
- uint32_t *key_size)
+ int multikey_idx, uint32_t *key_size)
{
assert(key_def_is_sequential(key_def));
assert(!has_optional_parts || key_def->is_nullable);
@@ -97,6 +99,7 @@ tuple_extract_key_sequential(struct tuple *tuple, struct key_def *key_def,
return tuple_extract_key_sequential_raw<has_optional_parts>(data,
data_end,
key_def,
+ multikey_idx,
key_size);
}
@@ -105,17 +108,18 @@ tuple_extract_key_sequential(struct tuple *tuple, struct key_def *key_def,
* @copydoc tuple_extract_key()
*/
template <bool contains_sequential_parts, bool has_optional_parts,
- bool has_json_paths>
+ bool has_json_paths, bool is_multikey>
static char *
tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
- uint32_t *key_size)
+ int multikey_idx, uint32_t *key_size)
{
assert(has_json_paths == key_def->has_json_paths);
assert(!has_optional_parts || key_def->is_nullable);
assert(has_optional_parts == key_def->has_optional_parts);
assert(contains_sequential_parts ==
key_def_contains_sequential_parts(key_def));
- assert(!key_def_is_multikey(key_def));
+ assert(is_multikey == key_def_is_multikey(key_def));
+ assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
assert(mp_sizeof_nil() == 1);
const char *data = tuple_data(tuple);
uint32_t part_count = key_def->part_count;
@@ -130,9 +134,13 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
if (!has_json_paths) {
field = tuple_field_raw(format, data, field_map,
key_def->parts[i].fieldno);
- } else {
+ } else if (!is_multikey) {
field = tuple_field_raw_by_part(format, data, field_map,
&key_def->parts[i], -1);
+ } else {
+ field = tuple_field_raw_by_part(format, data, field_map,
+ &key_def->parts[i],
+ multikey_idx);
}
if (has_optional_parts && field == NULL) {
bsize += mp_sizeof_nil();
@@ -177,9 +185,13 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
if (!has_json_paths) {
field = tuple_field_raw(format, data, field_map,
key_def->parts[i].fieldno);
- } else {
+ } else if (!is_multikey) {
field = tuple_field_raw_by_part(format, data, field_map,
&key_def->parts[i], -1);
+ } else {
+ field = tuple_field_raw_by_part(format, data, field_map,
+ &key_def->parts[i],
+ multikey_idx);
}
if (has_optional_parts && field == NULL) {
key_buf = mp_encode_nil(key_buf);
@@ -230,12 +242,13 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
template <bool has_optional_parts, bool has_json_paths>
static char *
tuple_extract_key_slowpath_raw(const char *data, const char *data_end,
- struct key_def *key_def, uint32_t *key_size)
+ struct key_def *key_def, int multikey_idx,
+ uint32_t *key_size)
{
assert(has_json_paths == key_def->has_json_paths);
assert(!has_optional_parts || key_def->is_nullable);
assert(has_optional_parts == key_def->has_optional_parts);
- assert(!key_def_is_multikey(key_def));
+ assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
assert(mp_sizeof_nil() == 1);
/* allocate buffer with maximal possible size */
char *key = (char *) region_alloc(&fiber()->gc, data_end - data);
@@ -311,8 +324,8 @@ tuple_extract_key_slowpath_raw(const char *data, const char *data_end,
const char *src = field;
const char *src_end = field_end;
if (has_json_paths && part->path != NULL) {
- if (tuple_go_to_path(&src, part->path,
- part->path_len, -1) != 0) {
+ if (tuple_go_to_path(&src, part->path, part->path_len,
+ multikey_idx) != 0) {
/*
* The path must be correct as
* it has already been validated
@@ -361,7 +374,7 @@ key_def_set_extract_func_plain(struct key_def *def)
} else {
def->tuple_extract_key = tuple_extract_key_slowpath
<contains_sequential_parts,
- has_optional_parts, false>;
+ has_optional_parts, false, false>;
def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw
<has_optional_parts, false>;
}
@@ -372,9 +385,15 @@ static void
key_def_set_extract_func_json(struct key_def *def)
{
assert(def->has_json_paths);
- def->tuple_extract_key = tuple_extract_key_slowpath
+ if (key_def_is_multikey(def)) {
+ def->tuple_extract_key = tuple_extract_key_slowpath
<contains_sequential_parts,
- has_optional_parts, true>;
+ has_optional_parts, true, true>;
+ } else {
+ def->tuple_extract_key = tuple_extract_key_slowpath
+ <contains_sequential_parts,
+ has_optional_parts, true, false>;
+ }
def->tuple_extract_key_raw = tuple_extract_key_slowpath_raw
<has_optional_parts, true>;
}
@@ -411,16 +430,17 @@ key_def_set_extract_func(struct key_def *key_def)
}
bool
-tuple_key_contains_null(struct tuple *tuple, struct key_def *def)
+tuple_key_contains_null(struct tuple *tuple, struct key_def *def,
+ int multikey_idx)
{
- assert(!key_def_is_multikey(def));
struct tuple_format *format = tuple_format(tuple);
const char *data = tuple_data(tuple);
const uint32_t *field_map = tuple_field_map(tuple);
for (struct key_part *part = def->parts, *end = part + def->part_count;
part < end; ++part) {
const char *field = tuple_field_raw_by_part(format, data,
- field_map, part, -1);
+ field_map, part,
+ multikey_idx);
if (field == NULL || mp_typeof(*field) == MP_NIL)
return true;
}
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 6ffbecd9..eddbdbed 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1344,7 +1344,7 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
struct vy_entry key;
if (vy_stmt_is_key(entry.stmt)) {
key.stmt = vy_stmt_extract_key(entry.stmt, lsm->pk_in_cmp_def,
- lsm->env->key_format);
+ lsm->env->key_format, -1);
if (key.stmt == NULL)
return -1;
} else {
@@ -1601,10 +1601,10 @@ vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
if (!lsm->check_is_unique)
return 0;
if (lsm->key_def->is_nullable &&
- tuple_key_contains_null(stmt, lsm->key_def))
+ tuple_key_contains_null(stmt, lsm->key_def, -1))
return 0;
struct tuple *key = vy_stmt_extract_key(stmt, lsm->key_def,
- lsm->env->key_format);
+ lsm->env->key_format, -1);
if (key == NULL)
return -1;
struct tuple *found;
@@ -2149,7 +2149,7 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
*/
/* Find the old tuple using the primary key. */
struct tuple *key = vy_stmt_extract_key_raw(tuple, tuple_end,
- pk->key_def, pk->env->key_format);
+ pk->key_def, pk->env->key_format, -1);
if (key == NULL)
return -1;
int rc = vy_get(pk, tx, vy_tx_read_view(tx), key, &stmt->old_tuple);
@@ -4064,7 +4064,7 @@ vy_build_on_replace(struct trigger *trigger, void *event)
/* Forward the statement to the new LSM tree. */
if (stmt->old_tuple != NULL) {
struct tuple *delete = vy_stmt_extract_key(stmt->old_tuple,
- lsm->cmp_def, lsm->env->key_format);
+ lsm->cmp_def, lsm->env->key_format, -1);
if (delete == NULL)
goto err;
vy_stmt_set_type(delete, IPROTO_DELETE);
@@ -4219,7 +4219,7 @@ vy_build_recover_stmt(struct vy_lsm *lsm, struct vy_lsm *pk,
struct tuple *old_tuple = old.stmt;
if (old_tuple != NULL) {
delete = vy_stmt_extract_key(old_tuple, lsm->cmp_def,
- lsm->env->key_format);
+ lsm->env->key_format, -1);
if (delete == NULL)
return -1;
vy_stmt_set_type(delete, IPROTO_DELETE);
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index b15a96e9..409c3d96 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -2167,7 +2167,7 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
const char *key = vy_stmt_is_key(first_entry.stmt) ?
tuple_data(first_entry.stmt) :
tuple_extract_key(first_entry.stmt,
- writer->cmp_def, NULL);
+ writer->cmp_def, -1, NULL);
if (key == NULL)
return -1;
if (run->info.page_count == 0) {
@@ -2321,7 +2321,7 @@ vy_run_writer_commit(struct vy_run_writer *writer)
const char *key = vy_stmt_is_key(writer->last.stmt) ?
tuple_data(writer->last.stmt) :
tuple_extract_key(writer->last.stmt,
- writer->cmp_def, NULL);
+ writer->cmp_def, -1, NULL);
if (key == NULL)
goto out;
@@ -2431,7 +2431,7 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
goto close_err;
}
key = vy_stmt_is_key(tuple) ? tuple_data(tuple) :
- tuple_extract_key(tuple, cmp_def, NULL);
+ tuple_extract_key(tuple, cmp_def, -1, NULL);
if (prev_tuple != NULL)
tuple_unref(prev_tuple);
prev_tuple = tuple;
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index c8088d34..847b6196 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -489,11 +489,12 @@ out:
struct tuple *
vy_stmt_extract_key(struct tuple *stmt, struct key_def *key_def,
- struct tuple_format *format)
+ struct tuple_format *format, int multikey_idx)
{
struct region *region = &fiber()->gc;
size_t region_svp = region_used(region);
- const char *key_raw = tuple_extract_key(stmt, key_def, NULL);
+ const char *key_raw = tuple_extract_key(stmt, key_def,
+ multikey_idx, NULL);
if (key_raw == NULL)
return NULL;
uint32_t part_count = mp_decode_array(&key_raw);
@@ -506,13 +507,13 @@ vy_stmt_extract_key(struct tuple *stmt, struct key_def *key_def,
struct tuple *
vy_stmt_extract_key_raw(const char *data, const char *data_end,
- struct key_def *key_def,
- struct tuple_format *format)
+ struct key_def *key_def, struct tuple_format *format,
+ int multikey_idx)
{
struct region *region = &fiber()->gc;
size_t region_svp = region_used(region);
- const char *key_raw = tuple_extract_key_raw(data, data_end,
- key_def, NULL);
+ const char *key_raw = tuple_extract_key_raw(data, data_end, key_def,
+ multikey_idx, NULL);
if (key_raw == NULL)
return NULL;
uint32_t part_count = mp_decode_array(&key_raw);
@@ -622,7 +623,7 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
case IPROTO_DELETE:
extracted = vy_stmt_is_key(value) ?
tuple_data_range(value, &size) :
- tuple_extract_key(value, key_def, &size);
+ tuple_extract_key(value, key_def, -1, &size);
if (extracted == NULL)
return -1;
request.key = extracted;
@@ -666,7 +667,7 @@ vy_stmt_encode_secondary(struct tuple *value, struct key_def *cmp_def,
uint32_t size;
const char *extracted = vy_stmt_is_key(value) ?
tuple_data_range(value, &size) :
- tuple_extract_key(value, cmp_def, &size);
+ tuple_extract_key(value, cmp_def, -1, &size);
if (extracted == NULL)
return -1;
if (type == IPROTO_REPLACE || type == IPROTO_INSERT) {
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 49fa1602..929e537a 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -606,7 +606,7 @@ vy_key_from_msgpack(struct tuple_format *format, const char *key)
*/
struct tuple *
vy_stmt_extract_key(struct tuple *stmt, struct key_def *key_def,
- struct tuple_format *format);
+ struct tuple_format *format, int multikey_idx);
/**
* Extract the key from msgpack by the given key definition
@@ -615,8 +615,8 @@ vy_stmt_extract_key(struct tuple *stmt, struct key_def *key_def,
*/
struct tuple *
vy_stmt_extract_key_raw(const char *data, const char *data_end,
- struct key_def *key_def,
- struct tuple_format *format);
+ struct key_def *key_def, struct tuple_format *format,
+ int multikey_idx);
/**
* Add a statement hash to a bloom filter builder.
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 4/7] Make tuple_bloom support multikey indexes
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (2 preceding siblings ...)
2019-05-08 17:22 ` [PATCH 3/7] Make tuple_extract_key support multikey indexes Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-08 17:22 ` [PATCH 5/7] vinyl: use field_map_builder for constructing stmt field map Vladimir Davydov
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
Just like in case of tuple_extract_key, simply pass multikey_idx to
tuple_bloom_builder_add and tuple_bloom_maybe_has. For now, we always
pass -1, but the following patches will pass offset in multikey array
if the key definition is multikey.
---
src/box/key_def.h | 3 ++-
src/box/tuple_bloom.c | 16 +++++++++++-----
src/box/tuple_bloom.h | 9 ++++++---
src/box/tuple_hash.cc | 4 ++--
src/box/vy_stmt.c | 4 ++--
5 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 8b94b3b6..f4a1a8fd 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -675,13 +675,14 @@ tuple_hash_field(uint32_t *ph1, uint32_t *pcarry, const char **field,
* @param pcarry - pointer to carry
* @param tuple - tuple to hash
* @param part - key part
+ * @param multikey_idx - multikey index hint
* @return size of processed data
*
* This function updates @ph1 and @pcarry.
*/
uint32_t
tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, struct tuple *tuple,
- struct key_part *part);
+ struct key_part *part, int multikey_idx);
/**
* Calculates a common hash value for a tuple
diff --git a/src/box/tuple_bloom.c b/src/box/tuple_bloom.c
index 935df798..99f19a2e 100644
--- a/src/box/tuple_bloom.c
+++ b/src/box/tuple_bloom.c
@@ -105,9 +105,11 @@ tuple_hash_array_add(struct tuple_hash_array *hash_arr, uint32_t hash)
int
tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
- struct tuple *tuple, struct key_def *key_def)
+ struct tuple *tuple, struct key_def *key_def,
+ int multikey_idx)
{
assert(builder->part_count == key_def->part_count);
+ assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
uint32_t h = HASH_SEED;
uint32_t carry = 0;
@@ -115,7 +117,8 @@ tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
for (uint32_t i = 0; i < key_def->part_count; i++) {
total_size += tuple_hash_key_part(&h, &carry, tuple,
- &key_def->parts[i]);
+ &key_def->parts[i],
+ multikey_idx);
uint32_t hash = PMurHash32_Result(h, carry, total_size);
if (tuple_hash_array_add(&builder->parts[i], hash) != 0)
return -1;
@@ -198,9 +201,11 @@ tuple_bloom_delete(struct tuple_bloom *bloom)
}
bool
-tuple_bloom_maybe_has(const struct tuple_bloom *bloom,
- struct tuple *tuple, struct key_def *key_def)
+tuple_bloom_maybe_has(const struct tuple_bloom *bloom, struct tuple *tuple,
+ struct key_def *key_def, int multikey_idx)
{
+ assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
+
if (bloom->is_legacy) {
return bloom_maybe_has(&bloom->parts[0],
tuple_hash(tuple, key_def));
@@ -214,7 +219,8 @@ tuple_bloom_maybe_has(const struct tuple_bloom *bloom,
for (uint32_t i = 0; i < key_def->part_count; i++) {
total_size += tuple_hash_key_part(&h, &carry, tuple,
- &key_def->parts[i]);
+ &key_def->parts[i],
+ multikey_idx);
uint32_t hash = PMurHash32_Result(h, carry, total_size);
if (!bloom_maybe_has(&bloom->parts[i], hash))
return false;
diff --git a/src/box/tuple_bloom.h b/src/box/tuple_bloom.h
index 5a630a49..1b7e4ac4 100644
--- a/src/box/tuple_bloom.h
+++ b/src/box/tuple_bloom.h
@@ -129,11 +129,13 @@ tuple_bloom_builder_delete(struct tuple_bloom_builder *builder);
* @param builder - bloom filter builder
* @param tuple - tuple to add
* @param key_def - key definition
+ * @param multikey_idx - multikey index hint
* @return 0 on success, -1 on OOM
*/
int
tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
- struct tuple *tuple, struct key_def *key_def);
+ struct tuple *tuple, struct key_def *key_def,
+ int multikey_idx);
/**
* Add a key hash to a tuple bloom filter builder.
@@ -169,12 +171,13 @@ tuple_bloom_delete(struct tuple_bloom *bloom);
* @param bloom - bloom filter
* @param tuple - tuple to check
* @param key_def - key definition
+ * @param multikey_idx - multikey index hint
* @return true if the tuple may have been stored in the bloom,
* false if the tuple is definitely not in the bloom
*/
bool
-tuple_bloom_maybe_has(const struct tuple_bloom *bloom,
- struct tuple *tuple, struct key_def *key_def);
+tuple_bloom_maybe_has(const struct tuple_bloom *bloom, struct tuple *tuple,
+ struct key_def *key_def, int multikey_idx);
/**
* Check if a tuple matching a key was stored in a tuple bloom filter.
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index 63de2bae..f90e1671 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -348,9 +348,9 @@ tuple_hash_null(uint32_t *ph1, uint32_t *pcarry)
uint32_t
tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, struct tuple *tuple,
- struct key_part *part)
+ struct key_part *part, int multikey_idx)
{
- const char *field = tuple_field_by_part(tuple, part, -1);
+ const char *field = tuple_field_by_part(tuple, part, multikey_idx);
if (field == NULL)
return tuple_hash_null(ph1, pcarry);
return tuple_hash_field(ph1, pcarry, &field, part->coll);
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 847b6196..ddad26f5 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -534,7 +534,7 @@ vy_stmt_bloom_builder_add(struct tuple_bloom_builder *builder,
return tuple_bloom_builder_add_key(builder, data,
part_count, key_def);
} else {
- return tuple_bloom_builder_add(builder, stmt, key_def);
+ return tuple_bloom_builder_add(builder, stmt, key_def, -1);
}
}
@@ -548,7 +548,7 @@ vy_stmt_bloom_maybe_has(const struct tuple_bloom *bloom,
return tuple_bloom_maybe_has_key(bloom, data,
part_count, key_def);
} else {
- return tuple_bloom_maybe_has(bloom, stmt, key_def);
+ return tuple_bloom_maybe_has(bloom, stmt, key_def, -1);
}
}
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 5/7] vinyl: use field_map_builder for constructing stmt field map
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (3 preceding siblings ...)
2019-05-08 17:22 ` [PATCH 4/7] Make tuple_bloom " Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-08 17:22 ` [PATCH 6/7] vinyl: use multikey hints while writing runs Vladimir Davydov
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
Currently, we construct a field map for a vinyl surrogate DELETE
statement by hand, which works fine as long as field maps don't have
extents. Once multikey indexes are introduced, there will be extents
hence we must switch to field_map_builder.
---
src/box/vy_stmt.c | 60 ++++++++++++++++++++++++++++---------------------------
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index ddad26f5..170c258a 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -146,18 +146,19 @@ vy_stmt_format_new(struct vy_stmt_env *env, struct key_def *const *keys,
* Allocate a vinyl statement object on base of the struct tuple
* with malloc() and the reference counter equal to 1.
* @param format Format of an index.
- * @param size Size of the variable part of the statement. It
+ * @param data_offset Offset of MessagePack data within the tuple.
+ * @param bsize Size of the variable part of the statement. It
* includes size of MessagePack tuple data and, for
* upserts, MessagePack array of operations.
* @retval not NULL Success.
* @retval NULL Memory error.
*/
static struct tuple *
-vy_stmt_alloc(struct tuple_format *format, uint32_t bsize)
+vy_stmt_alloc(struct tuple_format *format, uint32_t data_offset, uint32_t bsize)
{
+ assert(data_offset >= sizeof(struct vy_stmt) + format->field_map_size);
struct vy_stmt_env *env = format->engine;
- uint32_t total_size = sizeof(struct vy_stmt) + format->field_map_size +
- bsize;
+ uint32_t total_size = data_offset + bsize;
if (unlikely(total_size > env->max_tuple_size)) {
diag_set(ClientError, ER_VINYL_MAX_TUPLE_SIZE,
(unsigned) total_size);
@@ -169,14 +170,14 @@ vy_stmt_alloc(struct tuple_format *format, uint32_t bsize)
diag_set(OutOfMemory, total_size, "malloc", "struct vy_stmt");
return NULL;
}
- say_debug("vy_stmt_alloc(format = %d %u, bsize = %zu) = %p",
- format->id, format->field_map_size, bsize, tuple);
+ say_debug("vy_stmt_alloc(format = %d data_offset = %u, bsize = %u) = %p",
+ format->id, data_offset, bsize, tuple);
tuple->refs = 1;
tuple->format_id = tuple_format_id(format);
if (cord_is_main())
tuple_format_ref(format);
tuple->bsize = bsize;
- tuple->data_offset = sizeof(struct vy_stmt) + format->field_map_size;
+ tuple->data_offset = data_offset;
vy_stmt_set_lsn(tuple, 0);
vy_stmt_set_type(tuple, 0);
vy_stmt_set_flags(tuple, 0);
@@ -191,7 +192,8 @@ vy_stmt_dup(struct tuple *stmt)
* tuple field map. This map can be simple memcopied from
* the original tuple.
*/
- struct tuple *res = vy_stmt_alloc(tuple_format(stmt), stmt->bsize);
+ struct tuple *res = vy_stmt_alloc(tuple_format(stmt),
+ stmt->data_offset, stmt->bsize);
if (res == NULL)
return NULL;
assert(tuple_size(res) == tuple_size(stmt));
@@ -253,7 +255,7 @@ vy_key_new(struct tuple_format *format, const char *key, uint32_t part_count)
/* Allocate stmt */
uint32_t key_size = key_end - key;
uint32_t bsize = mp_sizeof_array(part_count) + key_size;
- struct tuple *stmt = vy_stmt_alloc(format, bsize);
+ struct tuple *stmt = vy_stmt_alloc(format, sizeof(struct vy_stmt), bsize);
if (stmt == NULL)
return NULL;
/* Copy MsgPack data */
@@ -318,14 +320,14 @@ vy_stmt_new_with_ops(struct tuple_format *format, const char *tuple_begin,
if (tuple_field_map_create(format, tuple_begin, false, &builder) != 0)
goto end;
uint32_t field_map_size = field_map_build_size(&builder);
- assert(field_map_size == format->field_map_size);
/*
* Allocate stmt. Offsets: one per key part + offset of the
* statement end.
*/
size_t mpsize = (tuple_end - tuple_begin);
size_t bsize = mpsize + ops_size;
- stmt = vy_stmt_alloc(format, bsize);
+ stmt = vy_stmt_alloc(format, sizeof(struct vy_stmt) +
+ field_map_size, bsize);
if (stmt == NULL)
goto end;
/* Copy MsgPack data */
@@ -389,13 +391,13 @@ vy_stmt_replace_from_upsert(struct tuple *upsert)
/* Copy statement data excluding UPSERT operations */
struct tuple_format *format = tuple_format(upsert);
- struct tuple *replace = vy_stmt_alloc(format, bsize);
+ struct tuple *replace = vy_stmt_alloc(format, upsert->data_offset, bsize);
if (replace == NULL)
return NULL;
/* Copy both data and field_map. */
char *dst = (char *)replace + sizeof(struct vy_stmt);
char *src = (char *)upsert + sizeof(struct vy_stmt);
- memcpy(dst, src, format->field_map_size + bsize);
+ memcpy(dst, src, upsert->data_offset + bsize - sizeof(struct vy_stmt));
vy_stmt_set_type(replace, IPROTO_REPLACE);
vy_stmt_set_lsn(replace, vy_stmt_lsn(upsert));
return replace;
@@ -407,17 +409,18 @@ vy_stmt_new_surrogate_delete_raw(struct tuple_format *format,
{
struct tuple *stmt = NULL;
uint32_t src_size = src_data_end - src_data;
- uint32_t total_size = src_size + format->field_map_size;
/* Surrogate tuple uses less memory than the original tuple */
struct region *region = &fiber()->gc;
size_t region_svp = region_used(region);
- char *data = region_alloc(region, total_size);
+ char *data = region_alloc(region, src_size);
if (data == NULL) {
diag_set(OutOfMemory, src_size, "region", "tuple");
return NULL;
}
- char *field_map_begin = data + src_size;
- uint32_t *field_map = (uint32_t *) (data + total_size);
+ struct field_map_builder builder;
+ if (field_map_builder_create(&builder, format->field_map_size,
+ region) != 0)
+ goto out;
/*
* Perform simultaneous parsing of the tuple and
* format::fields tree traversal to copy indexed field
@@ -429,14 +432,7 @@ vy_stmt_new_surrogate_delete_raw(struct tuple_format *format,
TUPLE_FORMAT_ITERATOR_KEY_PARTS_ONLY, &field_count,
region) != 0)
goto out;
- /*
- * Nullify field map to be able to detect by 0, which key
- * fields are absent in tuple_field().
- */
- memset((char *)field_map - format->field_map_size, 0,
- format->field_map_size);
char *pos = mp_encode_array(data, field_count);
-
struct tuple_format_iterator_entry entry;
while (tuple_format_iterator_next(&it, &entry) == 0 &&
entry.data != NULL) {
@@ -457,8 +453,12 @@ vy_stmt_new_surrogate_delete_raw(struct tuple_format *format,
entry.field->token.len);
}
/* Initialize field_map with data offset. */
- if (entry.field->offset_slot != TUPLE_OFFSET_SLOT_NIL)
- field_map[entry.field->offset_slot] = pos - data;
+ uint32_t offset_slot = entry.field->offset_slot;
+ if (offset_slot != TUPLE_OFFSET_SLOT_NIL &&
+ field_map_builder_set_slot(&builder, offset_slot,
+ pos - data, entry.multikey_idx,
+ entry.multikey_count, region) != 0)
+ goto out;
/* Copy field data. */
if (entry.field->type == FIELD_TYPE_ARRAY) {
pos = mp_encode_array(pos, entry.count);
@@ -473,13 +473,15 @@ vy_stmt_new_surrogate_delete_raw(struct tuple_format *format,
goto out;
assert(pos <= data + src_size);
uint32_t bsize = pos - data;
- stmt = vy_stmt_alloc(format, bsize);
+ uint32_t field_map_size = field_map_build_size(&builder);
+ stmt = vy_stmt_alloc(format, sizeof(struct vy_stmt) + field_map_size,
+ bsize);
if (stmt == NULL)
goto out;
char *stmt_data = (char *) tuple_data(stmt);
- char *stmt_field_map_begin = stmt_data - format->field_map_size;
+ char *stmt_field_map_begin = stmt_data - field_map_size;
memcpy(stmt_data, data, bsize);
- memcpy(stmt_field_map_begin, field_map_begin, format->field_map_size);
+ field_map_build(&builder, stmt_field_map_begin);
vy_stmt_set_type(stmt, IPROTO_DELETE);
mp_tuple_assert(stmt_data, stmt_data + bsize);
out:
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 6/7] vinyl: use multikey hints while writing runs
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (4 preceding siblings ...)
2019-05-08 17:22 ` [PATCH 5/7] vinyl: use field_map_builder for constructing stmt field map Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-08 17:22 ` [PATCH 7/7] vinyl: implement multikey index support Vladimir Davydov
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
Currently, we completely ignore vy_entry.hint while writing a run file,
because they only contain auxiliary information for tuple comparison.
However, soon we will use hints to store multikey offsets, which is
mandatory for extracting keys and hence writing secondary run files.
So this patch propagates vy_entry.hint as multikey offset to tuple_bloom
and tuple_extract_key in vy_run implementation.
---
src/box/vy_run.c | 34 ++++++++++++++++++++--------------
src/box/vy_stmt.c | 21 +++++++++++++--------
src/box/vy_stmt.h | 24 +++++++++++++++++++-----
3 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 409c3d96..0b9950f6 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1333,8 +1333,7 @@ vy_run_iterator_seek(struct vy_run_iterator *itr, struct vy_entry last,
/* Check the bloom filter on the first iteration. */
bool check_bloom = (itr->iterator_type == ITER_EQ &&
itr->curr.stmt == NULL && bloom != NULL);
- if (check_bloom && !vy_stmt_bloom_maybe_has(bloom, itr->key.stmt,
- itr->key_def)) {
+ if (check_bloom && !vy_bloom_maybe_has(bloom, itr->key, itr->key_def)) {
vy_run_iterator_stop(itr);
itr->stat->bloom_hit++;
return 0;
@@ -1773,7 +1772,9 @@ vy_run_dump_stmt(struct vy_entry entry, struct xlog *data_xlog,
struct xrow_header xrow;
int rc = (is_primary ?
vy_stmt_encode_primary(entry.stmt, key_def, 0, &xrow) :
- vy_stmt_encode_secondary(entry.stmt, key_def, &xrow));
+ vy_stmt_encode_secondary(entry.stmt, key_def,
+ vy_entry_multikey_idx(entry, key_def),
+ &xrow));
if (rc != 0)
return -1;
@@ -2166,8 +2167,10 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
return -1;
const char *key = vy_stmt_is_key(first_entry.stmt) ?
tuple_data(first_entry.stmt) :
- tuple_extract_key(first_entry.stmt,
- writer->cmp_def, -1, NULL);
+ tuple_extract_key(first_entry.stmt, writer->cmp_def,
+ vy_entry_multikey_idx(first_entry,
+ writer->cmp_def),
+ NULL);
if (key == NULL)
return -1;
if (run->info.page_count == 0) {
@@ -2196,8 +2199,7 @@ static int
vy_run_writer_write_to_page(struct vy_run_writer *writer, struct vy_entry entry)
{
if (writer->bloom != NULL &&
- vy_stmt_bloom_builder_add(writer->bloom, entry.stmt,
- writer->key_def) != 0)
+ vy_bloom_builder_add(writer->bloom, entry, writer->key_def) != 0)
return -1;
if (writer->last.stmt != NULL)
vy_stmt_unref_if_possible(writer->last.stmt);
@@ -2320,8 +2322,10 @@ vy_run_writer_commit(struct vy_run_writer *writer)
assert(writer->last.stmt != NULL);
const char *key = vy_stmt_is_key(writer->last.stmt) ?
tuple_data(writer->last.stmt) :
- tuple_extract_key(writer->last.stmt,
- writer->cmp_def, -1, NULL);
+ tuple_extract_key(writer->last.stmt, writer->cmp_def,
+ vy_entry_multikey_idx(writer->last,
+ writer->cmp_def),
+ NULL);
if (key == NULL)
goto out;
@@ -2424,11 +2428,13 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
struct tuple *tuple = vy_stmt_decode(&xrow, format);
if (tuple == NULL)
goto close_err;
- if (bloom_builder != NULL &&
- vy_stmt_bloom_builder_add(bloom_builder, tuple,
- key_def) != 0) {
- tuple_unref(tuple);
- goto close_err;
+ if (bloom_builder != NULL) {
+ struct vy_entry entry = {tuple, HINT_NONE};
+ if (vy_bloom_builder_add(bloom_builder, entry,
+ key_def) != 0) {
+ tuple_unref(tuple);
+ goto close_err;
+ }
}
key = vy_stmt_is_key(tuple) ? tuple_data(tuple) :
tuple_extract_key(tuple, cmp_def, -1, NULL);
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 170c258a..8ce785a5 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -527,30 +527,34 @@ vy_stmt_extract_key_raw(const char *data, const char *data_end,
}
int
-vy_stmt_bloom_builder_add(struct tuple_bloom_builder *builder,
- struct tuple *stmt, struct key_def *key_def)
+vy_bloom_builder_add(struct tuple_bloom_builder *builder,
+ struct vy_entry entry, struct key_def *key_def)
{
+ struct tuple *stmt = entry.stmt;
if (vy_stmt_is_key(stmt)) {
const char *data = tuple_data(stmt);
uint32_t part_count = mp_decode_array(&data);
return tuple_bloom_builder_add_key(builder, data,
part_count, key_def);
} else {
- return tuple_bloom_builder_add(builder, stmt, key_def, -1);
+ return tuple_bloom_builder_add(builder, stmt, key_def,
+ vy_entry_multikey_idx(entry, key_def));
}
}
bool
-vy_stmt_bloom_maybe_has(const struct tuple_bloom *bloom,
- struct tuple *stmt, struct key_def *key_def)
+vy_bloom_maybe_has(const struct tuple_bloom *bloom,
+ struct vy_entry entry, struct key_def *key_def)
{
+ struct tuple *stmt = entry.stmt;
if (vy_stmt_is_key(stmt)) {
const char *data = tuple_data(stmt);
uint32_t part_count = mp_decode_array(&data);
return tuple_bloom_maybe_has_key(bloom, data,
part_count, key_def);
} else {
- return tuple_bloom_maybe_has(bloom, stmt, key_def, -1);
+ return tuple_bloom_maybe_has(bloom, stmt, key_def,
+ vy_entry_multikey_idx(entry, key_def));
}
}
@@ -656,7 +660,7 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
int
vy_stmt_encode_secondary(struct tuple *value, struct key_def *cmp_def,
- struct xrow_header *xrow)
+ int multikey_idx, struct xrow_header *xrow)
{
memset(xrow, 0, sizeof(*xrow));
enum iproto_type type = vy_stmt_type(value);
@@ -669,7 +673,8 @@ vy_stmt_encode_secondary(struct tuple *value, struct key_def *cmp_def,
uint32_t size;
const char *extracted = vy_stmt_is_key(value) ?
tuple_data_range(value, &size) :
- tuple_extract_key(value, cmp_def, -1, &size);
+ tuple_extract_key(value, cmp_def,
+ multikey_idx, &size);
if (extracted == NULL)
return -1;
if (type == IPROTO_REPLACE || type == IPROTO_INSERT) {
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 929e537a..b0731d3d 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -623,16 +623,16 @@ vy_stmt_extract_key_raw(const char *data, const char *data_end,
* See tuple_bloom_builder_add() for more details.
*/
int
-vy_stmt_bloom_builder_add(struct tuple_bloom_builder *builder,
- struct tuple *stmt, struct key_def *key_def);
+vy_bloom_builder_add(struct tuple_bloom_builder *builder,
+ struct vy_entry entry, struct key_def *key_def);
/**
* Check if a statement hash is present in a bloom filter.
* See tuple_bloom_maybe_has() for more details.
*/
bool
-vy_stmt_bloom_maybe_has(const struct tuple_bloom *bloom,
- struct tuple *stmt, struct key_def *key_def);
+vy_bloom_maybe_has(const struct tuple_bloom *bloom,
+ struct vy_entry entry, struct key_def *key_def);
/**
* Encode vy_stmt for a primary key as xrow_header
@@ -655,6 +655,7 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
*
* @param value statement to encode
* @param key_def key definition
+ * @param multikey_idx multikey index hint
* @param xrow[out] xrow to fill
*
* @retval 0 if OK
@@ -662,7 +663,7 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
*/
int
vy_stmt_encode_secondary(struct tuple *value, struct key_def *cmp_def,
- struct xrow_header *xrow);
+ int multikey_idx, struct xrow_header *xrow);
/**
* Reconstruct vinyl tuple info and data from xrow
@@ -689,6 +690,19 @@ const char *
vy_stmt_str(struct tuple *stmt);
/**
+ * Extract a multikey index hint from a statement entry.
+ * Returns -1 if the key definition isn't multikey.
+ */
+static inline int
+vy_entry_multikey_idx(struct vy_entry entry, struct key_def *key_def)
+{
+ if (!key_def_is_multikey(key_def) || vy_stmt_is_key(entry.stmt))
+ return -1;
+ assert(entry.hint != HINT_NONE);
+ return (int)entry.hint;
+}
+
+/**
* Create a key entry from a MessagePack array without a header.
*/
static inline struct vy_entry
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 7/7] vinyl: implement multikey index support
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (5 preceding siblings ...)
2019-05-08 17:22 ` [PATCH 6/7] vinyl: use multikey hints while writing runs Vladimir Davydov
@ 2019-05-08 17:22 ` Vladimir Davydov
2019-05-13 16:34 ` [PATCH] Use MULTIKEY_NONE instead of -1 Vladimir Davydov
2019-05-13 19:26 ` [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-08 17:22 UTC (permalink / raw)
To: tarantool-patches
In case of multikey indexes, we use vy_entry.hint to store multikey
array entry index instead of a comparison hint. So all we need to do is
patch all places where a statement is inserted so that in case the key
definition is multikey we iterate over all multikey indexes and insert
an entry for each of them. The rest will be done automatically as vinyl
stores and compares vy_entry objects, which have hints built-in, while
comparators and other generic functions have already been patched to
treat hints as multikey indexes.
There are just a few places we need to patch:
- vy_tx_set, which inserts a statement into a transaction write set.
- vy_build_insert_stmt, which is used to fill the new index on index
creation and DDL recovery.
- vy_build_on_replace, which forwards modifications done to the space
during index creation to the new index.
- vy_check_is_unique_secondary, which checks a secondary index for
conflicts on insertion of a new statement.
- vy_tx_handle_deferred_delete, which generates deferred DELETE
statements if the old tuple is found in memory or in cache.
- vy_deferred_delete_on_replace, which applies deferred DELETEs on
compaction.
Plus, we need to teach vy_get_by_secondary_tuple to match a full
multikey tuple to a partial multikey tuple or a key, which implies
iterating over all multikey indexes of the full tuple and comparing
them to the corresponding entries to the partial tuple.
We already have tests that check the functionality for memtx. Enable and
tweak it a little so that it can be used for vinyl as well.
---
src/box/vinyl.c | 143 ++++++++++++++++++++++++------------------
src/box/vy_stmt.h | 25 ++++++++
src/box/vy_tx.c | 43 +++++++++----
src/box/vy_tx.h | 19 +-----
test/box/bitset.result | 15 +++++
test/box/bitset.test.lua | 6 ++
test/box/hash.result | 15 +++++
test/box/hash.test.lua | 6 ++
test/box/rtree_misc.result | 15 +++++
test/box/rtree_misc.test.lua | 6 ++
test/engine/engine.cfg | 3 -
test/engine/multikey.result | 40 +-----------
test/engine/multikey.test.lua | 16 +----
13 files changed, 209 insertions(+), 143 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index eddbdbed..5be10d59 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -701,11 +701,6 @@ vinyl_space_check_index_def(struct space *space, struct index_def *index_def)
return -1;
}
}
- if (key_def_is_multikey(index_def->key_def)) {
- diag_set(ClientError, ER_UNSUPPORTED,
- "Vinyl", "multikey indexes");
- return -1;
- }
return 0;
}
@@ -1355,19 +1350,24 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
lsm->pk->stat.lookup++;
- if (vy_point_lookup(lsm->pk, tx, rv, key, result) != 0) {
+ struct vy_entry pk_entry;
+ if (vy_point_lookup(lsm->pk, tx, rv, key, &pk_entry) != 0) {
rc = -1;
goto out;
}
- /*
- * Note, result stores a hint computed for the primary
- * index while entry was read from a secondary index so
- * we must not use vy_entry_compare() here.
- */
- if (result->stmt == NULL ||
- vy_stmt_compare(result->stmt, HINT_NONE, entry.stmt,
- HINT_NONE, lsm->cmp_def) != 0) {
+ bool match = false;
+ struct vy_entry full_entry;
+ if (pk_entry.stmt != NULL) {
+ vy_stmt_foreach_entry(full_entry, pk_entry.stmt, lsm->cmp_def) {
+ if (vy_entry_compare(full_entry, entry,
+ lsm->cmp_def) == 0) {
+ match = true;
+ break;
+ }
+ }
+ }
+ if (!match) {
/*
* If a tuple read from a secondary index doesn't
* match the tuple corresponding to it in the
@@ -1377,11 +1377,10 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
* case silently skip this tuple.
*/
vy_stmt_counter_acct_tuple(&lsm->stat.skip, entry.stmt);
- if (result->stmt != NULL) {
+ if (pk_entry.stmt != NULL) {
vy_stmt_counter_acct_tuple(&lsm->pk->stat.skip,
- result->stmt);
- tuple_unref(result->stmt);
- *result = vy_entry_none();
+ pk_entry.stmt);
+ tuple_unref(pk_entry.stmt);
}
/*
* We must purge stale tuples from the cache before
@@ -1390,6 +1389,7 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
* the tuple cache implementation.
*/
vy_cache_on_write(&lsm->cache, entry, NULL);
+ *result = vy_entry_none();
goto out;
}
@@ -1401,21 +1401,19 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
* the DELETE statement is not written to secondary indexes
* immediately.
*/
- if (tx != NULL && vy_tx_track_point(tx, lsm->pk, *result) != 0) {
- tuple_unref(result->stmt);
+ if (tx != NULL && vy_tx_track_point(tx, lsm->pk, pk_entry) != 0) {
+ tuple_unref(pk_entry.stmt);
rc = -1;
goto out;
}
if ((*rv)->vlsn == INT64_MAX) {
- vy_cache_add(&lsm->pk->cache, *result,
+ vy_cache_add(&lsm->pk->cache, pk_entry,
vy_entry_none(), key, ITER_EQ);
}
- /* Inherit the hint from the secondary index entry. */
- result->hint = entry.hint;
-
- vy_stmt_counter_acct_tuple(&lsm->pk->stat.get, result->stmt);
+ vy_stmt_counter_acct_tuple(&lsm->pk->stat.get, pk_entry.stmt);
+ *result = full_entry;
out:
tuple_unref(key.stmt);
return rc;
@@ -1576,35 +1574,22 @@ vy_check_is_unique_primary(struct vy_tx *tx, const struct vy_read_view **rv,
return 0;
}
-/**
- * Check if insertion of a new tuple violates unique constraint
- * of a secondary index.
- * @param tx Current transaction.
- * @param rv Read view.
- * @param space_name Space name.
- * @param index_name Index name.
- * @param lsm LSM tree corresponding to the index.
- * @param stmt New tuple.
- *
- * @retval 0 Success, unique constraint is satisfied.
- * @retval -1 Duplicate is found or read error occurred.
- */
static int
-vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
- const char *space_name, const char *index_name,
- struct vy_lsm *lsm, struct tuple *stmt)
+vy_check_is_unique_secondary_one(struct vy_tx *tx, const struct vy_read_view **rv,
+ const char *space_name, const char *index_name,
+ struct vy_lsm *lsm, struct tuple *stmt,
+ int multikey_idx)
{
assert(lsm->index_id > 0);
assert(vy_stmt_type(stmt) == IPROTO_INSERT ||
vy_stmt_type(stmt) == IPROTO_REPLACE);
- if (!lsm->check_is_unique)
- return 0;
if (lsm->key_def->is_nullable &&
- tuple_key_contains_null(stmt, lsm->key_def, -1))
+ tuple_key_contains_null(stmt, lsm->key_def, multikey_idx))
return 0;
struct tuple *key = vy_stmt_extract_key(stmt, lsm->key_def,
- lsm->env->key_format, -1);
+ lsm->env->key_format,
+ multikey_idx);
if (key == NULL)
return -1;
struct tuple *found;
@@ -1639,6 +1624,39 @@ vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
/**
* Check if insertion of a new tuple violates unique constraint
+ * of a secondary index.
+ * @param tx Current transaction.
+ * @param rv Read view.
+ * @param space_name Space name.
+ * @param index_name Index name.
+ * @param lsm LSM tree corresponding to the index.
+ * @param stmt New tuple.
+ *
+ * @retval 0 Success, unique constraint is satisfied.
+ * @retval -1 Duplicate is found or read error occurred.
+ */
+static int
+vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
+ const char *space_name, const char *index_name,
+ struct vy_lsm *lsm, struct tuple *stmt)
+{
+ if (!lsm->check_is_unique)
+ return 0;
+ if (!key_def_is_multikey(lsm->cmp_def)) {
+ return vy_check_is_unique_secondary_one(tx, rv,
+ space_name, index_name, lsm, stmt, -1);
+ }
+ int count = tuple_multikey_count(stmt, lsm->cmp_def);
+ for (int i = 0; i < count; ++i) {
+ if (vy_check_is_unique_secondary_one(tx, rv,
+ space_name, index_name, lsm, stmt, i) != 0)
+ return -1;
+ }
+ return 0;
+}
+
+/**
+ * Check if insertion of a new tuple violates unique constraint
* of any index of the space.
* @param env Vinyl environment.
* @param tx Current transaction.
@@ -4063,11 +4081,10 @@ vy_build_on_replace(struct trigger *trigger, void *event)
/* Forward the statement to the new LSM tree. */
if (stmt->old_tuple != NULL) {
- struct tuple *delete = vy_stmt_extract_key(stmt->old_tuple,
- lsm->cmp_def, lsm->env->key_format, -1);
+ struct tuple *delete = vy_stmt_new_surrogate_delete(format,
+ stmt->old_tuple);
if (delete == NULL)
goto err;
- vy_stmt_set_type(delete, IPROTO_DELETE);
int rc = vy_tx_set(tx, lsm, delete);
tuple_unref(delete);
if (rc != 0)
@@ -4114,12 +4131,13 @@ vy_build_insert_stmt(struct vy_lsm *lsm, struct vy_mem *mem,
return -1;
vy_stmt_set_lsn(region_stmt, lsn);
struct vy_entry entry;
- entry.stmt = region_stmt;
- entry.hint = vy_stmt_hint(region_stmt, lsm->cmp_def);
- if (vy_mem_insert(mem, entry) != 0)
- return -1;
- vy_mem_commit_stmt(mem, entry);
- vy_stmt_counter_acct_tuple(&lsm->stat.memory.count, region_stmt);
+ vy_stmt_foreach_entry(entry, region_stmt, lsm->cmp_def) {
+ if (vy_mem_insert(mem, entry) != 0)
+ return -1;
+ vy_mem_commit_stmt(mem, entry);
+ vy_stmt_counter_acct_tuple(&lsm->stat.memory.count,
+ region_stmt);
+ }
return 0;
}
@@ -4218,11 +4236,10 @@ vy_build_recover_stmt(struct vy_lsm *lsm, struct vy_lsm *pk,
struct tuple *insert = NULL;
struct tuple *old_tuple = old.stmt;
if (old_tuple != NULL) {
- delete = vy_stmt_extract_key(old_tuple, lsm->cmp_def,
- lsm->env->key_format, -1);
+ delete = vy_stmt_new_surrogate_delete(lsm->mem_format,
+ old_tuple);
if (delete == NULL)
return -1;
- vy_stmt_set_type(delete, IPROTO_DELETE);
}
enum iproto_type type = vy_stmt_type(mem_stmt);
if (type == IPROTO_REPLACE || type == IPROTO_INSERT) {
@@ -4578,13 +4595,15 @@ vy_deferred_delete_on_replace(struct trigger *trigger, void *event)
mem = lsm->mem;
}
struct vy_entry entry;
- entry.stmt = delete;
- entry.hint = vy_stmt_hint(delete, lsm->cmp_def);
- rc = vy_lsm_set(lsm, mem, entry, ®ion_stmt);
+ vy_stmt_foreach_entry(entry, delete, lsm->cmp_def) {
+ rc = vy_lsm_set(lsm, mem, entry, ®ion_stmt);
+ if (rc != 0)
+ break;
+ entry.stmt = region_stmt;
+ vy_lsm_commit_stmt(lsm, mem, entry);
+ }
if (rc != 0)
break;
- entry.stmt = region_stmt;
- vy_lsm_commit_stmt(lsm, mem, entry);
if (!is_first_statement)
continue;
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index b0731d3d..0026d614 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -750,6 +750,31 @@ vy_entry_compare_with_raw_key(struct vy_entry entry,
key, key_hint, key_def);
}
+/**
+ * Iterate over each key indexed in the given statement.
+ * @param entry loop variable
+ * @param src_stmt source statement
+ * @param key_def key definition
+ *
+ * For a multikey index, entry.hint is set to multikey entry offset
+ * and the loop iterates over each offset stored in the statement.
+ *
+ * For a unikey index, entry.hint is initialized with vy_stmt_hint()
+ * and the loop breaks after the first iteration.
+ *
+ * entry.stmt is set to src_stmt on each iteration.
+ */
+#define vy_stmt_foreach_entry(entry, src_stmt, key_def) \
+ for (uint32_t multikey_idx = 0, \
+ multikey_count = !key_def_is_multikey((key_def)) ? 1 : \
+ tuple_multikey_count((src_stmt), (key_def)); \
+ multikey_idx < multikey_count && \
+ (((entry).stmt = (src_stmt)), \
+ ((entry).hint = !key_def_is_multikey((key_def)) ? \
+ vy_stmt_hint((src_stmt), (key_def)) : \
+ multikey_idx), true); \
+ ++multikey_idx)
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index 92d7c2a1..b1dee0fa 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -629,17 +629,21 @@ vy_tx_handle_deferred_delete(struct vy_tx *tx, struct txv *v)
int rc = 0;
for (uint32_t i = 1; i < space->index_count; i++) {
struct vy_lsm *lsm = vy_lsm(space->index[i]);
- struct vy_entry delete_entry;
- delete_entry.stmt = delete_stmt;
- delete_entry.hint = vy_stmt_hint(delete_stmt, lsm->cmp_def);
- struct txv *delete_txv = txv_new(tx, lsm, delete_entry,
- UINT64_MAX);
- if (delete_txv == NULL) {
- rc = -1;
+ struct vy_entry entry;
+ vy_stmt_foreach_entry(entry, delete_stmt, lsm->cmp_def) {
+ struct txv *delete_txv = txv_new(tx, lsm, entry,
+ UINT64_MAX);
+ if (delete_txv == NULL) {
+ rc = -1;
+ break;
+ }
+ stailq_insert_entry(&tx->log, delete_txv, v,
+ next_in_log);
+ vy_stmt_counter_acct_tuple(&lsm->stat.txw.count,
+ entry.stmt);
+ }
+ if (rc != 0)
break;
- }
- stailq_insert_entry(&tx->log, delete_txv, v, next_in_log);
- vy_stmt_counter_acct_tuple(&lsm->stat.txw.count, delete_stmt);
}
tuple_unref(delete_stmt);
return rc;
@@ -1028,7 +1032,12 @@ vy_tx_track_point(struct vy_tx *tx, struct vy_lsm *lsm, struct vy_entry entry)
return vy_tx_track(tx, lsm, entry, true, entry, true);
}
-int
+/**
+ * Add one statement entry to a transaction. We add one entry
+ * for each index, and with multikey indexes it is possible there
+ * are multiple entries of a single statement in a single index.
+ */
+static int
vy_tx_set_entry(struct vy_tx *tx, struct vy_lsm *lsm,
struct vy_entry entry, uint64_t column_mask)
{
@@ -1123,6 +1132,18 @@ vy_tx_set_entry(struct vy_tx *tx, struct vy_lsm *lsm,
return 0;
}
+int
+vy_tx_set_with_colmask(struct vy_tx *tx, struct vy_lsm *lsm,
+ struct tuple *stmt, uint64_t column_mask)
+{
+ struct vy_entry entry;
+ vy_stmt_foreach_entry(entry, stmt, lsm->cmp_def) {
+ if (vy_tx_set_entry(tx, lsm, entry, column_mask) != 0)
+ return -1;
+ }
+ return 0;
+}
+
void
tx_manager_abort_writers_for_ddl(struct tx_manager *xm, struct space *space)
{
diff --git a/src/box/vy_tx.h b/src/box/vy_tx.h
index a5c0856d..2712263b 100644
--- a/src/box/vy_tx.h
+++ b/src/box/vy_tx.h
@@ -393,15 +393,6 @@ int
vy_tx_track_point(struct vy_tx *tx, struct vy_lsm *lsm, struct vy_entry entry);
/**
- * Add one statement entry to a transaction. We add one entry
- * for each index, and with multikey indexes it is possible there
- * are multiple entries of a single statement in a single index.
- */
-int
-vy_tx_set_entry(struct vy_tx *tx, struct vy_lsm *lsm,
- struct vy_entry entry, uint64_t column_mask);
-
-/**
* Insert a statement into a transaction write set.
* @param tx Transaction.
* @param lsm LSM tree the statement is for.
@@ -411,15 +402,9 @@ vy_tx_set_entry(struct vy_tx *tx, struct vy_lsm *lsm,
* @retval 0 Success
* @retval -1 Memory allocation error.
*/
-static inline int
+int
vy_tx_set_with_colmask(struct vy_tx *tx, struct vy_lsm *lsm,
- struct tuple *stmt, uint64_t column_mask)
-{
- struct vy_entry entry;
- entry.stmt = stmt;
- entry.hint = vy_stmt_hint(stmt, lsm->cmp_def);
- return vy_tx_set_entry(tx, lsm, entry, column_mask);
-}
+ struct tuple *stmt, uint64_t column_mask);
static inline int
vy_tx_set(struct vy_tx *tx, struct vy_lsm *lsm, struct tuple *stmt)
diff --git a/test/box/bitset.result b/test/box/bitset.result
index a06207df..78f74ec3 100644
--- a/test/box/bitset.result
+++ b/test/box/bitset.result
@@ -1981,3 +1981,18 @@ s:drop()
s = nil
---
...
+-- Bitset index cannot be multikey.
+s = box.schema.space.create('test')
+---
+...
+_ = s:create_index('primary')
+---
+...
+_ = s:create_index('bitset', {type = 'bitset', parts = {{'[2][*]', 'unsigned'}}})
+---
+- error: 'Can''t create or modify index ''bitset'' in space ''test'': BITSET index
+ cannot be multikey'
+...
+s:drop()
+---
+...
diff --git a/test/box/bitset.test.lua b/test/box/bitset.test.lua
index d9760e27..eb013a1c 100644
--- a/test/box/bitset.test.lua
+++ b/test/box/bitset.test.lua
@@ -147,3 +147,9 @@ for j=1,100 do check(math.random(9) - 1, {iterator = box.index.BITS_ALL_NOT_SET}
good
s:drop()
s = nil
+
+-- Bitset index cannot be multikey.
+s = box.schema.space.create('test')
+_ = s:create_index('primary')
+_ = s:create_index('bitset', {type = 'bitset', parts = {{'[2][*]', 'unsigned'}}})
+s:drop()
diff --git a/test/box/hash.result b/test/box/hash.result
index f0c30ade..9f08c49b 100644
--- a/test/box/hash.result
+++ b/test/box/hash.result
@@ -832,3 +832,18 @@ s:get(-9007199254740994LL)
s:drop()
---
...
+-- Hash index cannot be multikey.
+s = box.schema.space.create('test')
+---
+...
+_ = s:create_index('primary')
+---
+...
+_ = s:create_index('hash', {type = 'hash', parts = {{'[2][*]', 'unsigned'}}})
+---
+- error: 'Can''t create or modify index ''hash'' in space ''test'': HASH index cannot
+ be multikey'
+...
+s:drop()
+---
+...
diff --git a/test/box/hash.test.lua b/test/box/hash.test.lua
index d97312e3..9801873c 100644
--- a/test/box/hash.test.lua
+++ b/test/box/hash.test.lua
@@ -347,3 +347,9 @@ s:get(-1LL)
s:get(9007199254740992LL)
s:get(-9007199254740994LL)
s:drop()
+
+-- Hash index cannot be multikey.
+s = box.schema.space.create('test')
+_ = s:create_index('primary')
+_ = s:create_index('hash', {type = 'hash', parts = {{'[2][*]', 'unsigned'}}})
+s:drop()
diff --git a/test/box/rtree_misc.result b/test/box/rtree_misc.result
index 36d5b8f5..6e48bacc 100644
--- a/test/box/rtree_misc.result
+++ b/test/box/rtree_misc.result
@@ -667,3 +667,18 @@ i:select({1, 2, 3, 4, 5, 6}, {iterator = 'BITS_ALL_SET' } )
s:drop()
---
...
+-- Rtree index cannot be multikey.
+s = box.schema.space.create('test')
+---
+...
+_ = s:create_index('primary')
+---
+...
+_ = s:create_index('rtree', {type = 'rtree', parts = {{'[2][*]', 'array'}}})
+---
+- error: 'Can''t create or modify index ''rtree'' in space ''test'': RTREE index cannot
+ be multikey'
+...
+s:drop()
+---
+...
diff --git a/test/box/rtree_misc.test.lua b/test/box/rtree_misc.test.lua
index cef66fc1..000a928e 100644
--- a/test/box/rtree_misc.test.lua
+++ b/test/box/rtree_misc.test.lua
@@ -237,3 +237,9 @@ i:select({0, 0, 0}, {iterator = 'neighbor'})
i:select({1, 2, 3, 4, 5, 6}, {iterator = 'BITS_ALL_SET' } )
s:drop()
+
+-- Rtree index cannot be multikey.
+s = box.schema.space.create('test')
+_ = s:create_index('primary')
+_ = s:create_index('rtree', {type = 'rtree', parts = {{'[2][*]', 'array'}}})
+s:drop()
diff --git a/test/engine/engine.cfg b/test/engine/engine.cfg
index 8f34bae8..9f07629b 100644
--- a/test/engine/engine.cfg
+++ b/test/engine/engine.cfg
@@ -2,9 +2,6 @@
"*": {
"memtx": {"engine": "memtx"},
"vinyl": {"engine": "vinyl"}
- },
- "multikey.test.lua": {
- "memtx": {"engine": "memtx"}
}
}
diff --git a/test/engine/multikey.result b/test/engine/multikey.result
index 293e27f1..1d5d9e20 100644
--- a/test/engine/multikey.result
+++ b/test/engine/multikey.result
@@ -7,20 +7,6 @@ engine = test_run:get_cfg('engine')
--
-- gh-1260: Multikey indexes.
--
-s = box.schema.space.create('withdata', {engine = 'vinyl'})
----
-...
-pk = s:create_index('pk')
----
-...
--- Vinyl's space can't be multikey (yet).
-_ = s:create_index('idx', {parts = {{3, 'str', path = '[*].fname'}, {3, 'str', path = '[*].sname'}}})
----
-- error: Vinyl does not support multikey indexes
-...
-s:drop()
----
-...
s = box.schema.space.create('withdata', {engine = engine})
---
...
@@ -33,22 +19,6 @@ _ = s:create_index('idx', {parts = {{3, 'str', path = '[*].fname'}}})
pk = s:create_index('pk')
---
...
--- Only tree index type may be mutlikey.
-_ = s:create_index('idx', {type = 'hash', unique = true, parts = {{3, 'str', path = '[*].fname'}}})
----
-- error: 'Can''t create or modify index ''idx'' in space ''withdata'': HASH index
- cannot be multikey'
-...
-_ = s:create_index('idx', {type = 'bitset', unique = false, parts = {{3, 'str', path = '[*].fname'}}})
----
-- error: 'Can''t create or modify index ''idx'' in space ''withdata'': BITSET index
- cannot be multikey'
-...
-_ = s:create_index('idx', {type = 'rtree', unique = false, parts = {{3, 'array', path = '[*].fname'}}})
----
-- error: 'Can''t create or modify index ''idx'' in space ''withdata'': RTREE index
- cannot be multikey'
-...
-- Test incompatible multikey index parts.
_ = s:create_index('idx3', {parts = {{3, 'str', path = '[*].fname'}, {3, 'str', path = '["data"][*].sname'}}})
---
@@ -146,10 +116,8 @@ arr_idx:select({1})
- [5, [1, 1, 1], [{'fname': 'A', 'sname': 'B'}, {'fname': 'C', 'sname': 'D'}, {
'fname': 'A', 'sname': 'B'}]]
...
-s:delete(5)
+_ = s:delete(5)
---
-- [5, [1, 1, 1], [{'fname': 'A', 'sname': 'B'}, {'fname': 'C', 'sname': 'D'}, {'fname': 'A',
- 'sname': 'B'}]]
...
-- Check that there is no garbage in index.
arr_idx:select({1})
@@ -162,9 +130,8 @@ idx:get({'A', 'B'})
idx:get({'C', 'D'})
---
...
-idx:delete({'Vasya', 'Pupkin'})
+_ = idx:delete({'Vasya', 'Pupkin'})
---
-- [1, [1, 2, 3], [{'fname': 'James', 'sname': 'Bond'}, {'fname': 'Vasya', 'sname': 'Pupkin'}]]
...
s:insert({6, {1, 2}, {{fname='Vasya', sname='Pupkin'}}})
---
@@ -286,9 +253,8 @@ idx0:select()
- - [1, [1, 1, 1]]
- [2, [2, 2]]
...
-idx0:delete(2)
+_ = idx0:delete(2)
---
-- [2, [2, 2]]
...
idx0:get(2)
---
diff --git a/test/engine/multikey.test.lua b/test/engine/multikey.test.lua
index 0916bf52..f32f49d2 100644
--- a/test/engine/multikey.test.lua
+++ b/test/engine/multikey.test.lua
@@ -4,20 +4,10 @@ engine = test_run:get_cfg('engine')
--
-- gh-1260: Multikey indexes.
--
-s = box.schema.space.create('withdata', {engine = 'vinyl'})
-pk = s:create_index('pk')
--- Vinyl's space can't be multikey (yet).
-_ = s:create_index('idx', {parts = {{3, 'str', path = '[*].fname'}, {3, 'str', path = '[*].sname'}}})
-s:drop()
-
s = box.schema.space.create('withdata', {engine = engine})
-- Primary index must be unique so it can't be multikey.
_ = s:create_index('idx', {parts = {{3, 'str', path = '[*].fname'}}})
pk = s:create_index('pk')
--- Only tree index type may be mutlikey.
-_ = s:create_index('idx', {type = 'hash', unique = true, parts = {{3, 'str', path = '[*].fname'}}})
-_ = s:create_index('idx', {type = 'bitset', unique = false, parts = {{3, 'str', path = '[*].fname'}}})
-_ = s:create_index('idx', {type = 'rtree', unique = false, parts = {{3, 'array', path = '[*].fname'}}})
-- Test incompatible multikey index parts.
_ = s:create_index('idx3', {parts = {{3, 'str', path = '[*].fname'}, {3, 'str', path = '["data"][*].sname'}}})
_ = s:create_index('idx2', {parts = {{3, 'str', path = '[*].fname'}, {3, 'str', path = '[*].sname[*].a'}}})
@@ -43,12 +33,12 @@ idx:select()
-- Duplicates in multikey parts.
s:insert({5, {1, 1, 1}, {{fname='A', sname='B'}, {fname='C', sname='D'}, {fname='A', sname='B'}}})
arr_idx:select({1})
-s:delete(5)
+_ = s:delete(5)
-- Check that there is no garbage in index.
arr_idx:select({1})
idx:get({'A', 'B'})
idx:get({'C', 'D'})
-idx:delete({'Vasya', 'Pupkin'})
+_ = idx:delete({'Vasya', 'Pupkin'})
s:insert({6, {1, 2}, {{fname='Vasya', sname='Pupkin'}}})
s:insert({7, {1}, {{fname='James', sname='Bond'}}})
arr_idx:select({1})
@@ -84,7 +74,7 @@ idx0:get(2)
idx0:get(1)
idx0:get(3)
idx0:select()
-idx0:delete(2)
+_ = idx0:delete(2)
idx0:get(2)
idx0:select()
s:drop()
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH] Use MULTIKEY_NONE instead of -1
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (6 preceding siblings ...)
2019-05-08 17:22 ` [PATCH 7/7] vinyl: implement multikey index support Vladimir Davydov
@ 2019-05-13 16:34 ` Vladimir Davydov
2019-05-13 19:26 ` [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-13 16:34 UTC (permalink / raw)
To: tarantool-patches
Solely to improve code readability. No functional changes.
Suggested by @kostja.
---
src/box/field_map.h | 21 ++++++++++++++-------
src/box/index.cc | 3 ++-
src/box/lua/key_def.c | 2 +-
src/box/memtx_bitset.c | 2 +-
src/box/memtx_rtree.c | 2 +-
src/box/memtx_space.c | 3 ++-
src/box/request.c | 7 ++++---
src/box/space.c | 3 ++-
src/box/sql.c | 20 ++++++++++----------
src/box/tuple.c | 8 +++++---
src/box/tuple.h | 2 +-
src/box/tuple_bloom.c | 4 ++--
src/box/tuple_compare.cc | 21 ++++++++++++++-------
| 13 ++++++++-----
src/box/tuple_format.c | 2 +-
src/box/tuple_hash.cc | 11 +++++++----
src/box/vinyl.c | 9 ++++++---
src/box/vy_run.c | 3 ++-
src/box/vy_stmt.c | 3 ++-
src/box/vy_stmt.h | 4 ++--
20 files changed, 87 insertions(+), 56 deletions(-)
diff --git a/src/box/field_map.h b/src/box/field_map.h
index 0fd35c3e..b0dfeb4e 100644
--- a/src/box/field_map.h
+++ b/src/box/field_map.h
@@ -38,6 +38,12 @@ struct region;
struct field_map_builder_slot;
/**
+ * A special value of multikey index that means that the key
+ * definition is not multikey and no indirection is expected.
+ */
+enum { MULTIKEY_NONE = -1 };
+
+/**
* A field map is a special area is reserved before tuple's
* MessagePack data. It is a sequence of the 32-bit unsigned
* offsets of tuple's indexed fields.
@@ -146,7 +152,7 @@ field_map_get_offset(const uint32_t *field_map, int32_t offset_slot,
int multikey_idx)
{
uint32_t offset;
- if (multikey_idx >= 0 && field_map[offset_slot] > 0) {
+ if (multikey_idx != MULTIKEY_NONE && field_map[offset_slot] > 0) {
assert((int32_t)field_map[offset_slot] < 0);
/**
* The field_map extent has the following
@@ -192,10 +198,10 @@ field_map_builder_slot_extent_new(struct field_map_builder *builder,
/**
* Set data offset for a field identified by unique offset_slot.
*
- * When multikey_idx > 0 this routine initialize corresponding
- * field_map_builder_slot_extent is identified by multikey_idx
- * and multikey_count. Performs allocation on region when
- * required.
+ * When multikey_idx != MULTIKEY_NONE this routine initializes
+ * corresponding field_map_builder_slot_extent identified by
+ * multikey_idx and multikey_count. Performs allocation on region
+ * when required.
*
* The offset_slot argument must be negative and offset must be
* positive (by definition).
@@ -209,10 +215,11 @@ field_map_builder_set_slot(struct field_map_builder *builder,
assert(offset_slot < 0);
assert((uint32_t)-offset_slot <= builder->slot_count);
assert(offset > 0);
- assert(multikey_idx < (int32_t)multikey_count);
- if (multikey_idx == -1) {
+ if (multikey_idx == MULTIKEY_NONE) {
builder->slots[offset_slot].offset = offset;
} else {
+ assert(multikey_idx >= 0);
+ assert(multikey_idx < (int32_t)multikey_count);
struct field_map_builder_slot_extent *extent;
if (builder->slots[offset_slot].has_extent) {
extent = builder->slots[offset_slot].extent;
diff --git a/src/box/index.cc b/src/box/index.cc
index 39b0ee46..4a444e5d 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -156,7 +156,8 @@ box_tuple_extract_key(box_tuple_t *tuple, uint32_t space_id, uint32_t index_id,
struct index *index = index_find(space, index_id);
if (index == NULL)
return NULL;
- return tuple_extract_key(tuple, index->def->key_def, -1, key_size);
+ return tuple_extract_key(tuple, index->def->key_def,
+ MULTIKEY_NONE, key_size);
}
static inline int
diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index 28d40478..dfcc8944 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -265,7 +265,7 @@ lbox_key_def_extract_key(struct lua_State *L)
struct region *region = &fiber()->gc;
size_t region_svp = region_used(region);
uint32_t key_size;
- char *key = tuple_extract_key(tuple, key_def, -1, &key_size);
+ char *key = tuple_extract_key(tuple, key_def, MULTIKEY_NONE, &key_size);
tuple_unref(tuple);
if (key == NULL)
return luaT_error(L);
diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
index 275e736d..8c626684 100644
--- a/src/box/memtx_bitset.c
+++ b/src/box/memtx_bitset.c
@@ -302,7 +302,7 @@ memtx_bitset_index_replace(struct index *base, struct tuple *old_tuple,
if (new_tuple != NULL) {
const char *field = tuple_field_by_part(new_tuple,
- base->def->key_def->parts, -1);
+ base->def->key_def->parts, MULTIKEY_NONE);
uint32_t key_len;
const void *key = make_key(field, &key_len);
#ifndef OLD_GOOD_BITSET
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index 19cb69a1..45e8fb8e 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -122,7 +122,7 @@ extract_rectangle(struct rtree_rect *rect, struct tuple *tuple,
assert(index_def->key_def->part_count == 1);
assert(!key_def_is_multikey(index_def->key_def));
const char *elems = tuple_field_by_part(tuple,
- index_def->key_def->parts, -1);
+ index_def->key_def->parts, MULTIKEY_NONE);
unsigned dimension = index_def->opts.dimension;
uint32_t count = mp_decode_array(&elems);
return mp_decode_rect(rect, dimension, elems, count, "Field");
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 265bf923..5ddb4f7e 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -445,7 +445,8 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
/* Extract the primary key from tuple. */
const char *key = tuple_extract_key_raw(request->tuple,
request->tuple_end,
- index->def->key_def, -1, NULL);
+ index->def->key_def,
+ MULTIKEY_NONE, NULL);
if (key == NULL)
return -1;
/* Cut array header */
diff --git a/src/box/request.c b/src/box/request.c
index 8b84260a..44a43ee1 100644
--- a/src/box/request.c
+++ b/src/box/request.c
@@ -92,7 +92,8 @@ request_create_from_tuple(struct request *request, struct space *space,
uint32_t size, key_size;
const char *data = tuple_data_range(old_tuple, &size);
request->key = tuple_extract_key_raw(data, data + size,
- space->index[0]->def->key_def, -1, &key_size);
+ space->index[0]->def->key_def, MULTIKEY_NONE,
+ &key_size);
if (request->key == NULL)
return -1;
request->key_end = request->key + key_size;
@@ -125,8 +126,8 @@ request_rebind_to_primary_key(struct request *request, struct space *space,
struct index *pk = space_index(space, 0);
assert(pk != NULL);
uint32_t key_len;
- char *key = tuple_extract_key(found_tuple, pk->def->key_def, -1,
- &key_len);
+ char *key = tuple_extract_key(found_tuple, pk->def->key_def,
+ MULTIKEY_NONE, &key_len);
assert(key != NULL);
request->key = key;
request->key_end = key + key_len;
diff --git a/src/box/space.c b/src/box/space.c
index 9a266604..243e7da2 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -320,7 +320,8 @@ space_before_replace(struct space *space, struct txn *txn,
break;
index = pk;
key = tuple_extract_key_raw(request->tuple, request->tuple_end,
- index->def->key_def, -1, NULL);
+ index->def->key_def, MULTIKEY_NONE,
+ NULL);
if (key == NULL)
return -1;
part_count = mp_decode_array(&key);
diff --git a/src/box/sql.c b/src/box/sql.c
index c3f404ee..fbfa5999 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -480,8 +480,8 @@ int tarantoolsqlEphemeralDelete(BtCursor *pCur)
char *key;
uint32_t key_size;
key = tuple_extract_key(pCur->last_tuple,
- pCur->iter->index->def->key_def, -1,
- &key_size);
+ pCur->iter->index->def->key_def,
+ MULTIKEY_NONE, &key_size);
if (key == NULL)
return SQL_TARANTOOL_DELETE_FAIL;
@@ -506,8 +506,8 @@ int tarantoolsqlDelete(BtCursor *pCur, u8 flags)
int rc;
key = tuple_extract_key(pCur->last_tuple,
- pCur->iter->index->def->key_def, -1,
- &key_size);
+ pCur->iter->index->def->key_def,
+ MULTIKEY_NONE, &key_size);
if (key == NULL)
return SQL_TARANTOOL_DELETE_FAIL;
rc = sql_delete_by_key(pCur->space, pCur->index->def->iid, key,
@@ -561,8 +561,8 @@ int tarantoolsqlEphemeralClearTable(BtCursor *pCur)
uint32_t key_size;
while (iterator_next(it, &tuple) == 0 && tuple != NULL) {
- key = tuple_extract_key(tuple, it->index->def->key_def, -1,
- &key_size);
+ key = tuple_extract_key(tuple, it->index->def->key_def,
+ MULTIKEY_NONE, &key_size);
if (space_ephemeral_delete(pCur->space, key) != 0) {
iterator_delete(it);
return SQL_TARANTOOL_DELETE_FAIL;
@@ -594,8 +594,8 @@ int tarantoolsqlClearTable(struct space *space, uint32_t *tuple_count)
if (iter == NULL)
return SQL_TARANTOOL_ITERATOR_FAIL;
while (iterator_next(iter, &tuple) == 0 && tuple != NULL) {
- request.key = tuple_extract_key(tuple, pk->def->key_def, -1,
- &key_size);
+ request.key = tuple_extract_key(tuple, pk->def->key_def,
+ MULTIKEY_NONE, &key_size);
request.key_end = request.key + key_size;
rc = box_process_rw(&request, space, &unused);
if (rc != 0) {
@@ -783,7 +783,7 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor,
uint32_t field_offset =
field_map_get_offset(field_map,
field->offset_slot,
- -1);
+ MULTIKEY_NONE);
p = base + field_offset;
}
}
@@ -801,7 +801,7 @@ out:
#ifndef NDEBUG
/* Sanity check. */
original_size = region_used(&fiber()->gc);
- key = tuple_extract_key(tuple, key_def, -1, &key_size);
+ key = tuple_extract_key(tuple, key_def, MULTIKEY_NONE, &key_size);
if (key != NULL) {
int new_rc = sqlVdbeRecordCompareMsgpack(key, unpacked);
region_truncate(&fiber()->gc, original_size);
diff --git a/src/box/tuple.c b/src/box/tuple.c
index fe815a64..45c6727a 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -462,7 +462,7 @@ tuple_go_to_path(const char **data, const char *path, uint32_t path_len,
while ((rc = json_lexer_next_token(&lexer, &token)) == 0) {
switch (token.type) {
case JSON_TOKEN_ANY:
- if (multikey_idx < 0)
+ if (multikey_idx == MULTIKEY_NONE)
return -1;
token.num = multikey_idx;
FALLTHROUGH;
@@ -535,7 +535,8 @@ tuple_field_raw_by_full_path(struct tuple_format *format, const char *tuple,
}
return tuple_field_raw_by_path(format, tuple, field_map, fieldno,
path + lexer.offset,
- path_len - lexer.offset, NULL, -1);
+ path_len - lexer.offset,
+ NULL, MULTIKEY_NONE);
}
uint32_t
@@ -548,7 +549,8 @@ tuple_raw_multikey_count(struct tuple_format *format, const char *data,
tuple_field_raw_by_path(format, data, field_map,
key_def->multikey_fieldno,
key_def->multikey_path,
- key_def->multikey_path_len, NULL, -1);
+ key_def->multikey_path_len,
+ NULL, MULTIKEY_NONE);
if (array_raw == NULL)
return 0;
assert(mp_typeof(*array_raw) == MP_ARRAY);
diff --git a/src/box/tuple.h b/src/box/tuple.h
index 2a46f371..4acda789 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -604,7 +604,7 @@ tuple_field_raw(struct tuple_format *format, const char *tuple,
const uint32_t *field_map, uint32_t field_no)
{
return tuple_field_raw_by_path(format, tuple, field_map, field_no,
- NULL, 0, NULL, -1);
+ NULL, 0, NULL, MULTIKEY_NONE);
}
/**
diff --git a/src/box/tuple_bloom.c b/src/box/tuple_bloom.c
index 99f19a2e..8b74da22 100644
--- a/src/box/tuple_bloom.c
+++ b/src/box/tuple_bloom.c
@@ -109,7 +109,7 @@ tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
int multikey_idx)
{
assert(builder->part_count == key_def->part_count);
- assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
+ assert(!key_def_is_multikey(key_def) || multikey_idx != MULTIKEY_NONE);
uint32_t h = HASH_SEED;
uint32_t carry = 0;
@@ -204,7 +204,7 @@ bool
tuple_bloom_maybe_has(const struct tuple_bloom *bloom, struct tuple *tuple,
struct key_def *key_def, int multikey_idx)
{
- assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
+ assert(!key_def_is_multikey(key_def) || multikey_idx != MULTIKEY_NONE);
if (bloom->is_legacy) {
return bloom_maybe_has(&bloom->parts[0],
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 0aa032af..c1a70a08 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -512,9 +512,11 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
(int)tuple_b_hint);
} else if (has_json_paths) {
field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
- field_map_a, part, -1);
+ field_map_a, part,
+ MULTIKEY_NONE);
field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
- field_map_b, part, -1);
+ field_map_b, part,
+ MULTIKEY_NONE);
} else {
field_a = tuple_field_raw(format_a, tuple_a_raw,
field_map_a, part->fieldno);
@@ -576,9 +578,11 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
(int)tuple_b_hint);
} else if (has_json_paths) {
field_a = tuple_field_raw_by_part(format_a, tuple_a_raw,
- field_map_a, part, -1);
+ field_map_a, part,
+ MULTIKEY_NONE);
field_b = tuple_field_raw_by_part(format_b, tuple_b_raw,
- field_map_b, part, -1);
+ field_map_b, part,
+ MULTIKEY_NONE);
} else {
field_a = tuple_field_raw(format_a, tuple_a_raw,
field_map_a, part->fieldno);
@@ -630,7 +634,8 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint,
(int)tuple_hint);
} else if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part, -1);
+ field_map, part,
+ MULTIKEY_NONE);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
@@ -664,7 +669,8 @@ tuple_compare_with_key_slowpath(struct tuple *tuple, hint_t tuple_hint,
(int)tuple_hint);
} else if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part, -1);
+ field_map, part,
+ MULTIKEY_NONE);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
@@ -1564,7 +1570,8 @@ static hint_t
tuple_hint(struct tuple *tuple, struct key_def *key_def)
{
assert(!key_def_is_multikey(key_def));
- const char *field = tuple_field_by_part(tuple, key_def->parts, -1);
+ const char *field = tuple_field_by_part(tuple, key_def->parts,
+ MULTIKEY_NONE);
if (is_nullable && field == NULL)
return hint_nil();
return field_hint<type, is_nullable>(field, key_def->parts->coll);
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 15e43aae..3bd3cde7 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -119,7 +119,7 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
assert(contains_sequential_parts ==
key_def_contains_sequential_parts(key_def));
assert(is_multikey == key_def_is_multikey(key_def));
- assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
+ assert(!key_def_is_multikey(key_def) || multikey_idx != MULTIKEY_NONE);
assert(mp_sizeof_nil() == 1);
const char *data = tuple_data(tuple);
uint32_t part_count = key_def->part_count;
@@ -136,7 +136,8 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
key_def->parts[i].fieldno);
} else if (!is_multikey) {
field = tuple_field_raw_by_part(format, data, field_map,
- &key_def->parts[i], -1);
+ &key_def->parts[i],
+ MULTIKEY_NONE);
} else {
field = tuple_field_raw_by_part(format, data, field_map,
&key_def->parts[i],
@@ -187,7 +188,8 @@ tuple_extract_key_slowpath(struct tuple *tuple, struct key_def *key_def,
key_def->parts[i].fieldno);
} else if (!is_multikey) {
field = tuple_field_raw_by_part(format, data, field_map,
- &key_def->parts[i], -1);
+ &key_def->parts[i],
+ MULTIKEY_NONE);
} else {
field = tuple_field_raw_by_part(format, data, field_map,
&key_def->parts[i],
@@ -248,7 +250,7 @@ tuple_extract_key_slowpath_raw(const char *data, const char *data_end,
assert(has_json_paths == key_def->has_json_paths);
assert(!has_optional_parts || key_def->is_nullable);
assert(has_optional_parts == key_def->has_optional_parts);
- assert(!key_def_is_multikey(key_def) || multikey_idx >= 0);
+ assert(!key_def_is_multikey(key_def) || multikey_idx != MULTIKEY_NONE);
assert(mp_sizeof_nil() == 1);
/* allocate buffer with maximal possible size */
char *key = (char *) region_alloc(&fiber()->gc, data_end - data);
@@ -453,7 +455,8 @@ tuple_validate_key_parts(struct key_def *key_def, struct tuple *tuple)
assert(!key_def_is_multikey(key_def));
for (uint32_t idx = 0; idx < key_def->part_count; idx++) {
struct key_part *part = &key_def->parts[idx];
- const char *field = tuple_field_by_part(tuple, part, -1);
+ const char *field = tuple_field_by_part(tuple, part,
+ MULTIKEY_NONE);
if (field == NULL) {
if (key_part_is_nullable(part))
continue;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 31f0515e..cee03f0a 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -1144,7 +1144,7 @@ tuple_format_iterator_next(struct tuple_format_iterator *it,
entry->multikey_idx = it->multikey_frame->idx;
} else {
entry->multikey_count = 0;
- entry->multikey_idx = -1;
+ entry->multikey_idx = MULTIKEY_NONE;
}
if (field == NULL || (it->flags & TUPLE_FORMAT_ITERATOR_VALIDATE) == 0)
return 0;
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index f90e1671..223c5f6f 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -160,7 +160,8 @@ struct TupleHash
uint32_t carry = 0;
uint32_t total_size = 0;
const char *field = tuple_field_by_part(tuple,
- key_def->parts, -1);
+ key_def->parts,
+ MULTIKEY_NONE);
TupleFieldHash<TYPE, MORE_TYPES...>::
hash(&field, &h, &carry, &total_size);
return PMurHash32_Result(h, carry, total_size);
@@ -173,7 +174,8 @@ struct TupleHash<FIELD_TYPE_UNSIGNED> {
{
assert(!key_def_is_multikey(key_def));
const char *field = tuple_field_by_part(tuple,
- key_def->parts, -1);
+ key_def->parts,
+ MULTIKEY_NONE);
uint64_t val = mp_decode_uint(&field);
if (likely(val <= UINT32_MAX))
return val;
@@ -373,7 +375,7 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def)
const char *field;
if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw, field_map,
- key_def->parts, -1);
+ key_def->parts, MULTIKEY_NONE);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
prev_fieldno);
@@ -394,7 +396,8 @@ tuple_hash_slowpath(struct tuple *tuple, struct key_def *key_def)
struct key_part *part = &key_def->parts[part_id];
if (has_json_paths) {
field = tuple_field_raw_by_part(format, tuple_raw,
- field_map, part, -1);
+ field_map, part,
+ MULTIKEY_NONE);
} else {
field = tuple_field_raw(format, tuple_raw, field_map,
part->fieldno);
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 5be10d59..d4929a37 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1339,7 +1339,8 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
struct vy_entry key;
if (vy_stmt_is_key(entry.stmt)) {
key.stmt = vy_stmt_extract_key(entry.stmt, lsm->pk_in_cmp_def,
- lsm->env->key_format, -1);
+ lsm->env->key_format,
+ MULTIKEY_NONE);
if (key.stmt == NULL)
return -1;
} else {
@@ -1644,7 +1645,8 @@ vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv,
return 0;
if (!key_def_is_multikey(lsm->cmp_def)) {
return vy_check_is_unique_secondary_one(tx, rv,
- space_name, index_name, lsm, stmt, -1);
+ space_name, index_name, lsm, stmt,
+ MULTIKEY_NONE);
}
int count = tuple_multikey_count(stmt, lsm->cmp_def);
for (int i = 0; i < count; ++i) {
@@ -2167,7 +2169,8 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
*/
/* Find the old tuple using the primary key. */
struct tuple *key = vy_stmt_extract_key_raw(tuple, tuple_end,
- pk->key_def, pk->env->key_format, -1);
+ pk->key_def, pk->env->key_format,
+ MULTIKEY_NONE);
if (key == NULL)
return -1;
int rc = vy_get(pk, tx, vy_tx_read_view(tx), key, &stmt->old_tuple);
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 0b9950f6..3b38aac6 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -2437,7 +2437,8 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
}
}
key = vy_stmt_is_key(tuple) ? tuple_data(tuple) :
- tuple_extract_key(tuple, cmp_def, -1, NULL);
+ tuple_extract_key(tuple, cmp_def,
+ MULTIKEY_NONE, NULL);
if (prev_tuple != NULL)
tuple_unref(prev_tuple);
prev_tuple = tuple;
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 8ce785a5..9e20b6bf 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -629,7 +629,8 @@ vy_stmt_encode_primary(struct tuple *value, struct key_def *key_def,
case IPROTO_DELETE:
extracted = vy_stmt_is_key(value) ?
tuple_data_range(value, &size) :
- tuple_extract_key(value, key_def, -1, &size);
+ tuple_extract_key(value, key_def,
+ MULTIKEY_NONE, &size);
if (extracted == NULL)
return -1;
request.key = extracted;
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 0026d614..a80456ad 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -691,13 +691,13 @@ vy_stmt_str(struct tuple *stmt);
/**
* Extract a multikey index hint from a statement entry.
- * Returns -1 if the key definition isn't multikey.
+ * Returns MULTIKEY_NONE if the key definition isn't multikey.
*/
static inline int
vy_entry_multikey_idx(struct vy_entry entry, struct key_def *key_def)
{
if (!key_def_is_multikey(key_def) || vy_stmt_is_key(entry.stmt))
- return -1;
+ return MULTIKEY_NONE;
assert(entry.hint != HINT_NONE);
return (int)entry.hint;
}
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/7] Support multikey indexes in Vinyl
2019-05-08 17:22 [PATCH 0/7] Support multikey indexes in Vinyl Vladimir Davydov
` (7 preceding siblings ...)
2019-05-13 16:34 ` [PATCH] Use MULTIKEY_NONE instead of -1 Vladimir Davydov
@ 2019-05-13 19:26 ` Vladimir Davydov
8 siblings, 0 replies; 11+ messages in thread
From: Vladimir Davydov @ 2019-05-13 19:26 UTC (permalink / raw)
To: tarantool-patches
Pushed all the patches in the thread to the master branch after
soliciting verbal approval from Kostja.
^ permalink raw reply [flat|nested] 11+ messages in thread