* [PATCH 01/12] vinyl: use vy_lsm_env::empty_key where appropriate
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 10:59 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 02/12] vinyl: make vy_tuple_delete static Vladimir Davydov
` (11 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
No need to allocate an empty key during DDL - we already have one
preallocated in vy_lsm_env.
---
src/box/vinyl.c | 34 ++++++++++------------------------
src/box/vy_tx.c | 10 +++-------
src/box/vy_tx.h | 4 +---
3 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 947ca355..23c5b9c3 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1038,17 +1038,15 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
* before the trigger was installed so that DDL doesn't miss
* their working set.
*/
-static int
+static void
vy_abort_writers_for_ddl(struct vy_env *env, struct vy_lsm *lsm)
{
- if (tx_manager_abort_writers(env->xm, lsm) != 0)
- return -1;
+ tx_manager_abort_writers(env->xm, lsm);
/*
* Wait for prepared transactions to complete
* (we can't abort them as they reached WAL).
*/
wal_sync();
- return 0;
}
/** Argument passed to vy_check_format_on_replace(). */
@@ -1109,10 +1107,6 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
*/
struct vy_lsm *pk = vy_lsm(space->index[0]);
- struct tuple *key = vy_stmt_new_select(pk->env->key_format, NULL, 0);
- if (key == NULL)
- return -1;
-
struct trigger on_replace;
struct vy_check_format_ctx ctx;
ctx.format = format;
@@ -1121,13 +1115,12 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
trigger_create(&on_replace, vy_check_format_on_replace, &ctx, NULL);
trigger_add(&space->on_replace, &on_replace);
- int rc = vy_abort_writers_for_ddl(env, pk);
- if (rc != 0)
- goto out;
+ vy_abort_writers_for_ddl(env, pk);
struct vy_read_iterator itr;
- vy_read_iterator_open(&itr, pk, NULL, ITER_ALL, key,
+ vy_read_iterator_open(&itr, pk, NULL, ITER_ALL, pk->env->empty_key,
&env->xm->p_committed_read_view);
+ int rc;
int loops = 0;
struct tuple *tuple;
while ((rc = vy_read_iterator_next(&itr, &tuple)) == 0) {
@@ -1151,10 +1144,9 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
break;
}
vy_read_iterator_close(&itr);
-out:
+
diag_destroy(&ctx.diag);
trigger_clear(&on_replace);
- tuple_unref(key);
return rc;
}
@@ -4183,10 +4175,6 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index,
* may yield, we install an on_replace trigger to forward
* DML requests issued during the build.
*/
- struct tuple *key = vy_stmt_new_select(pk->env->key_format, NULL, 0);
- if (key == NULL)
- return -1;
-
struct trigger on_replace;
struct vy_build_ctx ctx;
ctx.lsm = new_lsm;
@@ -4198,13 +4186,12 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index,
trigger_create(&on_replace, vy_build_on_replace, &ctx, NULL);
trigger_add(&src_space->on_replace, &on_replace);
- int rc = vy_abort_writers_for_ddl(env, pk);
- if (rc != 0)
- goto out;
+ vy_abort_writers_for_ddl(env, pk);
struct vy_read_iterator itr;
- vy_read_iterator_open(&itr, pk, NULL, ITER_ALL, key,
+ vy_read_iterator_open(&itr, pk, NULL, ITER_ALL, pk->env->empty_key,
&env->xm->p_committed_read_view);
+ int rc;
int loops = 0;
struct tuple *tuple;
int64_t build_lsn = env->xm->lsn;
@@ -4260,10 +4247,9 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index,
diag_move(&ctx.diag, diag_get());
rc = -1;
}
-out:
+
diag_destroy(&ctx.diag);
trigger_clear(&on_replace);
- tuple_unref(key);
return rc;
}
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index 53bc1d86..b1fcfabb 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -1056,21 +1056,17 @@ vy_tx_set_with_colmask(struct vy_tx *tx, struct vy_lsm *lsm,
return 0;
}
-int
+void
tx_manager_abort_writers(struct tx_manager *xm, struct vy_lsm *lsm)
{
- struct tuple *key = vy_stmt_new_select(lsm->env->key_format, NULL, 0);
- if (key == NULL)
- return -1;
struct vy_tx *tx;
rlist_foreach_entry(tx, &xm->writers, in_writers) {
assert(!vy_tx_is_ro(tx));
if (tx->state == VINYL_TX_READY &&
- write_set_search_key(&tx->write_set, lsm, key) != NULL)
+ write_set_search_key(&tx->write_set, lsm,
+ lsm->env->empty_key) != NULL)
tx->state = VINYL_TX_ABORT;
}
- tuple_unref(key);
- return 0;
}
void
diff --git a/src/box/vy_tx.h b/src/box/vy_tx.h
index 87066091..9524936f 100644
--- a/src/box/vy_tx.h
+++ b/src/box/vy_tx.h
@@ -277,10 +277,8 @@ tx_manager_mem_used(struct tx_manager *xm);
/**
* Abort all rw transactions that affect the given LSM tree
* and haven't reached WAL yet.
- *
- * Returns 0 on success, -1 on memory allocation error.
*/
-int
+void
tx_manager_abort_writers(struct tx_manager *xm, struct vy_lsm *lsm);
/** Initialize a tx object. */
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 02/12] vinyl: make vy_tuple_delete static
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
2019-02-21 10:26 ` [PATCH 01/12] vinyl: use vy_lsm_env::empty_key where appropriate Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:00 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 03/12] key_def: cleanup virtual function initialization Vladimir Davydov
` (10 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
It isn't used anywhere outside vy_stmt.c.
---
src/box/vy_stmt.c | 16 ++++++++--------
src/box/vy_stmt.h | 7 -------
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index af5c6408..618e6025 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -94,14 +94,7 @@ vy_tuple_new(struct tuple_format *format, const char *data, const char *end)
return tuple;
}
-struct tuple_format_vtab vy_tuple_format_vtab = {
- vy_tuple_delete,
- vy_tuple_new,
-};
-
-size_t vy_max_tuple_size = 1024 * 1024;
-
-void
+static void
vy_tuple_delete(struct tuple_format *format, struct tuple *tuple)
{
say_debug("%s(%p)", __func__, tuple);
@@ -119,6 +112,13 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple)
free(tuple);
}
+struct tuple_format_vtab vy_tuple_format_vtab = {
+ vy_tuple_delete,
+ vy_tuple_new,
+};
+
+size_t vy_max_tuple_size = 1024 * 1024;
+
/**
* Allocate a vinyl statement object on base of the struct tuple
* with malloc() and the reference counter equal to 1.
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index d131a461..dd1f2460 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -220,13 +220,6 @@ vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
}
/**
- * Free the tuple of a vinyl space.
- * @pre tuple->refs == 0
- */
-void
-vy_tuple_delete(struct tuple_format *format, struct tuple *tuple);
-
-/**
* Duplicate the statememnt.
*
* @param stmt statement
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 03/12] key_def: cleanup virtual function initialization
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
2019-02-21 10:26 ` [PATCH 01/12] vinyl: use vy_lsm_env::empty_key where appropriate Vladimir Davydov
2019-02-21 10:26 ` [PATCH 02/12] vinyl: make vy_tuple_delete static Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:01 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 04/12] key_def: move cmp and hash functions declarations to key_def.h Vladimir Davydov
` (9 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
- Rename key_def_set_cmp to key_def_func_set, because it sets not only
comparators these days.
- Rename tuple_extract_key_set to tuple_extract_key_func_set to match
tuple_hash_func_set.
- Introduce tuple_compare_func_set and use it instead of setting
comparators explicitly in key_def.c.
---
src/box/key_def.c | 15 +++++++--------
src/box/tuple_compare.cc | 11 +++++++++--
src/box/tuple_compare.h | 24 +++++++-----------------
| 2 +-
| 2 +-
5 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 9411ade3..75e3a0be 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -129,12 +129,11 @@ key_def_delete(struct key_def *def)
}
static void
-key_def_set_cmp(struct key_def *def)
+key_def_func_set(struct key_def *def)
{
- def->tuple_compare = tuple_compare_create(def);
- def->tuple_compare_with_key = tuple_compare_with_key_create(def);
+ tuple_compare_func_set(def);
tuple_hash_func_set(def);
- tuple_extract_key_set(def);
+ tuple_extract_key_func_set(def);
}
static void
@@ -208,7 +207,7 @@ key_def_new(const struct key_part_def *parts, uint32_t part_count)
&path_pool, TUPLE_OFFSET_SLOT_NIL, 0);
}
assert(path_pool == (char *)def + sz);
- key_def_set_cmp(def);
+ key_def_func_set(def);
return def;
}
@@ -261,7 +260,7 @@ box_key_def_new(uint32_t *fields, uint32_t *types, uint32_t part_count)
NULL, COLL_NONE, SORT_ORDER_ASC, NULL, 0,
NULL, TUPLE_OFFSET_SLOT_NIL, 0);
}
- key_def_set_cmp(key_def);
+ key_def_func_set(key_def);
return key_def;
}
@@ -333,7 +332,7 @@ key_def_update_optionality(struct key_def *def, uint32_t min_field_count)
if (def->has_optional_parts)
break;
}
- key_def_set_cmp(def);
+ key_def_func_set(def);
}
int
@@ -686,7 +685,7 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
part->offset_slot_cache, part->format_epoch);
}
assert(path_pool == (char *)new_def + sz);
- key_def_set_cmp(new_def);
+ key_def_func_set(new_def);
return new_def;
}
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index ded802c7..b8dc350f 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -1049,7 +1049,7 @@ static const tuple_compare_t compare_slowpath_funcs[] = {
tuple_compare_slowpath<true, true, true>
};
-tuple_compare_t
+static tuple_compare_t
tuple_compare_create(const struct key_def *def)
{
int cmp_func_idx = (def->is_nullable ? 1 : 0) +
@@ -1277,7 +1277,7 @@ static const tuple_compare_with_key_t compare_with_key_slowpath_funcs[] = {
tuple_compare_with_key_slowpath<true, true, true>
};
-tuple_compare_with_key_t
+static tuple_compare_with_key_t
tuple_compare_with_key_create(const struct key_def *def)
{
int cmp_func_idx = (def->is_nullable ? 1 : 0) +
@@ -1322,3 +1322,10 @@ tuple_compare_with_key_create(const struct key_def *def)
}
/* }}} tuple_compare_with_key */
+
+void
+tuple_compare_func_set(struct key_def *def)
+{
+ def->tuple_compare = tuple_compare_create(def);
+ def->tuple_compare_with_key = tuple_compare_with_key_create(def);
+}
diff --git a/src/box/tuple_compare.h b/src/box/tuple_compare.h
index e3a63204..baecbafe 100644
--- a/src/box/tuple_compare.h
+++ b/src/box/tuple_compare.h
@@ -30,17 +30,15 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-#include <stddef.h>
#include <stdint.h>
-#include <stdbool.h>
-
-#include "key_def.h"
-#include "tuple.h"
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
+struct tuple;
+struct key_def;
+
/**
* Return the length of the longest common prefix of two tuples.
* @param tuple_a first tuple
@@ -53,19 +51,11 @@ tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
struct key_def *key_def);
/**
- * Create a comparison function for the key_def
- *
- * @param key_def key_definition
- * @returns a comparision function
+ * Initialize comparator functions for the key_def.
+ * @param key_def key definition
*/
-tuple_compare_t
-tuple_compare_create(const struct key_def *key_def);
-
-/**
- * @copydoc tuple_compare_create()
- */
-tuple_compare_with_key_t
-tuple_compare_with_key_create(const struct key_def *key_def);
+void
+tuple_compare_func_set(struct key_def *def);
#if defined(__cplusplus)
} /* extern "C" */
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 5cf46724..915b6bae 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -356,7 +356,7 @@ static const tuple_extract_key_t extract_key_slowpath_funcs[] = {
* Initialize tuple_extract_key() and tuple_extract_key_raw()
*/
void
-tuple_extract_key_set(struct key_def *key_def)
+tuple_extract_key_func_set(struct key_def *key_def)
{
if (key_def_is_sequential(key_def)) {
if (key_def->has_optional_parts) {
--git a/src/box/tuple_extract_key.h b/src/box/tuple_extract_key.h
index 8a346595..129b0d82 100644
--- a/src/box/tuple_extract_key.h
+++ b/src/box/tuple_extract_key.h
@@ -41,7 +41,7 @@ struct key_def;
* @param key_def key definition
*/
void
-tuple_extract_key_set(struct key_def *key_def);
+tuple_extract_key_func_set(struct key_def *key_def);
#if defined(__cplusplus)
} /* extern "C" */
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 03/12] key_def: cleanup virtual function initialization
2019-02-21 10:26 ` [PATCH 03/12] key_def: cleanup virtual function initialization Vladimir Davydov
@ 2019-02-21 11:01 ` Konstantin Osipov
2019-02-21 12:05 ` Vladimir Davydov
0 siblings, 1 reply; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:01 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
Please use subject-verb-object, on other words, key_def_set_func,
key_def_set_extract_key_func, and so on.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [tarantool-patches] Re: [PATCH 03/12] key_def: cleanup virtual function initialization
2019-02-21 11:01 ` [tarantool-patches] " Konstantin Osipov
@ 2019-02-21 12:05 ` Vladimir Davydov
0 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 12:05 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 02:01:28PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
>
> Please use subject-verb-object, on other words, key_def_set_func,
> key_def_set_extract_key_func, and so on.
Done on the branch. The new patch is below.
From aeb32f330b42ec42a1d993d0ac586206e923eab0 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Date: Tue, 19 Feb 2019 14:09:12 +0300
Subject: [PATCH] key_def: cleanup virtual function initialization
- Rename key_def_set_cmp to key_def_set_func, because it sets not only
comparators these days.
- Rename tuple_hash_func_set and tuple_extract_key_set to
key_def_set_hash_func and key_def_set_extract_func, because it's more
like subject-verb-object naming convention used throughout the code.
- Introduce key_def_set_compare_func and use it instead of setting
comparators explicitly in key_def.c.
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 9411ade3..5a75acb3 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -129,12 +129,11 @@ key_def_delete(struct key_def *def)
}
static void
-key_def_set_cmp(struct key_def *def)
+key_def_set_func(struct key_def *def)
{
- def->tuple_compare = tuple_compare_create(def);
- def->tuple_compare_with_key = tuple_compare_with_key_create(def);
- tuple_hash_func_set(def);
- tuple_extract_key_set(def);
+ key_def_set_compare_func(def);
+ key_def_set_hash_func(def);
+ key_def_set_extract_func(def);
}
static void
@@ -208,7 +207,7 @@ key_def_new(const struct key_part_def *parts, uint32_t part_count)
&path_pool, TUPLE_OFFSET_SLOT_NIL, 0);
}
assert(path_pool == (char *)def + sz);
- key_def_set_cmp(def);
+ key_def_set_func(def);
return def;
}
@@ -261,7 +260,7 @@ box_key_def_new(uint32_t *fields, uint32_t *types, uint32_t part_count)
NULL, COLL_NONE, SORT_ORDER_ASC, NULL, 0,
NULL, TUPLE_OFFSET_SLOT_NIL, 0);
}
- key_def_set_cmp(key_def);
+ key_def_set_func(key_def);
return key_def;
}
@@ -333,7 +332,7 @@ key_def_update_optionality(struct key_def *def, uint32_t min_field_count)
if (def->has_optional_parts)
break;
}
- key_def_set_cmp(def);
+ key_def_set_func(def);
}
int
@@ -686,7 +685,7 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
part->offset_slot_cache, part->format_epoch);
}
assert(path_pool == (char *)new_def + sz);
- key_def_set_cmp(new_def);
+ key_def_set_func(new_def);
return new_def;
}
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index ded802c7..cbdd150b 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -1049,7 +1049,7 @@ static const tuple_compare_t compare_slowpath_funcs[] = {
tuple_compare_slowpath<true, true, true>
};
-tuple_compare_t
+static tuple_compare_t
tuple_compare_create(const struct key_def *def)
{
int cmp_func_idx = (def->is_nullable ? 1 : 0) +
@@ -1277,7 +1277,7 @@ static const tuple_compare_with_key_t compare_with_key_slowpath_funcs[] = {
tuple_compare_with_key_slowpath<true, true, true>
};
-tuple_compare_with_key_t
+static tuple_compare_with_key_t
tuple_compare_with_key_create(const struct key_def *def)
{
int cmp_func_idx = (def->is_nullable ? 1 : 0) +
@@ -1322,3 +1322,10 @@ tuple_compare_with_key_create(const struct key_def *def)
}
/* }}} tuple_compare_with_key */
+
+void
+key_def_set_compare_func(struct key_def *def)
+{
+ def->tuple_compare = tuple_compare_create(def);
+ def->tuple_compare_with_key = tuple_compare_with_key_create(def);
+}
diff --git a/src/box/tuple_compare.h b/src/box/tuple_compare.h
index e3a63204..f8fa6eb9 100644
--- a/src/box/tuple_compare.h
+++ b/src/box/tuple_compare.h
@@ -30,17 +30,15 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-#include <stddef.h>
#include <stdint.h>
-#include <stdbool.h>
-
-#include "key_def.h"
-#include "tuple.h"
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
+struct tuple;
+struct key_def;
+
/**
* Return the length of the longest common prefix of two tuples.
* @param tuple_a first tuple
@@ -53,19 +51,11 @@ tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
struct key_def *key_def);
/**
- * Create a comparison function for the key_def
- *
- * @param key_def key_definition
- * @returns a comparision function
+ * Initialize comparator functions for the key_def.
+ * @param key_def key definition
*/
-tuple_compare_t
-tuple_compare_create(const struct key_def *key_def);
-
-/**
- * @copydoc tuple_compare_create()
- */
-tuple_compare_with_key_t
-tuple_compare_with_key_create(const struct key_def *key_def);
+void
+key_def_set_compare_func(struct key_def *def);
#if defined(__cplusplus)
} /* extern "C" */
diff --git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 5cf46724..17f06db7 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -356,7 +356,7 @@ static const tuple_extract_key_t extract_key_slowpath_funcs[] = {
* Initialize tuple_extract_key() and tuple_extract_key_raw()
*/
void
-tuple_extract_key_set(struct key_def *key_def)
+key_def_set_extract_func(struct key_def *key_def)
{
if (key_def_is_sequential(key_def)) {
if (key_def->has_optional_parts) {
diff --git a/src/box/tuple_extract_key.h b/src/box/tuple_extract_key.h
index 8a346595..7be16241 100644
--- a/src/box/tuple_extract_key.h
+++ b/src/box/tuple_extract_key.h
@@ -41,7 +41,7 @@ struct key_def;
* @param key_def key definition
*/
void
-tuple_extract_key_set(struct key_def *key_def);
+key_def_set_extract_func(struct key_def *key_def);
#if defined(__cplusplus)
} /* extern "C" */
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index 5bfd40cf..5213ae97 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -222,7 +222,7 @@ uint32_t
key_hash_slowpath(const char *key, struct key_def *key_def);
void
-tuple_hash_func_set(struct key_def *key_def) {
+key_def_set_hash_func(struct key_def *key_def) {
if (key_def->is_nullable || key_def->has_json_paths)
goto slowpath;
/*
diff --git a/src/box/tuple_hash.h b/src/box/tuple_hash.h
index abc961bf..d87e3234 100644
--- a/src/box/tuple_hash.h
+++ b/src/box/tuple_hash.h
@@ -42,7 +42,7 @@ extern "C" {
* @param key_def key definition
*/
void
-tuple_hash_func_set(struct key_def *def);
+key_def_set_hash_func(struct key_def *def);
/**
* Compute hash of a tuple field.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 04/12] key_def: move cmp and hash functions declarations to key_def.h
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (2 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 03/12] key_def: cleanup virtual function initialization Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:02 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 05/12] vinyl: move vy_tuple_key_contains_null to generic code Vladimir Davydov
` (8 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
Most of them are already there - for instance see see tuple_extract_key
and tuple_compare. Let's move the rest there too for consistency.
---
src/box/key_def.c | 1 +
src/box/key_def.h | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
src/box/memtx_hash.c | 1 -
src/box/sql/analyze.c | 1 -
src/box/tuple_bloom.c | 1 -
src/box/tuple_compare.h | 14 -----------
src/box/tuple_hash.cc | 1 +
src/box/tuple_hash.h | 58 ++------------------------------------------
src/box/vy_run.c | 1 -
9 files changed, 68 insertions(+), 74 deletions(-)
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 75e3a0be..92b2586d 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -30,6 +30,7 @@
*/
#include "json/json.h"
#include "key_def.h"
+#include "tuple_format.h"
#include "tuple_compare.h"
#include "tuple_extract_key.h"
#include "tuple_hash.h"
diff --git a/src/box/key_def.h b/src/box/key_def.h
index bf3e63b6..788a200d 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -503,6 +503,17 @@ tuple_extract_key_raw(const char *data, const char *data_end,
}
/**
+ * Return the length of the longest common prefix of two tuples.
+ * @param tuple_a first tuple
+ * @param tuple_b second tuple
+ * @param key_def key defintion
+ * @return number of key parts the two tuples have in common
+ */
+uint32_t
+tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
+ struct key_def *key_def);
+
+/**
* 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
@@ -551,6 +562,59 @@ tuple_compare_with_key(const struct tuple *tuple, const char *key,
return key_def->tuple_compare_with_key(tuple, key, part_count, key_def);
}
+/**
+ * Compute hash of a tuple field.
+ * @param ph1 - pointer to running hash
+ * @param pcarry - pointer to carry
+ * @param field - pointer to field data
+ * @param coll - collation to use for hashing strings or NULL
+ * @return size of processed data
+ *
+ * This function updates @ph1 and @pcarry and advances @field
+ * by the number of processed bytes.
+ */
+uint32_t
+tuple_hash_field(uint32_t *ph1, uint32_t *pcarry, const char **field,
+ struct coll *coll);
+
+/**
+ * Compute hash of a key part.
+ * @param ph1 - pointer to running hash
+ * @param pcarry - pointer to carry
+ * @param tuple - tuple to hash
+ * @param part - key part
+ * @return size of processed data
+ *
+ * This function updates @ph1 and @pcarry.
+ */
+uint32_t
+tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, const struct tuple *tuple,
+ struct key_part *part);
+
+/**
+ * Calculates a common hash value for a tuple
+ * @param tuple - a tuple
+ * @param key_def - key_def for field description
+ * @return - hash value
+ */
+static inline uint32_t
+tuple_hash(const struct tuple *tuple, struct key_def *key_def)
+{
+ return key_def->tuple_hash(tuple, key_def);
+}
+
+/**
+ * Calculate a common hash value for a key
+ * @param key - full key (msgpack fields w/o array marker)
+ * @param key_def - key_def for field description
+ * @return - hash value
+ */
+static inline uint32_t
+key_hash(const char *key, struct key_def *key_def)
+{
+ return key_def->key_hash(key, key_def);
+}
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
index eae07e8a..511d0e51 100644
--- a/src/box/memtx_hash.c
+++ b/src/box/memtx_hash.c
@@ -32,7 +32,6 @@
#include "say.h"
#include "fiber.h"
#include "tuple.h"
-#include "tuple_hash.h"
#include "memtx_engine.h"
#include "space.h"
#include "schema.h" /* space_cache_find() */
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index e0f2724a..8c83288e 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -108,7 +108,6 @@
#include "box/box.h"
#include "box/index.h"
#include "box/key_def.h"
-#include "box/tuple_compare.h"
#include "box/schema.h"
#include "third_party/qsort_arg.h"
diff --git a/src/box/tuple_bloom.c b/src/box/tuple_bloom.c
index 0ea552aa..cf887c7b 100644
--- a/src/box/tuple_bloom.c
+++ b/src/box/tuple_bloom.c
@@ -41,7 +41,6 @@
#include "errcode.h"
#include "key_def.h"
#include "tuple.h"
-#include "tuple_hash.h"
#include "salad/bloom.h"
#include "trivia/util.h"
#include "third_party/PMurHash.h"
diff --git a/src/box/tuple_compare.h b/src/box/tuple_compare.h
index baecbafe..2193335f 100644
--- a/src/box/tuple_compare.h
+++ b/src/box/tuple_compare.h
@@ -30,27 +30,13 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-#include <stdint.h>
-
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
-struct tuple;
struct key_def;
/**
- * Return the length of the longest common prefix of two tuples.
- * @param tuple_a first tuple
- * @param tuple_b second tuple
- * @param key_def key defintion
- * @return number of key parts the two tuples have in common
- */
-uint32_t
-tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
- struct key_def *key_def);
-
-/**
* Initialize comparator functions for the key_def.
* @param key_def key definition
*/
diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
index 5bfd40cf..82d145ab 100644
--- a/src/box/tuple_hash.cc
+++ b/src/box/tuple_hash.cc
@@ -30,6 +30,7 @@
*/
#include "tuple_hash.h"
+#include "tuple.h"
#include "third_party/PMurHash.h"
#include "coll.h"
#include <math.h>
diff --git a/src/box/tuple_hash.h b/src/box/tuple_hash.h
index abc961bf..431613b5 100644
--- a/src/box/tuple_hash.h
+++ b/src/box/tuple_hash.h
@@ -30,13 +30,12 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
-#include "key_def.h"
-#include "tuple.h"
-
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
+struct key_def;
+
/**
* Initialize tuple_hash() and key_hash() function for the key_def
* @param key_def key definition
@@ -44,59 +43,6 @@ extern "C" {
void
tuple_hash_func_set(struct key_def *def);
-/**
- * Compute hash of a tuple field.
- * @param ph1 - pointer to running hash
- * @param pcarry - pointer to carry
- * @param field - pointer to field data
- * @param coll - collation to use for hashing strings or NULL
- * @return size of processed data
- *
- * This function updates @ph1 and @pcarry and advances @field
- * by the number of processed bytes.
- */
-uint32_t
-tuple_hash_field(uint32_t *ph1, uint32_t *pcarry, const char **field,
- struct coll *coll);
-
-/**
- * Compute hash of a key part.
- * @param ph1 - pointer to running hash
- * @param pcarry - pointer to carry
- * @param tuple - tuple to hash
- * @param part - key part
- * @return size of processed data
- *
- * This function updates @ph1 and @pcarry.
- */
-uint32_t
-tuple_hash_key_part(uint32_t *ph1, uint32_t *pcarry, const struct tuple *tuple,
- struct key_part *part);
-
-/**
- * Calculates a common hash value for a tuple
- * @param tuple - a tuple
- * @param key_def - key_def for field description
- * @return - hash value
- */
-static inline uint32_t
-tuple_hash(const struct tuple *tuple, struct key_def *key_def)
-{
- return key_def->tuple_hash(tuple, key_def);
-}
-
-/**
- * Calculate a common hash value for a key
- * @param key - full key (msgpack fields w/o array marker)
- * @param key_def - key_def for field description
- * @return - hash value
- */
-static inline uint32_t
-key_hash(const char *key, struct key_def *key_def)
-{
- return key_def->key_hash(key, key_def);
-}
-
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 7b15603e..14975bdf 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -41,7 +41,6 @@
#include "replication.h"
#include "tuple_bloom.h"
-#include "tuple_compare.h"
#include "xlog.h"
#include "xrow.h"
#include "vy_history.h"
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 05/12] vinyl: move vy_tuple_key_contains_null to generic code
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (3 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 04/12] key_def: move cmp and hash functions declarations to key_def.h Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:02 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 06/12] vinyl: move vy_key_dup " Vladimir Davydov
` (7 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
The function doesn't require any knowledge of vinyl statement layout and
can work on regular tuples. Let's rename it to tuple_key_contains_null,
move its implementation to tuple_extract_key.cc, and declare it in
key_def.h, as we do with other similar functions.
---
src/box/key_def.h | 9 +++++++++
| 16 ++++++++++++++++
src/box/vinyl.c | 2 +-
src/box/vy_stmt.h | 22 ----------------------
4 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 788a200d..dd62f6a3 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -464,6 +464,15 @@ key_part_cmp(const struct key_part *parts1, uint32_t part_count1,
const struct key_part *parts2, uint32_t part_count2);
/**
+ * Check if a key of @a tuple contains NULL.
+ * @param tuple Tuple to check.
+ * @param def Key def to check by.
+ * @retval Does the key contain NULL or not?
+ */
+bool
+tuple_key_contains_null(const struct tuple *tuple, struct key_def *def);
+
+/**
* Extract key from tuple by given key definition and return
* buffer allocated on box_txn_alloc with this key. This function
* has O(n) complexity, where n is the number of key parts.
--git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
index 915b6bae..fead2328 100644
--- a/src/box/tuple_extract_key.cc
+++ b/src/box/tuple_extract_key.cc
@@ -399,3 +399,19 @@ tuple_extract_key_func_set(struct key_def *key_def)
}
}
}
+
+bool
+tuple_key_contains_null(const struct tuple *tuple, struct key_def *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);
+ if (field == NULL || mp_typeof(*field) == MP_NIL)
+ return true;
+ }
+ return false;
+}
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 23c5b9c3..428bae1d 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1503,7 +1503,7 @@ 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 &&
- vy_tuple_key_contains_null(stmt, lsm->key_def))
+ tuple_key_contains_null(stmt, lsm->key_def))
return 0;
struct tuple *key = vy_stmt_extract_key(stmt, lsm->key_def,
lsm->env->key_format);
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index dd1f2460..3efad12c 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -669,28 +669,6 @@ vy_stmt_snprint(char *buf, int size, const struct tuple *stmt);
const char *
vy_stmt_str(const struct tuple *stmt);
-/**
- * Check if a key of @a tuple contains NULL.
- * @param tuple Tuple to check.
- * @param def Key def to check by.
- * @retval Does the key contain NULL or not?
- */
-static inline bool
-vy_tuple_key_contains_null(const struct tuple *tuple, struct key_def *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);
- if (field == NULL || mp_typeof(*field) == MP_NIL)
- return true;
- }
- return false;
-}
-
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 06/12] vinyl: move vy_key_dup to generic code
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (4 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 05/12] vinyl: move vy_tuple_key_contains_null to generic code Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:04 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Vladimir Davydov
` (6 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
This helper function has nothing to do with vinyl - it's simply
duplicates a msgpack array. Let's move it to tuple.c and rename
it to key_dup.
---
src/box/tuple.c | 15 +++++++++++++++
src/box/tuple.h | 9 +++++++++
src/box/vy_run.c | 16 ++++++++--------
src/box/vy_stmt.c | 15 ---------------
src/box/vy_stmt.h | 8 --------
5 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/src/box/tuple.c b/src/box/tuple.c
index 0770db66..6769a487 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -758,3 +758,18 @@ mp_str(const char *data)
return "<failed to format message pack>";
return buf;
}
+
+char *
+key_dup(const char *key)
+{
+ assert(mp_typeof(*key) == MP_ARRAY);
+ const char *end = key;
+ mp_next(&end);
+ char *res = malloc(end - key);
+ if (res == NULL) {
+ diag_set(OutOfMemory, end - key, "malloc", "key");
+ return NULL;
+ }
+ memcpy(res, key, end - key);
+ return res;
+}
diff --git a/src/box/tuple.h b/src/box/tuple.h
index e803260c..d8a020e0 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -1012,6 +1012,15 @@ tuple_bless(struct tuple *tuple)
ssize_t
tuple_to_buf(const struct tuple *tuple, char *buf, size_t size);
+/**
+ * Duplicate a raw key (MsgPack array).
+ * @param key the key to duplicate.
+ * @retval not NULL Pointer to the key copy allocated with malloc().
+ * @retval NULL Memory error.
+ */
+char *
+key_dup(const char *key);
+
#if defined(__cplusplus)
} /* extern "C" */
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 14975bdf..b85582b2 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -206,7 +206,7 @@ vy_page_info_create(struct vy_page_info *page_info, uint64_t offset,
memset(page_info, 0, sizeof(*page_info));
page_info->offset = offset;
page_info->unpacked_size = 0;
- page_info->min_key = vy_key_dup(min_key);
+ page_info->min_key = key_dup(min_key);
return page_info->min_key == NULL ? -1 : 0;
}
@@ -510,7 +510,7 @@ vy_page_info_decode(struct vy_page_info *page, const struct xrow_header *xrow,
case VY_PAGE_INFO_MIN_KEY:
key_beg = pos;
mp_next(&pos);
- page->min_key = vy_key_dup(key_beg);
+ page->min_key = key_dup(key_beg);
if (page->min_key == NULL)
return -1;
break;
@@ -595,14 +595,14 @@ vy_run_info_decode(struct vy_run_info *run_info,
case VY_RUN_INFO_MIN_KEY:
tmp = pos;
mp_next(&pos);
- run_info->min_key = vy_key_dup(tmp);
+ run_info->min_key = key_dup(tmp);
if (run_info->min_key == NULL)
return -1;
break;
case VY_RUN_INFO_MAX_KEY:
tmp = pos;
mp_next(&pos);
- run_info->max_key = vy_key_dup(tmp);
+ run_info->max_key = key_dup(tmp);
if (run_info->max_key == NULL)
return -1;
break;
@@ -2157,7 +2157,7 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
return -1;
if (run->info.page_count == 0) {
assert(run->info.min_key == NULL);
- run->info.min_key = vy_key_dup(key);
+ run->info.min_key = key_dup(key);
if (run->info.min_key == NULL)
return -1;
}
@@ -2311,7 +2311,7 @@ vy_run_writer_commit(struct vy_run_writer *writer)
goto out;
assert(run->info.max_key == NULL);
- run->info.max_key = vy_key_dup(key);
+ run->info.max_key = key_dup(key);
if (run->info.max_key == NULL)
goto out;
@@ -2424,7 +2424,7 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
if (key == NULL)
goto close_err;
if (run->info.min_key == NULL) {
- run->info.min_key = vy_key_dup(key);
+ run->info.min_key = key_dup(key);
if (run->info.min_key == NULL)
goto close_err;
}
@@ -2454,7 +2454,7 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
}
if (key != NULL) {
- run->info.max_key = vy_key_dup(key);
+ run->info.max_key = key_dup(key);
if (run->info.max_key == NULL)
goto close_err;
}
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 618e6025..9d9f2c75 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -252,21 +252,6 @@ vy_stmt_new_select(struct tuple_format *format, const char *key,
return stmt;
}
-char *
-vy_key_dup(const char *key)
-{
- assert(mp_typeof(*key) == MP_ARRAY);
- const char *end = key;
- mp_next(&end);
- char *res = malloc(end - key);
- if (res == NULL) {
- diag_set(OutOfMemory, end - key, "malloc", "key");
- return NULL;
- }
- memcpy(res, key, end - key);
- return res;
-}
-
/**
* Create a statement without type and with reserved space for operations.
* Operations can be saved in the space available by @param extra.
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 3efad12c..dff0b94f 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -424,14 +424,6 @@ vy_stmt_new_select(struct tuple_format *format, const char *key,
uint32_t part_count);
/**
- * Copy the key in a new memory area.
- * @retval not NULL Success.
- * @retval NULL Memory error.
- */
-char *
-vy_key_dup(const char *key);
-
-/**
* Create a new surrogate DELETE from @a key using format.
*
* Example:
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 06/12] vinyl: move vy_key_dup to generic code
2019-02-21 10:26 ` [PATCH 06/12] vinyl: move vy_key_dup " Vladimir Davydov
@ 2019-02-21 11:04 ` Konstantin Osipov
2019-02-21 11:52 ` Vladimir Davydov
0 siblings, 1 reply; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:04 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> This helper function has nothing to do with vinyl - it's simply
> duplicates a msgpack array. Let's move it to tuple.c and rename
> it to key_dup.
Why to tuple.c and not in to msgpack?
I don't like what this function is doing and how it's doing it, so
I'd rather bury it in vinyl or get rid of it, not make a generic
one.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [tarantool-patches] Re: [PATCH 06/12] vinyl: move vy_key_dup to generic code
2019-02-21 11:04 ` [tarantool-patches] " Konstantin Osipov
@ 2019-02-21 11:52 ` Vladimir Davydov
0 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 11:52 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 02:04:12PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> > This helper function has nothing to do with vinyl - it's simply
> > duplicates a msgpack array. Let's move it to tuple.c and rename
> > it to key_dup.
>
> Why to tuple.c and not in to msgpack?
Because there's no place for msgpack helpers inside tarantool. Besides,
other functions operating on raw keys (which are msgpack arrays) have
key_ prefix, e.g. key_compare.
>
> I don't like what this function is doing and how it's doing it, so
> I'd rather bury it in vinyl or get rid of it, not make a generic
> one.
OK. I just wanted to expel it from vinyl code so as to avoid confusion
between vinyl key statement and raw key msgpack.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 07/12] vinyl: sanitize full/empty key stmt detection
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (5 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 06/12] vinyl: move vy_key_dup " Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:10 ` [tarantool-patches] " Konstantin Osipov
2019-03-01 12:57 ` Vladimir Davydov
2019-02-21 10:26 ` [PATCH 08/12] vinyl: remove optimized comparators Vladimir Davydov
` (5 subsequent siblings)
12 siblings, 2 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
Historically, we use tuple_field_count to check whether a statement
represents an empty key (match all) or a full key (point lookup): if
the number of fields in a tuple is greater than or equal to the number
of parts in a key definition, it can be used as a full key; if the
number of fields is zero, then the statement represents an empty key.
While this used to be correct not so long ago, appearance of JSON
indexes changed the rules of the game: now a tuple can have nested
indexed fields so that the same field number appears in the key
definition multiple times. This means tuple_field_count can be less
than the number of key parts and hence the full key check won't work
for a statement representing a tuple.
Actually, any tuple in vinyl can be used as a full key as it has all
key parts by definition, there's no need to use tuple_field_count for
such statements - we only need to do that for statements representing
keys. Keeping that in mind, let's introduce helpers for checking
whether a statement can be used as a full/empty key and use them
throughout the code.
---
src/box/vinyl.c | 2 +-
src/box/vy_cache.c | 14 +++++++++-----
src/box/vy_mem.c | 2 +-
src/box/vy_point_lookup.c | 5 ++---
src/box/vy_range.c | 5 ++---
src/box/vy_read_iterator.c | 6 +++---
src/box/vy_read_set.c | 18 ++++++------------
src/box/vy_run.c | 2 +-
src/box/vy_stmt.h | 38 ++++++++++++++++++++++++++++++++++++++
src/box/vy_tx.c | 4 ++--
10 files changed, 65 insertions(+), 31 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 428bae1d..69880238 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1374,7 +1374,7 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx,
int rc;
struct tuple *tuple;
- if (tuple_field_count(key) >= lsm->cmp_def->part_count) {
+ if (vy_stmt_is_full_key(key, lsm->cmp_def)) {
/*
* Use point lookup for a full key.
*/
diff --git a/src/box/vy_cache.c b/src/box/vy_cache.c
index ead56e95..320ea04d 100644
--- a/src/box/vy_cache.c
+++ b/src/box/vy_cache.c
@@ -270,13 +270,15 @@ vy_cache_add(struct vy_cache *cache, struct tuple *stmt,
* Regardless of order, the statement is the first in
* sequence of statements that is equal to the key.
*/
- boundary_level = tuple_field_count(key);
+ boundary_level = vy_stmt_key_part_count(key,
+ cache->cmp_def);
}
} else {
assert(prev_stmt != NULL);
if (order == ITER_EQ || order == ITER_REQ) {
/* that is the last statement that is equal to key */
- boundary_level = tuple_field_count(key);
+ boundary_level = vy_stmt_key_part_count(key,
+ cache->cmp_def);
} else {
/* that is the last statement */
boundary_level = 0;
@@ -527,7 +529,8 @@ static inline bool
vy_cache_iterator_is_stop(struct vy_cache_iterator *itr,
struct vy_cache_entry *entry)
{
- uint8_t key_level = tuple_field_count(itr->key);
+ uint8_t key_level = vy_stmt_key_part_count(itr->key,
+ itr->cache->cmp_def);
/* select{} is actually an EQ iterator with part_count == 0 */
bool iter_is_eq = itr->iterator_type == ITER_EQ || key_level == 0;
if (iterator_direction(itr->iterator_type) > 0) {
@@ -556,7 +559,8 @@ static inline bool
vy_cache_iterator_is_end_stop(struct vy_cache_iterator *itr,
struct vy_cache_entry *last_entry)
{
- uint8_t key_level = tuple_field_count(itr->key);
+ uint8_t key_level = vy_stmt_key_part_count(itr->key,
+ itr->cache->cmp_def);
/* select{} is actually an EQ iterator with part_count == 0 */
bool iter_is_eq = itr->iterator_type == ITER_EQ || key_level == 0;
if (iterator_direction(itr->iterator_type) > 0) {
@@ -644,7 +648,7 @@ vy_cache_iterator_seek(struct vy_cache_iterator *itr,
*entry = NULL;
itr->cache->stat.lookup++;
- if (tuple_field_count(key) > 0) {
+ if (!vy_stmt_is_empty_key(key)) {
bool exact;
itr->curr_pos = iterator_type == ITER_EQ ||
iterator_type == ITER_GE ||
diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index f8b89d4e..b672cc6f 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -367,7 +367,7 @@ vy_mem_iterator_seek(struct vy_mem_iterator *itr,
tree_key.stmt = key;
/* (lsn == INT64_MAX - 1) means that lsn is ignored in comparison */
tree_key.lsn = INT64_MAX - 1;
- if (tuple_field_count(key) > 0) {
+ if (!vy_stmt_is_empty_key(key)) {
if (iterator_type == ITER_EQ) {
bool exact;
itr->curr_pos =
diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c
index 6b8bbd23..18d5622b 100644
--- a/src/box/vy_point_lookup.c
+++ b/src/box/vy_point_lookup.c
@@ -197,8 +197,7 @@ vy_point_lookup(struct vy_lsm *lsm, struct vy_tx *tx,
struct tuple *key, struct tuple **ret)
{
/* All key parts must be set for a point lookup. */
- assert(vy_stmt_type(key) != IPROTO_SELECT ||
- tuple_field_count(key) >= lsm->cmp_def->part_count);
+ assert(vy_stmt_is_full_key(key, lsm->cmp_def));
*ret = NULL;
double start_time = ev_monotonic_now(loop());
@@ -301,7 +300,7 @@ int
vy_point_lookup_mem(struct vy_lsm *lsm, const struct vy_read_view **rv,
struct tuple *key, struct tuple **ret)
{
- assert(tuple_field_count(key) >= lsm->cmp_def->part_count);
+ assert(vy_stmt_is_full_key(key, lsm->cmp_def));
int rc;
struct vy_history history;
diff --git a/src/box/vy_range.c b/src/box/vy_range.c
index d9afcfff..bbd40615 100644
--- a/src/box/vy_range.c
+++ b/src/box/vy_range.c
@@ -81,8 +81,7 @@ vy_range_tree_find_by_key(vy_range_tree_t *tree,
enum iterator_type iterator_type,
const struct tuple *key)
{
- uint32_t key_field_count = tuple_field_count(key);
- if (key_field_count == 0) {
+ if (vy_stmt_is_empty_key(key)) {
switch (iterator_type) {
case ITER_LT:
case ITER_LE:
@@ -125,7 +124,7 @@ vy_range_tree_find_by_key(vy_range_tree_t *tree,
range = vy_range_tree_psearch(tree, key);
/* switch to previous for case (4) */
if (range != NULL && range->begin != NULL &&
- key_field_count < range->cmp_def->part_count &&
+ !vy_stmt_is_full_key(key, range->cmp_def) &&
vy_stmt_compare_with_key(key, range->begin,
range->cmp_def) == 0)
range = vy_range_tree_prev(tree, range);
diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
index 2984174a..740ee940 100644
--- a/src/box/vy_read_iterator.c
+++ b/src/box/vy_read_iterator.c
@@ -208,7 +208,7 @@ vy_read_iterator_is_exact_match(struct vy_read_iterator *itr,
return itr->last_stmt == NULL && stmt != NULL &&
(type == ITER_EQ || type == ITER_REQ ||
type == ITER_GE || type == ITER_LE) &&
- tuple_field_count(key) >= cmp_def->part_count &&
+ vy_stmt_is_full_key(key, cmp_def) &&
vy_stmt_compare(stmt, key, cmp_def) == 0;
}
@@ -445,7 +445,7 @@ vy_read_iterator_advance(struct vy_read_iterator *itr)
{
if (itr->last_stmt != NULL && (itr->iterator_type == ITER_EQ ||
itr->iterator_type == ITER_REQ) &&
- tuple_field_count(itr->key) >= itr->lsm->cmp_def->part_count) {
+ vy_stmt_is_full_key(itr->key, itr->lsm->cmp_def)) {
/*
* There may be one statement at max satisfying
* EQ with a full key.
@@ -678,7 +678,7 @@ vy_read_iterator_open(struct vy_read_iterator *itr, struct vy_lsm *lsm,
itr->key = key;
itr->read_view = rv;
- if (tuple_field_count(key) == 0) {
+ if (vy_stmt_is_empty_key(key)) {
/*
* Strictly speaking, a GT/LT iterator should return
* nothing if the key is empty, because every key is
diff --git a/src/box/vy_read_set.c b/src/box/vy_read_set.c
index 0f3fab61..b95d2e4e 100644
--- a/src/box/vy_read_set.c
+++ b/src/box/vy_read_set.c
@@ -53,10 +53,8 @@ vy_read_interval_cmpl(const struct vy_read_interval *a,
return -1;
if (!a->left_belongs && b->left_belongs)
return 1;
- uint32_t a_parts = tuple_field_count(a->left);
- uint32_t b_parts = tuple_field_count(b->left);
- a_parts = MIN(a_parts, cmp_def->part_count);
- b_parts = MIN(b_parts, cmp_def->part_count);
+ uint32_t a_parts = vy_stmt_key_part_count(a->left, cmp_def);
+ uint32_t b_parts = vy_stmt_key_part_count(b->left, cmp_def);
if (a->left_belongs)
return a_parts < b_parts ? -1 : a_parts > b_parts;
else
@@ -76,10 +74,8 @@ vy_read_interval_cmpr(const struct vy_read_interval *a,
return 1;
if (!a->right_belongs && b->right_belongs)
return -1;
- uint32_t a_parts = tuple_field_count(a->right);
- uint32_t b_parts = tuple_field_count(b->right);
- a_parts = MIN(a_parts, cmp_def->part_count);
- b_parts = MIN(b_parts, cmp_def->part_count);
+ uint32_t a_parts = vy_stmt_key_part_count(a->right, cmp_def);
+ uint32_t b_parts = vy_stmt_key_part_count(b->right, cmp_def);
if (a->right_belongs)
return a_parts > b_parts ? -1 : a_parts < b_parts;
else
@@ -102,10 +98,8 @@ vy_read_interval_should_merge(const struct vy_read_interval *l,
return true;
if (!l->right_belongs && !r->left_belongs)
return false;
- uint32_t l_parts = tuple_field_count(l->right);
- uint32_t r_parts = tuple_field_count(r->left);
- l_parts = MIN(l_parts, cmp_def->part_count);
- r_parts = MIN(r_parts, cmp_def->part_count);
+ uint32_t l_parts = vy_stmt_key_part_count(l->right, cmp_def);
+ uint32_t r_parts = vy_stmt_key_part_count(r->left, cmp_def);
if (l->right_belongs)
return l_parts <= r_parts;
else
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index b85582b2..915e7f95 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1273,7 +1273,7 @@ vy_run_iterator_do_seek(struct vy_run_iterator *itr,
struct vy_run_iterator_pos end_pos = {run->info.page_count, 0};
bool equal_found = false;
int rc;
- if (tuple_field_count(key) > 0) {
+ if (!vy_stmt_is_empty_key(key)) {
rc = vy_run_iterator_search(itr, iterator_type, key,
&itr->curr_pos, &equal_found);
if (rc != 0 || itr->search_ended)
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index dff0b94f..0832aa0f 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -220,6 +220,44 @@ vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
}
/**
+ * Return the number of key parts defined in the given vinyl
+ * statement.
+ *
+ * If the statement represents a tuple, we assume that it has
+ * all key parts defined.
+ */
+static inline uint32_t
+vy_stmt_key_part_count(const struct tuple *stmt, struct key_def *key_def)
+{
+ if (vy_stmt_type(stmt) == IPROTO_SELECT) {
+ uint32_t part_count = tuple_field_count(stmt);
+ assert(part_count <= key_def->part_count);
+ return part_count;
+ }
+ return key_def->part_count;
+}
+
+/**
+ * Return true if the given vinyl statement contains all
+ * key parts, i.e. can be used for an exact match lookup.
+ */
+static inline bool
+vy_stmt_is_full_key(const struct tuple *stmt, struct key_def *key_def)
+{
+ return vy_stmt_key_part_count(stmt, key_def) == key_def->part_count;
+}
+
+/**
+ * Return true if the given vinyl statement stores an empty
+ * (match all) key.
+ */
+static inline bool
+vy_stmt_is_empty_key(const struct tuple *stmt)
+{
+ return tuple_field_count(stmt) == 0;
+}
+
+/**
* Duplicate the statememnt.
*
* @param stmt statement
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index b1fcfabb..ac02ee4d 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -943,7 +943,7 @@ vy_tx_track(struct vy_tx *tx, struct vy_lsm *lsm,
int
vy_tx_track_point(struct vy_tx *tx, struct vy_lsm *lsm, struct tuple *stmt)
{
- assert(tuple_field_count(stmt) >= lsm->cmp_def->part_count);
+ assert(vy_stmt_is_full_key(stmt, lsm->cmp_def));
if (vy_tx_is_in_read_view(tx)) {
/* No point in tracking reads. */
@@ -1102,7 +1102,7 @@ vy_txw_iterator_seek(struct vy_txw_iterator *itr,
struct vy_lsm *lsm = itr->lsm;
struct write_set_key k = { lsm, key };
struct txv *txv;
- if (tuple_field_count(key) > 0) {
+ if (!vy_stmt_is_empty_key(key)) {
if (iterator_type == ITER_EQ)
txv = write_set_search(&itr->tx->write_set, &k);
else if (iterator_type == ITER_GE || iterator_type == ITER_GT)
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection
2019-02-21 10:26 ` [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Vladimir Davydov
@ 2019-02-21 11:10 ` Konstantin Osipov
2019-02-21 12:11 ` Vladimir Davydov
2019-03-01 12:57 ` Vladimir Davydov
1 sibling, 1 reply; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:10 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> Historically, we use tuple_field_count to check whether a statement
> represents an empty key (match all) or a full key (point lookup): if
> the number of fields in a tuple is greater than or equal to the number
> of parts in a key definition, it can be used as a full key; if the
> number of fields is zero, then the statement represents an empty key.
I don't understand the purpose of this patch yet.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [tarantool-patches] Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection
2019-02-21 11:10 ` [tarantool-patches] " Konstantin Osipov
@ 2019-02-21 12:11 ` Vladimir Davydov
0 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 12:11 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 02:10:51PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> > Historically, we use tuple_field_count to check whether a statement
> > represents an empty key (match all) or a full key (point lookup): if
> > the number of fields in a tuple is greater than or equal to the number
> > of parts in a key definition, it can be used as a full key; if the
> > number of fields is zero, then the statement represents an empty key.
>
> I don't understand the purpose of this patch yet.
Well, it's just a technical debt left from the json index patch.
In fact, the code may be buggy/suboptimal without this patch. E.g.
tuple cache heavily relays on whether the passed statement is a full
or a partial key. If it fails to detect it, it won't store a chain
when it should. We used to use tuple_field_count to check whether
a tuple represents a full key or not:
if (tuple_field_count(tuple) >= cmp_def->part_count)
/* full key */
With json indexes this isn't correct anymore, because the same field may
be indexed multiple times, e.g. [2].path1 and [2].path2. Due to this,
the full key check may fail for a full tuple, which is obviously wrong
although a full tuple must have all key parts defined. This patch makes
those check work only tuples representing keys while assuming that all
tuples may be used as full keys. And it wraps it inside a pair of helper
functions.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection
2019-02-21 10:26 ` [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Vladimir Davydov
2019-02-21 11:10 ` [tarantool-patches] " Konstantin Osipov
@ 2019-03-01 12:57 ` Vladimir Davydov
1 sibling, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-03-01 12:57 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 01:26:07PM +0300, Vladimir Davydov wrote:
> Historically, we use tuple_field_count to check whether a statement
> represents an empty key (match all) or a full key (point lookup): if
> the number of fields in a tuple is greater than or equal to the number
> of parts in a key definition, it can be used as a full key; if the
> number of fields is zero, then the statement represents an empty key.
>
> While this used to be correct not so long ago, appearance of JSON
> indexes changed the rules of the game: now a tuple can have nested
> indexed fields so that the same field number appears in the key
> definition multiple times. This means tuple_field_count can be less
> than the number of key parts and hence the full key check won't work
> for a statement representing a tuple.
>
> Actually, any tuple in vinyl can be used as a full key as it has all
> key parts by definition, there's no need to use tuple_field_count for
> such statements - we only need to do that for statements representing
> keys. Keeping that in mind, let's introduce helpers for checking
> whether a statement can be used as a full/empty key and use them
> throughout the code.
> ---
> src/box/vinyl.c | 2 +-
> src/box/vy_cache.c | 14 +++++++++-----
> src/box/vy_mem.c | 2 +-
> src/box/vy_point_lookup.c | 5 ++---
> src/box/vy_range.c | 5 ++---
> src/box/vy_read_iterator.c | 6 +++---
> src/box/vy_read_set.c | 18 ++++++------------
> src/box/vy_run.c | 2 +-
> src/box/vy_stmt.h | 38 ++++++++++++++++++++++++++++++++++++++
> src/box/vy_tx.c | 4 ++--
> 10 files changed, 65 insertions(+), 31 deletions(-)
Pushed to 2.1 and 1.10.
Also, implemented a test that demonstrates the issues with JSON indexes
in vinyl and pushed it to 2.1:
From 2f14800131340621c0818fc326ea618aa4296c63 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Date: Fri, 1 Mar 2019 15:45:32 +0300
Subject: [PATCH] test: check vinyl/json corner cases
Follow-up
5993e149d90e vinyl: sanitize full/empty key stmt detection
4273ec52e122 box: introduce JSON Indexes
diff --git a/test/vinyl/json.result b/test/vinyl/json.result
new file mode 100644
index 00000000..f17619f4
--- /dev/null
+++ b/test/vinyl/json.result
@@ -0,0 +1,141 @@
+test_run = require('test_run').new()
+---
+...
+--
+-- Lookup in the primary index when applying a deferred DELETE
+-- for a secondary index on commit.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+---
+...
+sk = s:create_index('sk', {unique = false, parts = {2, 'unsigned'}})
+---
+...
+s:replace{{a = 1, b = 2, c = 3}, 10}
+---
+- [{'b': 2, 'a': 1, 'c': 3}, 10]
+...
+sk:select()
+---
+- - [{'b': 2, 'a': 1, 'c': 3}, 10]
+...
+s:drop()
+---
+...
+--
+-- Lookup on INSERT to check the unique constraint.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+---
+...
+s:replace{{a = 1, b = 2, c = 3}, 1}
+---
+- [{'b': 2, 'a': 1, 'c': 3}, 1]
+...
+box.snapshot()
+---
+- ok
+...
+s:replace{{a = 1, b = 2, c = 3}, 2}
+---
+- [{'b': 2, 'a': 1, 'c': 3}, 2]
+...
+s:insert{{a = 1, b = 2, c = 3}, 3}
+---
+- error: Duplicate key exists in unique index 'pk' in space 'test'
+...
+pk:stat().disk.iterator.lookup -- 0 (served from memory)
+---
+- 0
+...
+s:drop()
+---
+...
+--
+-- Gap locks coalescing.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+---
+...
+s:replace{{a = 1, b = 1, c = 1}}
+---
+- [{'b': 1, 'a': 1, 'c': 1}]
+...
+s:replace{{a = 1, b = 1, c = 2}}
+---
+- [{'b': 1, 'a': 1, 'c': 2}]
+...
+box.begin()
+---
+...
+gap_locks_1 = box.stat.vinyl().tx.gap_locks
+---
+...
+s:select({1, 1}, {iterator = 'ge', limit = 1})
+---
+- - [{'b': 1, 'a': 1, 'c': 1}]
+...
+s:select({1, 1}, {iterator = 'gt'})
+---
+- []
+...
+gap_locks_2 = box.stat.vinyl().tx.gap_locks
+---
+...
+gap_locks_2 - gap_locks_1 -- 2 (tracking intervals must not be coalesced)
+---
+- 2
+...
+box.commit()
+---
+...
+s:drop()
+---
+...
+--
+-- Cache iterator stop condition.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+---
+...
+s:replace{{a = 1, b = 1, c = 1}}
+---
+- [{'b': 1, 'a': 1, 'c': 1}]
+...
+s:replace{{a = 1, b = 1, c = 2}}
+---
+- [{'b': 1, 'a': 1, 'c': 2}]
+...
+s:replace{{a = 1, b = 1, c = 3}}
+---
+- [{'b': 1, 'a': 1, 'c': 3}]
+...
+s:insert{{a = 1, b = 1, c = 3}}
+---
+- error: Duplicate key exists in unique index 'pk' in space 'test'
+...
+s:select{1, 1, 1}
+---
+- - [{'b': 1, 'a': 1, 'c': 1}]
+...
+s:select{1, 1}
+---
+- - [{'b': 1, 'a': 1, 'c': 1}]
+ - [{'b': 1, 'a': 1, 'c': 2}]
+ - [{'b': 1, 'a': 1, 'c': 3}]
+...
+s:drop()
+---
+...
diff --git a/test/vinyl/json.test.lua b/test/vinyl/json.test.lua
new file mode 100644
index 00000000..2f9f9f6e
--- /dev/null
+++ b/test/vinyl/json.test.lua
@@ -0,0 +1,53 @@
+test_run = require('test_run').new()
+
+--
+-- Lookup in the primary index when applying a deferred DELETE
+-- for a secondary index on commit.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+sk = s:create_index('sk', {unique = false, parts = {2, 'unsigned'}})
+s:replace{{a = 1, b = 2, c = 3}, 10}
+sk:select()
+s:drop()
+
+--
+-- Lookup on INSERT to check the unique constraint.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+s:replace{{a = 1, b = 2, c = 3}, 1}
+box.snapshot()
+s:replace{{a = 1, b = 2, c = 3}, 2}
+s:insert{{a = 1, b = 2, c = 3}, 3}
+pk:stat().disk.iterator.lookup -- 0 (served from memory)
+s:drop()
+
+--
+-- Gap locks coalescing.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+s:replace{{a = 1, b = 1, c = 1}}
+s:replace{{a = 1, b = 1, c = 2}}
+box.begin()
+gap_locks_1 = box.stat.vinyl().tx.gap_locks
+s:select({1, 1}, {iterator = 'ge', limit = 1})
+s:select({1, 1}, {iterator = 'gt'})
+gap_locks_2 = box.stat.vinyl().tx.gap_locks
+gap_locks_2 - gap_locks_1 -- 2 (tracking intervals must not be coalesced)
+box.commit()
+s:drop()
+
+--
+-- Cache iterator stop condition.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }})
+s:replace{{a = 1, b = 1, c = 1}}
+s:replace{{a = 1, b = 1, c = 2}}
+s:replace{{a = 1, b = 1, c = 3}}
+s:insert{{a = 1, b = 1, c = 3}}
+s:select{1, 1, 1}
+s:select{1, 1}
+s:drop()
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 08/12] vinyl: remove optimized comparators
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (6 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:11 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 09/12] vinyl: introduce statement environment Vladimir Davydov
` (4 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
A vinyl statement (vy_stmt struct) may represent either a tuple or a
key. We differentiate between the two kinds by statement type - we use
SELECT for keys and other types for tuples. This was done that way so
that we could pass both tuples and keys to a read iterator as a search
key. To avoid branching in comparators when the types of compared
statements are known in advance, we provide several comparators, each of
which expects certain statement types, e.g. a tuple and a key. Actually,
such a micro optimization looks like an overkill, because a typical
comparator is called by a function pointer and has a lot of comparisons
in the code, see tuple_compare_slowpath for instance. Eliminating one
branch will hardly make the code perform better. At the same time, it
makes the code more difficult to write. Besides, once we remove nils
from statements read from disk (aka surrogate tuples), which is required
to support multikey index, the number of places where types of compared
statements are known will diminish drastically. That said, let's remove
optimized comparators and always use vy_stmt_compare, which checks types
of compared statements and calls the appropriate comparator.
---
src/box/vinyl.c | 8 +--
src/box/vy_cache.c | 8 +--
src/box/vy_cache.h | 2 +-
src/box/vy_lsm.c | 8 +--
src/box/vy_mem.c | 15 +++---
src/box/vy_mem.h | 2 +-
src/box/vy_range.c | 13 +++--
src/box/vy_read_iterator.c | 22 ++++----
src/box/vy_run.c | 36 ++++++-------
src/box/vy_stmt.h | 121 +++++++-------------------------------------
src/box/vy_tx.c | 6 +--
src/box/vy_upsert.c | 2 +-
src/box/vy_write_iterator.c | 2 +-
13 files changed, 77 insertions(+), 168 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 69880238..f2f37734 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1306,7 +1306,7 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
return -1;
if (*result == NULL ||
- vy_tuple_compare(*result, tuple, lsm->key_def) != 0) {
+ vy_stmt_compare(*result, tuple, lsm->key_def) != 0) {
/*
* If a tuple read from a secondary index doesn't
* match the tuple corresponding to it in the
@@ -1525,7 +1525,7 @@ 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_tuple_compare(stmt, found, lsm->pk->key_def) == 0) {
+ vy_stmt_compare(stmt, found, lsm->pk->key_def) == 0) {
tuple_unref(found);
return 0;
}
@@ -1730,7 +1730,7 @@ 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_tuple_compare(old_tuple, new_tuple, pk->key_def) != 0) {
+ vy_stmt_compare(old_tuple, new_tuple, pk->key_def) != 0) {
diag_set(ClientError, ER_CANT_UPDATE_PRIMARY_KEY,
index_name_by_id(space, pk->index_id),
space_name(space));
@@ -3507,7 +3507,7 @@ vy_squash_process(struct vy_squash *squash)
while (!vy_mem_tree_iterator_is_invalid(&mem_itr)) {
const struct tuple *mem_stmt;
mem_stmt = *vy_mem_tree_iterator_get_elem(&mem->tree, &mem_itr);
- if (vy_tuple_compare(result, mem_stmt, lsm->cmp_def) != 0 ||
+ if (vy_stmt_compare(result, mem_stmt, lsm->cmp_def) != 0 ||
vy_stmt_type(mem_stmt) != IPROTO_UPSERT)
break;
assert(vy_stmt_lsn(mem_stmt) >= MAX_LSN);
diff --git a/src/box/vy_cache.c b/src/box/vy_cache.c
index 320ea04d..8354b1c2 100644
--- a/src/box/vy_cache.c
+++ b/src/box/vy_cache.c
@@ -370,8 +370,8 @@ vy_cache_add(struct vy_cache *cache, struct tuple *stmt,
&inserted);
assert(*prev_check_entry != NULL);
struct tuple *prev_check_stmt = (*prev_check_entry)->stmt;
- int cmp = vy_tuple_compare(prev_stmt, prev_check_stmt,
- cache->cmp_def);
+ int cmp = vy_stmt_compare(prev_stmt, prev_check_stmt,
+ cache->cmp_def);
if (entry->flags & flag) {
/* The found entry must be exactly prev_stmt. (2) */
@@ -722,8 +722,8 @@ vy_cache_iterator_skip(struct vy_cache_iterator *itr,
if (itr->search_started &&
(itr->curr_stmt == NULL || last_stmt == NULL ||
iterator_direction(itr->iterator_type) *
- vy_tuple_compare(itr->curr_stmt, last_stmt,
- itr->cache->cmp_def) > 0))
+ vy_stmt_compare(itr->curr_stmt, last_stmt,
+ itr->cache->cmp_def) > 0))
return 0;
*stop = false;
diff --git a/src/box/vy_cache.h b/src/box/vy_cache.h
index 7cb93155..a846622e 100644
--- a/src/box/vy_cache.h
+++ b/src/box/vy_cache.h
@@ -74,7 +74,7 @@ static inline int
vy_cache_tree_cmp(struct vy_cache_entry *a,
struct vy_cache_entry *b, struct key_def *cmp_def)
{
- return vy_tuple_compare(a->stmt, b->stmt, cmp_def);
+ return vy_stmt_compare(a->stmt, b->stmt, cmp_def);
}
/**
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 1b59e436..73d369fb 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -405,7 +405,7 @@ vy_lsm_recover_slice(struct vy_lsm *lsm, struct vy_range *range,
goto out;
}
if (begin != NULL && end != NULL &&
- vy_key_compare(begin, end, lsm->cmp_def) >= 0) {
+ vy_stmt_compare(begin, end, lsm->cmp_def) >= 0) {
diag_set(ClientError, ER_INVALID_VYLOG_FILE,
tt_sprintf("begin >= end for slice %lld",
(long long)slice_info->id));
@@ -451,7 +451,7 @@ vy_lsm_recover_range(struct vy_lsm *lsm,
goto out;
}
if (begin != NULL && end != NULL &&
- vy_key_compare(begin, end, lsm->cmp_def) >= 0) {
+ vy_stmt_compare(begin, end, lsm->cmp_def) >= 0) {
diag_set(ClientError, ER_INVALID_VYLOG_FILE,
tt_sprintf("begin >= end for range %lld",
(long long)range_info->id));
@@ -632,8 +632,8 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery,
int cmp = 0;
if (prev != NULL &&
(prev->end == NULL || range->begin == NULL ||
- (cmp = vy_key_compare(prev->end, range->begin,
- lsm->cmp_def)) != 0)) {
+ (cmp = vy_stmt_compare(prev->end, range->begin,
+ lsm->cmp_def)) != 0)) {
const char *errmsg = cmp > 0 ?
"Nearby ranges %lld and %lld overlap" :
"Keys between ranges %lld and %lld not spanned";
diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c
index b672cc6f..79417671 100644
--- a/src/box/vy_mem.c
+++ b/src/box/vy_mem.c
@@ -146,7 +146,7 @@ vy_mem_older_lsn(struct vy_mem *mem, const struct tuple *stmt)
const struct tuple *result;
result = *vy_mem_tree_iterator_get_elem(&mem->tree, &itr);
- if (vy_tuple_compare(result, stmt, mem->cmp_def) != 0)
+ if (vy_stmt_compare(result, stmt, mem->cmp_def) != 0)
return NULL;
return result;
}
@@ -195,7 +195,7 @@ vy_mem_insert_upsert(struct vy_mem *mem, const struct tuple *stmt)
const struct tuple **older = vy_mem_tree_iterator_get_elem(&mem->tree,
&inserted);
if (older == NULL || vy_stmt_type(*older) != IPROTO_UPSERT ||
- vy_tuple_compare(stmt, *older, mem->cmp_def) != 0)
+ vy_stmt_compare(stmt, *older, mem->cmp_def) != 0)
return 0;
uint8_t n_upserts = vy_stmt_n_upserts(*older);
/*
@@ -335,8 +335,8 @@ vy_mem_iterator_find_lsn(struct vy_mem_iterator *itr,
&prev_pos);
if (vy_stmt_lsn(prev_stmt) > (**itr->read_view).vlsn ||
vy_stmt_flags(prev_stmt) & VY_STMT_SKIP_READ ||
- vy_tuple_compare(itr->curr_stmt, prev_stmt,
- cmp_def) != 0)
+ vy_stmt_compare(itr->curr_stmt, prev_stmt,
+ cmp_def) != 0)
break;
itr->curr_pos = prev_pos;
itr->curr_stmt = prev_stmt;
@@ -463,7 +463,7 @@ vy_mem_iterator_next_key(struct vy_mem_iterator *itr)
itr->curr_stmt = NULL;
return 1;
}
- } while (vy_tuple_compare(prev_stmt, itr->curr_stmt, cmp_def) == 0);
+ } while (vy_stmt_compare(prev_stmt, itr->curr_stmt, cmp_def) == 0);
if (itr->iterator_type == ITER_EQ &&
vy_stmt_compare(itr->key, itr->curr_stmt, cmp_def) != 0) {
@@ -497,7 +497,7 @@ next:
const struct tuple *next_stmt;
next_stmt = *vy_mem_tree_iterator_get_elem(&itr->mem->tree, &next_pos);
- if (vy_tuple_compare(itr->curr_stmt, next_stmt, cmp_def) != 0)
+ if (vy_stmt_compare(itr->curr_stmt, next_stmt, cmp_def) != 0)
return 1;
itr->curr_pos = next_pos;
@@ -551,8 +551,7 @@ vy_mem_iterator_skip(struct vy_mem_iterator *itr,
if (itr->search_started &&
(itr->curr_stmt == NULL || last_stmt == NULL ||
iterator_direction(itr->iterator_type) *
- vy_tuple_compare(itr->curr_stmt, last_stmt,
- itr->mem->cmp_def) > 0))
+ vy_stmt_compare(itr->curr_stmt, last_stmt, itr->mem->cmp_def) > 0))
return 0;
vy_history_cleanup(history);
diff --git a/src/box/vy_mem.h b/src/box/vy_mem.h
index b41c3c0b..39f238b3 100644
--- a/src/box/vy_mem.h
+++ b/src/box/vy_mem.h
@@ -90,7 +90,7 @@ static int
vy_mem_tree_cmp(const struct tuple *a, const struct tuple *b,
struct key_def *cmp_def)
{
- int res = vy_tuple_compare(a, b, cmp_def);
+ int res = vy_stmt_compare(a, b, cmp_def);
if (res)
return res;
int64_t a_lsn = vy_stmt_lsn(a), b_lsn = vy_stmt_lsn(b);
diff --git a/src/box/vy_range.c b/src/box/vy_range.c
index bbd40615..b36198f2 100644
--- a/src/box/vy_range.c
+++ b/src/box/vy_range.c
@@ -63,8 +63,8 @@ vy_range_tree_cmp(struct vy_range *range_a, struct vy_range *range_b)
return 1;
assert(range_a->cmp_def == range_b->cmp_def);
- return vy_key_compare(range_a->begin, range_b->begin,
- range_a->cmp_def);
+ return vy_stmt_compare(range_a->begin, range_b->begin,
+ range_a->cmp_def);
}
int
@@ -73,7 +73,7 @@ vy_range_tree_key_cmp(const struct tuple *stmt, struct vy_range *range)
/* Any key > -inf. */
if (range->begin == NULL)
return 1;
- return vy_stmt_compare_with_key(stmt, range->begin, range->cmp_def);
+ return vy_stmt_compare(stmt, range->begin, range->cmp_def);
}
struct vy_range *
@@ -125,8 +125,7 @@ vy_range_tree_find_by_key(vy_range_tree_t *tree,
/* switch to previous for case (4) */
if (range != NULL && range->begin != NULL &&
!vy_stmt_is_full_key(key, range->cmp_def) &&
- vy_stmt_compare_with_key(key, range->begin,
- range->cmp_def) == 0)
+ vy_stmt_compare(key, range->begin, range->cmp_def) == 0)
range = vy_range_tree_prev(tree, range);
/* for case 5 or subcase of case 4 */
if (range == NULL)
@@ -160,8 +159,8 @@ vy_range_tree_find_by_key(vy_range_tree_t *tree,
if (range != NULL) {
/* fix curr_range for cases 2 and 3 */
if (range->begin != NULL &&
- vy_stmt_compare_with_key(key, range->begin,
- range->cmp_def) != 0) {
+ vy_stmt_compare(key, range->begin,
+ range->cmp_def) != 0) {
struct vy_range *prev;
prev = vy_range_tree_prev(tree, range);
if (prev != NULL)
diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
index 740ee940..94c5f4b9 100644
--- a/src/box/vy_read_iterator.c
+++ b/src/box/vy_read_iterator.c
@@ -148,17 +148,17 @@ vy_read_iterator_range_is_done(struct vy_read_iterator *itr,
int dir = iterator_direction(itr->iterator_type);
if (dir > 0 && range->end != NULL &&
- (next_key == NULL || vy_tuple_compare_with_key(next_key,
- range->end, cmp_def) >= 0) &&
+ (next_key == NULL || vy_stmt_compare(next_key, range->end,
+ cmp_def) >= 0) &&
(itr->iterator_type != ITER_EQ ||
- vy_stmt_compare_with_key(itr->key, range->end, cmp_def) >= 0))
+ vy_stmt_compare(itr->key, range->end, cmp_def) >= 0))
return true;
if (dir < 0 && range->begin != NULL &&
- (next_key == NULL || vy_tuple_compare_with_key(next_key,
- range->begin, cmp_def) < 0) &&
+ (next_key == NULL || vy_stmt_compare(next_key, range->begin,
+ cmp_def) < 0) &&
(itr->iterator_type != ITER_REQ ||
- vy_stmt_compare_with_key(itr->key, range->begin, cmp_def) <= 0))
+ vy_stmt_compare(itr->key, range->begin, cmp_def) <= 0))
return true;
return false;
@@ -185,7 +185,7 @@ vy_read_iterator_cmp_stmt(struct vy_read_iterator *itr,
if (a == NULL && b == NULL)
return 0;
return iterator_direction(itr->iterator_type) *
- vy_tuple_compare(a, b, itr->lsm->cmp_def);
+ vy_stmt_compare(a, b, itr->lsm->cmp_def);
}
/**
@@ -763,12 +763,12 @@ vy_read_iterator_next_range(struct vy_read_iterator *itr)
* Make sure the next statement falls in the range.
*/
if (dir > 0 && (range->end == NULL ||
- vy_tuple_compare_with_key(itr->last_stmt,
- range->end, cmp_def) < 0))
+ vy_stmt_compare(itr->last_stmt, range->end,
+ cmp_def) < 0))
break;
if (dir < 0 && (range->begin == NULL ||
- vy_tuple_compare_with_key(itr->last_stmt,
- range->begin, cmp_def) > 0))
+ vy_stmt_compare(itr->last_stmt, range->begin,
+ cmp_def) > 0))
break;
}
itr->curr_range = range;
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 915e7f95..06ef299e 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -449,21 +449,21 @@ vy_slice_cut(struct vy_slice *slice, int64_t id, struct tuple *begin,
*result = NULL;
if (begin != NULL && slice->end != NULL &&
- vy_key_compare(begin, slice->end, cmp_def) >= 0)
+ vy_stmt_compare(begin, slice->end, cmp_def) >= 0)
return 0; /* no intersection: begin >= slice->end */
if (end != NULL && slice->begin != NULL &&
- vy_key_compare(end, slice->begin, cmp_def) <= 0)
+ vy_stmt_compare(end, slice->begin, cmp_def) <= 0)
return 0; /* no intersection: end <= slice->end */
/* begin = MAX(begin, slice->begin) */
if (slice->begin != NULL &&
- (begin == NULL || vy_key_compare(begin, slice->begin, cmp_def) < 0))
+ (begin == NULL || vy_stmt_compare(begin, slice->begin, cmp_def) < 0))
begin = slice->begin;
/* end = MIN(end, slice->end) */
if (slice->end != NULL &&
- (end == NULL || vy_key_compare(end, slice->end, cmp_def) > 0))
+ (end == NULL || vy_stmt_compare(end, slice->end, cmp_def) > 0))
end = slice->end;
*result = vy_slice_new(id, slice->run, begin, end, cmp_def);
@@ -1206,8 +1206,8 @@ vy_run_iterator_find_lsn(struct vy_run_iterator *itr,
return -1;
if (vy_stmt_lsn(test_stmt) > (**itr->read_view).vlsn ||
vy_stmt_flags(test_stmt) & VY_STMT_SKIP_READ ||
- vy_tuple_compare(itr->curr_stmt, test_stmt,
- cmp_def) != 0) {
+ vy_stmt_compare(itr->curr_stmt, test_stmt,
+ cmp_def) != 0) {
tuple_unref(test_stmt);
break;
}
@@ -1219,8 +1219,7 @@ vy_run_iterator_find_lsn(struct vy_run_iterator *itr,
/* Check if the result is within the slice boundaries. */
if (iterator_type == ITER_LE || iterator_type == ITER_LT) {
if (slice->begin != NULL &&
- vy_tuple_compare_with_key(itr->curr_stmt, slice->begin,
- cmp_def) < 0) {
+ vy_stmt_compare(itr->curr_stmt, slice->begin, cmp_def) < 0) {
vy_run_iterator_stop(itr);
return 0;
}
@@ -1228,8 +1227,7 @@ vy_run_iterator_find_lsn(struct vy_run_iterator *itr,
assert(iterator_type == ITER_GE || iterator_type == ITER_GT ||
iterator_type == ITER_EQ);
if (slice->end != NULL &&
- vy_tuple_compare_with_key(itr->curr_stmt, slice->end,
- cmp_def) >= 0) {
+ vy_stmt_compare(itr->curr_stmt, slice->end, cmp_def) >= 0) {
vy_run_iterator_stop(itr);
return 0;
}
@@ -1360,7 +1358,7 @@ vy_run_iterator_seek(struct vy_run_iterator *itr,
* | ge | begin | ge |
* | eq | stop |
*/
- cmp = vy_stmt_compare_with_key(key, slice->begin, cmp_def);
+ cmp = vy_stmt_compare(key, slice->begin, cmp_def);
if (cmp < 0 && iterator_type == ITER_EQ) {
vy_run_iterator_stop(itr);
return 0;
@@ -1386,7 +1384,7 @@ vy_run_iterator_seek(struct vy_run_iterator *itr,
* > end | lt | end | lt |
* | le | end | lt |
*/
- cmp = vy_stmt_compare_with_key(key, slice->end, cmp_def);
+ cmp = vy_stmt_compare(key, slice->end, cmp_def);
if (cmp > 0 || (cmp == 0 && iterator_type != ITER_LT)) {
iterator_type = ITER_LT;
key = slice->end;
@@ -1479,7 +1477,7 @@ vy_run_iterator_next_key(struct vy_run_iterator *itr, struct tuple **ret)
if (vy_run_iterator_read(itr, itr->curr_pos, &next_key) != 0)
return -1;
- } while (vy_tuple_compare(itr->curr_stmt, next_key, itr->cmp_def) == 0);
+ } while (vy_stmt_compare(itr->curr_stmt, next_key, itr->cmp_def) == 0);
tuple_unref(itr->curr_stmt);
itr->curr_stmt = next_key;
@@ -1520,7 +1518,7 @@ next:
if (vy_run_iterator_read(itr, next_pos, &next_key) != 0)
return -1;
- if (vy_tuple_compare(itr->curr_stmt, next_key, itr->cmp_def) != 0) {
+ if (vy_stmt_compare(itr->curr_stmt, next_key, itr->cmp_def) != 0) {
tuple_unref(next_key);
return 0;
}
@@ -1570,8 +1568,7 @@ vy_run_iterator_skip(struct vy_run_iterator *itr,
if (itr->search_started &&
(itr->curr_stmt == NULL || last_stmt == NULL ||
iterator_direction(itr->iterator_type) *
- vy_tuple_compare(itr->curr_stmt, last_stmt,
- itr->cmp_def) > 0))
+ vy_stmt_compare(itr->curr_stmt, last_stmt, itr->cmp_def) > 0))
return 0;
vy_history_cleanup(history);
@@ -2584,8 +2581,8 @@ vy_slice_stream_search(struct vy_stmt_stream *virt_stream)
stream->is_primary);
if (fnd_key == NULL)
return -1;
- int cmp = vy_tuple_compare_with_key(fnd_key,
- stream->slice->begin, stream->cmp_def);
+ int cmp = vy_stmt_compare(fnd_key, stream->slice->begin,
+ stream->cmp_def);
if (cmp < 0)
beg = mid + 1;
else
@@ -2636,8 +2633,7 @@ vy_slice_stream_next(struct vy_stmt_stream *virt_stream, struct tuple **ret)
/* Check that the tuple is not out of slice bounds = */
if (stream->slice->end != NULL &&
stream->page_no >= stream->slice->last_page_no &&
- vy_tuple_compare_with_key(tuple, stream->slice->end,
- stream->cmp_def) >= 0) {
+ vy_stmt_compare(tuple, stream->slice->end, stream->cmp_def) >= 0) {
tuple_unref(tuple);
return 0;
}
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 0832aa0f..ab352906 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -321,94 +321,9 @@ vy_stmt_unref_if_possible(struct tuple *stmt)
}
/**
- * Specialized comparators are faster than general-purpose comparators.
- * For example, vy_stmt_compare - slowest comparator because it in worst case
- * checks all combinations of key and tuple types, but
- * vy_key_compare - fastest comparator, because it shouldn't check statement
- * types.
+ * Compare two vinyl statements taking into account their
+ * formats (key or tuple).
*/
-
-/**
- * Compare SELECT/DELETE statements using the key definition
- * @param a left operand (SELECT/DELETE)
- * @param b right operand (SELECT/DELETE)
- * @param cmp_def key definition, with primary parts
- *
- * @retval 0 if a == b
- * @retval > 0 if a > b
- * @retval < 0 if a < b
- *
- * @sa key_compare()
- */
-static inline int
-vy_key_compare(const struct tuple *a, const struct tuple *b,
- struct key_def *cmp_def)
-{
- assert(vy_stmt_type(a) == IPROTO_SELECT);
- assert(vy_stmt_type(b) == IPROTO_SELECT);
- return key_compare(tuple_data(a), tuple_data(b), cmp_def);
-}
-
-/**
- * Compare REPLACE/UPSERTS statements.
- * @param a left operand (REPLACE/UPSERT)
- * @param b right operand (REPLACE/UPSERT)
- * @param cmp_def key definition with primary parts
- *
- * @retval 0 if a == b
- * @retval > 0 if a > b
- * @retval < 0 if a < b
- *
- * @sa tuple_compare()
- */
-static inline int
-vy_tuple_compare(const struct tuple *a, const struct tuple *b,
- struct key_def *cmp_def)
-{
- enum iproto_type type;
- type = vy_stmt_type(a);
- assert(type == IPROTO_INSERT || type == IPROTO_REPLACE ||
- type == IPROTO_UPSERT || type == IPROTO_DELETE);
- type = vy_stmt_type(b);
- assert(type == IPROTO_INSERT || type == IPROTO_REPLACE ||
- type == IPROTO_UPSERT || type == IPROTO_DELETE);
- (void)type;
- return tuple_compare(a, b, cmp_def);
-}
-
-/**
- * Compare REPLACE/UPSERT with SELECT/DELETE using the key
- * definition
- * @param tuple Left operand (REPLACE/UPSERT)
- * @param key MessagePack array of key fields, right operand.
- *
- * @retval > 0 tuple > key.
- * @retval == 0 tuple == key in all fields
- * @retval == 0 tuple is prefix of key
- * @retval == 0 key is a prefix of tuple
- * @retval < 0 tuple < key.
- *
- * @sa tuple_compare_with_key()
- */
-static inline int
-vy_tuple_compare_with_raw_key(const struct tuple *tuple, const char *key,
- struct key_def *key_def)
-{
- uint32_t part_count = mp_decode_array(&key);
- return tuple_compare_with_key(tuple, key, part_count, key_def);
-}
-
-/** @sa vy_tuple_compare_with_raw_key(). */
-static inline int
-vy_tuple_compare_with_key(const struct tuple *tuple, const struct tuple *key,
- struct key_def *key_def)
-{
- const char *key_mp = tuple_data(key);
- uint32_t part_count = mp_decode_array(&key_mp);
- return tuple_compare_with_key(tuple, key_mp, part_count, key_def);
-}
-
-/** @sa tuple_compare. */
static inline int
vy_stmt_compare(const struct tuple *a, const struct tuple *b,
struct key_def *key_def)
@@ -416,36 +331,36 @@ vy_stmt_compare(const struct tuple *a, const struct tuple *b,
bool a_is_tuple = vy_stmt_type(a) != IPROTO_SELECT;
bool b_is_tuple = vy_stmt_type(b) != IPROTO_SELECT;
if (a_is_tuple && b_is_tuple) {
- return vy_tuple_compare(a, b, key_def);
+ return tuple_compare(a, b, key_def);
} else if (a_is_tuple && !b_is_tuple) {
- return vy_tuple_compare_with_key(a, b, key_def);
+ 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) {
- return -vy_tuple_compare_with_key(b, a, key_def);
+ 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 vy_key_compare(a, b, key_def);
+ return key_compare(tuple_data(a), tuple_data(b), key_def);
}
}
-/** @sa tuple_compare_with_raw_key. */
+/**
+ * Compare a vinyl statement (key or tuple) with a raw key
+ * (msgpack array).
+ */
static inline int
vy_stmt_compare_with_raw_key(const struct tuple *stmt, const char *key,
struct key_def *key_def)
{
- if (vy_stmt_type(stmt) != IPROTO_SELECT)
- return vy_tuple_compare_with_raw_key(stmt, key, key_def);
+ if (vy_stmt_type(stmt) != IPROTO_SELECT) {
+ 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);
}
-/** @sa tuple_compare_with_key. */
-static inline int
-vy_stmt_compare_with_key(const struct tuple *stmt, const struct tuple *key,
- struct key_def *key_def)
-{
- assert(vy_stmt_type(key) == IPROTO_SELECT);
- return vy_stmt_compare_with_raw_key(stmt, tuple_data(key), key_def);
-}
-
/**
* Create the SELECT statement from raw MessagePack data.
* @param format Format of an index.
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index ac02ee4d..b9a837a3 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -67,7 +67,7 @@ write_set_cmp(struct txv *a, struct txv *b)
{
int rc = a->lsm < b->lsm ? -1 : a->lsm > b->lsm;
if (rc == 0)
- return vy_tuple_compare(a->stmt, b->stmt, a->lsm->cmp_def);
+ return vy_stmt_compare(a->stmt, b->stmt, a->lsm->cmp_def);
return rc;
}
@@ -1189,8 +1189,8 @@ vy_txw_iterator_skip(struct vy_txw_iterator *itr,
if (itr->search_started &&
(itr->curr_txv == NULL || last_stmt == NULL ||
iterator_direction(itr->iterator_type) *
- vy_tuple_compare(itr->curr_txv->stmt, last_stmt,
- itr->lsm->cmp_def) > 0))
+ vy_stmt_compare(itr->curr_txv->stmt, last_stmt,
+ itr->lsm->cmp_def) > 0))
return 0;
vy_history_cleanup(history);
diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c
index ebea2789..3c5a4abb 100644
--- a/src/box/vy_upsert.c
+++ b/src/box/vy_upsert.c
@@ -205,7 +205,7 @@ 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_tuple_compare(old_stmt, result_stmt, cmp_def) != 0) {
+ vy_stmt_compare(old_stmt, result_stmt, cmp_def) != 0) {
/*
* Key has been changed: ignore this UPSERT and
* @retval the old stmt.
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index 7a6a2062..dacd2e72 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -227,7 +227,7 @@ heap_less(heap_t *heap, struct heap_node *node1, struct heap_node *node2)
struct vy_write_src *src2 =
container_of(node2, struct vy_write_src, heap_node);
- int cmp = vy_tuple_compare(src1->tuple, src2->tuple, stream->cmp_def);
+ int cmp = vy_stmt_compare(src1->tuple, src2->tuple, stream->cmp_def);
if (cmp != 0)
return cmp < 0;
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 08/12] vinyl: remove optimized comparators
2019-02-21 10:26 ` [PATCH 08/12] vinyl: remove optimized comparators Vladimir Davydov
@ 2019-02-21 11:11 ` Konstantin Osipov
0 siblings, 0 replies; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:11 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> A vinyl statement (vy_stmt struct) may represent either a tuple or a
> key. We differentiate between the two kinds by statement type - we use
> SELECT for keys and other types for tuples. This was done that way so
> that we could pass both tuples and keys to a read iterator as a search
> key. To avoid branching in comparators when the types of compared
> statements are known in advance, we provide several comparators, each of
> which expects certain statement types, e.g. a tuple and a key. Actually,
> such a micro optimization looks like an overkill, because a typical
> comparator is called by a function pointer and has a lot of comparisons
> in the code, see tuple_compare_slowpath for instance. Eliminating one
> branch will hardly make the code perform better. At the same time, it
> makes the code more difficult to write. Besides, once we remove nils
> from statements read from disk (aka surrogate tuples), which is required
> to support multikey index, the number of places where types of compared
> statements are known will diminish drastically. That said, let's remove
> optimized comparators and always use vy_stmt_compare, which checks types
> of compared statements and calls the appropriate comparator.
This is OK to push, but please don't push it until it's really
needed (e.g. until you get OK to push on a patch which uses it).
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 09/12] vinyl: introduce statement environment
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (7 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 08/12] vinyl: remove optimized comparators Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:14 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 10/12] vinyl: rename key stmt construction routine Vladimir Davydov
` (3 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
Store tuple_format_vtab, max_tuple_size, and key_format there.
This will allow us to determine a statement type (key or tuple)
by checking its format against key_format.
---
src/box/vinyl.c | 29 ++++++++++++++++-------------
src/box/vy_lsm.c | 28 +++++++++++-----------------
src/box/vy_lsm.h | 9 +++++----
src/box/vy_stmt.c | 35 +++++++++++++++++++++++++++++------
src/box/vy_stmt.h | 34 +++++++++++++++++++++++++++-------
test/unit/vy_iterators_helper.c | 21 +++++++--------------
test/unit/vy_iterators_helper.h | 3 +--
test/unit/vy_mem.c | 6 ++----
test/unit/vy_point_lookup.c | 11 +++++------
9 files changed, 103 insertions(+), 73 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index f2f37734..c469077e 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -105,6 +105,8 @@ struct vy_env {
struct mempool iterator_pool;
/** Memory quota */
struct vy_quota quota;
+ /** Statement environment. */
+ struct vy_stmt_env stmt_env;
/** Common LSM tree environment. */
struct vy_lsm_env lsm_env;
/** Environment for cache subsystem */
@@ -602,6 +604,7 @@ static struct space *
vinyl_engine_create_space(struct engine *engine, struct space_def *def,
struct rlist *key_list)
{
+ struct vy_env *env = vy_env(engine);
struct space *space = malloc(sizeof(*space));
if (space == NULL) {
diag_set(OutOfMemory, sizeof(*space),
@@ -624,11 +627,10 @@ vinyl_engine_create_space(struct engine *engine, struct space_def *def,
rlist_foreach_entry(index_def, key_list, link)
keys[key_count++] = index_def->key_def;
- struct tuple_format *format =
- tuple_format_new(&vy_tuple_format_vtab, NULL, keys, key_count,
- def->fields, def->field_count,
- def->exact_field_count, def->dict, false,
- false);
+ struct tuple_format *format;
+ format = vy_stmt_format_new(&env->stmt_env, keys, key_count,
+ def->fields, def->field_count,
+ def->exact_field_count, def->dict);
if (format == NULL) {
free(space);
return NULL;
@@ -718,9 +720,9 @@ vinyl_space_create_index(struct space *space, struct index_def *index_def)
pk = vy_lsm(space_index(space, 0));
assert(pk != NULL);
}
- struct vy_lsm *lsm = vy_lsm_new(&env->lsm_env, &env->cache_env,
- &env->mem_env, index_def,
- space->format, pk,
+ struct vy_lsm *lsm = vy_lsm_new(&env->lsm_env, &env->stmt_env,
+ &env->cache_env, &env->mem_env,
+ index_def, space->format, pk,
space_group_id(space));
if (lsm == NULL) {
free(index);
@@ -2515,6 +2517,7 @@ vy_env_new(const char *path, size_t memory,
if (e->squash_queue == NULL)
goto error_squash_queue;
+ vy_stmt_env_create(&e->stmt_env);
vy_mem_env_create(&e->mem_env, memory);
vy_scheduler_create(&e->scheduler, write_threads,
vy_env_dump_complete_cb,
@@ -2522,6 +2525,7 @@ vy_env_new(const char *path, size_t memory,
if (vy_lsm_env_create(&e->lsm_env, e->path,
&e->scheduler.generation,
+ e->stmt_env.key_format,
vy_squash_schedule, e) != 0)
goto error_lsm_env;
@@ -2563,6 +2567,7 @@ vy_env_delete(struct vy_env *e)
vy_lsm_env_destroy(&e->lsm_env);
vy_mem_env_destroy(&e->mem_env);
vy_cache_env_destroy(&e->cache_env);
+ vy_stmt_env_destroy(&e->stmt_env);
vy_quota_destroy(&e->quota);
if (e->recovery != NULL)
vy_recovery_delete(e->recovery);
@@ -2635,8 +2640,7 @@ vinyl_engine_set_memory(struct vinyl_engine *vinyl, size_t size)
void
vinyl_engine_set_max_tuple_size(struct vinyl_engine *vinyl, size_t max_size)
{
- (void)vinyl;
- vy_max_tuple_size = max_size;
+ vinyl->env->stmt_env.max_tuple_size = max_size;
}
void
@@ -3051,9 +3055,8 @@ vy_send_lsm(struct vy_join_ctx *ctx, struct vy_lsm_recovery_info *lsm_info)
lsm_info->key_part_count);
if (ctx->key_def == NULL)
goto out;
- ctx->format = tuple_format_new(&vy_tuple_format_vtab, NULL,
- &ctx->key_def, 1, NULL, 0, 0, NULL,
- false, false);
+ ctx->format = vy_stmt_format_new(&ctx->env->stmt_env, &ctx->key_def, 1,
+ NULL, 0, 0, NULL);
if (ctx->format == NULL)
goto out_free_key_def;
tuple_format_ref(ctx->format);
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 73d369fb..eca8a334 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -70,23 +70,17 @@ static const int64_t VY_MAX_RANGE_SIZE = 2LL * 1024 * 1024 * 1024;
int
vy_lsm_env_create(struct vy_lsm_env *env, const char *path,
- int64_t *p_generation,
+ int64_t *p_generation, struct tuple_format *key_format,
vy_upsert_thresh_cb upsert_thresh_cb,
void *upsert_thresh_arg)
{
- env->key_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
- NULL, 0, NULL, 0, 0, NULL, false,
- false);
- if (env->key_format == NULL)
+ env->empty_key = vy_stmt_new_select(key_format, NULL, 0);
+ if (env->empty_key == NULL)
return -1;
- tuple_format_ref(env->key_format);
- env->empty_key = vy_stmt_new_select(env->key_format, NULL, 0);
- if (env->empty_key == NULL) {
- tuple_format_unref(env->key_format);
- return -1;
- }
env->path = path;
env->p_generation = p_generation;
+ env->key_format = key_format;
+ tuple_format_ref(key_format);
env->upsert_thresh_cb = upsert_thresh_cb;
env->upsert_thresh_arg = upsert_thresh_arg;
env->too_long_threshold = TIMEOUT_INFINITY;
@@ -124,9 +118,10 @@ vy_lsm_mem_tree_size(struct vy_lsm *lsm)
}
struct vy_lsm *
-vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
- struct vy_mem_env *mem_env, struct index_def *index_def,
- struct tuple_format *format, struct vy_lsm *pk, uint32_t group_id)
+vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
+ struct vy_cache_env *cache_env, struct vy_mem_env *mem_env,
+ struct index_def *index_def, struct tuple_format *format,
+ struct vy_lsm *pk, uint32_t group_id)
{
static int64_t run_buckets[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 15, 20, 25, 50, 100,
@@ -161,9 +156,8 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
*/
lsm->disk_format = format;
} else {
- lsm->disk_format = tuple_format_new(&vy_tuple_format_vtab, NULL,
- &cmp_def, 1, NULL, 0, 0,
- NULL, false, false);
+ lsm->disk_format = vy_stmt_format_new(stmt_env, &cmp_def, 1,
+ NULL, 0, 0, NULL);
if (lsm->disk_format == NULL)
goto fail_format;
}
diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h
index 1bb447fb..b94e7a43 100644
--- a/src/box/vy_lsm.h
+++ b/src/box/vy_lsm.h
@@ -130,7 +130,7 @@ struct vy_lsm_env {
/** Create a common LSM tree environment. */
int
vy_lsm_env_create(struct vy_lsm_env *env, const char *path,
- int64_t *p_generation,
+ int64_t *p_generation, struct tuple_format *key_format,
vy_upsert_thresh_cb upsert_thresh_cb,
void *upsert_thresh_arg);
@@ -333,9 +333,10 @@ vy_lsm_mem_tree_size(struct vy_lsm *lsm);
/** Allocate a new LSM tree object. */
struct vy_lsm *
-vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
- struct vy_mem_env *mem_env, struct index_def *index_def,
- struct tuple_format *format, struct vy_lsm *pk, uint32_t group_id);
+vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
+ struct vy_cache_env *cache_env, struct vy_mem_env *mem_env,
+ struct index_def *index_def, struct tuple_format *format,
+ struct vy_lsm *pk, uint32_t group_id);
/** Free an LSM tree object. */
void
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 9d9f2c75..57abad23 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -112,12 +112,34 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple)
free(tuple);
}
-struct tuple_format_vtab vy_tuple_format_vtab = {
- vy_tuple_delete,
- vy_tuple_new,
-};
+void
+vy_stmt_env_create(struct vy_stmt_env *env)
+{
+ env->tuple_format_vtab.tuple_new = vy_tuple_new;
+ env->tuple_format_vtab.tuple_delete = vy_tuple_delete;
+ env->max_tuple_size = 1024 * 1024;
+ env->key_format = vy_stmt_format_new(env, NULL, 0, NULL, 0, 0, NULL);
+ if (env->key_format == NULL)
+ panic("failed to create vinyl key format");
+ tuple_format_ref(env->key_format);
+}
-size_t vy_max_tuple_size = 1024 * 1024;
+void
+vy_stmt_env_destroy(struct vy_stmt_env *env)
+{
+ tuple_format_unref(env->key_format);
+}
+
+struct tuple_format *
+vy_stmt_format_new(struct vy_stmt_env *env, struct key_def *const *keys,
+ uint16_t key_count, const struct field_def *fields,
+ uint32_t field_count, uint32_t exact_field_count,
+ struct tuple_dictionary *dict)
+{
+ return tuple_format_new(&env->tuple_format_vtab, env, keys, key_count,
+ fields, field_count, exact_field_count, dict,
+ false, false);
+}
/**
* Allocate a vinyl statement object on base of the struct tuple
@@ -132,9 +154,10 @@ size_t vy_max_tuple_size = 1024 * 1024;
static struct tuple *
vy_stmt_alloc(struct tuple_format *format, uint32_t bsize)
{
+ struct vy_stmt_env *env = format->engine;
uint32_t total_size = sizeof(struct vy_stmt) + format->field_map_size +
bsize;
- if (unlikely(total_size > vy_max_tuple_size)) {
+ if (unlikely(total_size > env->max_tuple_size)) {
diag_set(ClientError, ER_VINYL_MAX_TUPLE_SIZE,
(unsigned) total_size);
error_log(diag_last_error(diag_get()));
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index ab352906..3881a2f4 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -49,6 +49,7 @@ extern "C" {
struct xrow_header;
struct region;
struct tuple_format;
+struct tuple_dictionary;
struct iovec;
#define MAX_LSN (INT64_MAX / 2)
@@ -61,14 +62,33 @@ static_assert(VY_UPSERT_THRESHOLD <= UINT8_MAX, "n_upserts max value");
static_assert(VY_UPSERT_INF == VY_UPSERT_THRESHOLD + 1,
"inf must be threshold + 1");
-/** Vinyl statement vtable. */
-extern struct tuple_format_vtab vy_tuple_format_vtab;
+/** Vinyl statement environment. */
+struct vy_stmt_env {
+ /** Vinyl statement vtable. */
+ struct tuple_format_vtab tuple_format_vtab;
+ /**
+ * Max tuple size
+ * @see box.cfg.vinyl_max_tuple_size
+ */
+ size_t max_tuple_size;
+ /** Tuple format for keys. */
+ struct tuple_format *key_format;
+};
-/**
- * Max tuple size
- * @see box.cfg.vinyl_max_tuple_size
- */
-extern size_t vy_max_tuple_size;
+/** Initialize a vinyl statement environment. */
+void
+vy_stmt_env_create(struct vy_stmt_env *env);
+
+/** Destroy a vinyl statement environment. */
+void
+vy_stmt_env_destroy(struct vy_stmt_env *env);
+
+/** Create a vinyl statement format. */
+struct tuple_format *
+vy_stmt_format_new(struct vy_stmt_env *env, struct key_def *const *keys,
+ uint16_t key_count, const struct field_def *fields,
+ uint32_t field_count, uint32_t exact_field_count,
+ struct tuple_dictionary *dict);
/** Statement flags. */
enum {
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 55d8504b..bcb60001 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -6,7 +6,7 @@
struct tt_uuid INSTANCE_UUID;
-struct tuple_format *vy_key_format = NULL;
+struct vy_stmt_env stmt_env;
struct vy_mem_env mem_env;
struct vy_cache_env cache_env;
@@ -19,12 +19,9 @@ vy_iterator_C_test_init(size_t cache_size)
memory_init();
fiber_init(fiber_c_invoke);
tuple_init(NULL);
+ vy_stmt_env_create(&stmt_env);
vy_cache_env_create(&cache_env, cord_slab_cache());
vy_cache_env_set_quota(&cache_env, cache_size);
- vy_key_format = tuple_format_new(&vy_tuple_format_vtab, NULL, NULL, 0,
- NULL, 0, 0, NULL, false, false);
- tuple_format_ref(vy_key_format);
-
size_t mem_size = 64 * 1024 * 1024;
vy_mem_env_create(&mem_env, mem_size);
}
@@ -33,8 +30,8 @@ void
vy_iterator_C_test_finish()
{
vy_mem_env_destroy(&mem_env);
- tuple_format_unref(vy_key_format);
vy_cache_env_destroy(&cache_env);
+ vy_stmt_env_destroy(&stmt_env);
tuple_free();
fiber_free();
memory_free();
@@ -122,7 +119,7 @@ vy_new_simple_stmt(struct tuple_format *format,
case IPROTO_SELECT: {
const char *key = buf;
uint part_count = mp_decode_array(&key);
- ret = vy_stmt_new_select(vy_key_format, key, part_count);
+ ret = vy_stmt_new_select(stmt_env.key_format, key, part_count);
fail_if(ret == NULL);
break;
}
@@ -199,11 +196,8 @@ struct vy_mem *
create_test_mem(struct key_def *def)
{
/* Create format */
- struct key_def * const defs[] = { def };
- struct tuple_format *format =
- tuple_format_new(&vy_tuple_format_vtab, NULL, defs,
- def->part_count, NULL, 0, 0, NULL, false,
- false);
+ struct tuple_format *format;
+ format = vy_stmt_format_new(&stmt_env, &def, 1, NULL, 0, 0, NULL);
fail_if(format == NULL);
/* Create mem */
@@ -220,8 +214,7 @@ create_test_cache(uint32_t *fields, uint32_t *types,
*def = box_key_def_new(fields, types, key_cnt);
assert(*def != NULL);
vy_cache_create(cache, &cache_env, *def, true);
- *format = tuple_format_new(&vy_tuple_format_vtab, NULL, def, 1, NULL, 0,
- 0, NULL, false, false);
+ *format = vy_stmt_format_new(&stmt_env, def, 1, NULL, 0, 0, NULL);
tuple_format_ref(*format);
}
diff --git a/test/unit/vy_iterators_helper.h b/test/unit/vy_iterators_helper.h
index 9690f684..49b4f4fd 100644
--- a/test/unit/vy_iterators_helper.h
+++ b/test/unit/vy_iterators_helper.h
@@ -51,8 +51,7 @@
#define STMT_TEMPLATE_DEFERRED_DELETE(lsn, type, ...) \
STMT_TEMPLATE_FLAGS(lsn, type, VY_STMT_DEFERRED_DELETE, __VA_ARGS__)
-extern struct tuple_format_vtab vy_tuple_format_vtab;
-extern struct tuple_format *vy_key_format;
+extern struct vy_stmt_env stmt_env;
extern struct vy_mem_env mem_env;
extern struct vy_cache_env cache_env;
diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c
index b8272eac..0226a62a 100644
--- a/test/unit/vy_mem.c
+++ b/test/unit/vy_mem.c
@@ -76,10 +76,8 @@ test_iterator_restore_after_insertion()
assert(key_def != NULL);
/* Create format */
- struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
- NULL, &key_def, 1, NULL,
- 0, 0, NULL, false,
- false);
+ struct tuple_format *format = vy_stmt_format_new(&stmt_env, &key_def, 1,
+ NULL, 0, 0, NULL);
assert(format != NULL);
tuple_format_ref(format);
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index 8ed16f6c..5f20d09b 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -66,7 +66,8 @@ test_basic()
int rc;
struct vy_lsm_env lsm_env;
- rc = vy_lsm_env_create(&lsm_env, ".", &generation, NULL, NULL);
+ rc = vy_lsm_env_create(&lsm_env, ".", &generation,
+ stmt_env.key_format, NULL, NULL);
is(rc, 0, "vy_lsm_env_create");
struct vy_run_env run_env;
@@ -83,10 +84,8 @@ test_basic()
isnt(key_def, NULL, "key_def is not NULL");
vy_cache_create(&cache, &cache_env, key_def, true);
- struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
- NULL, &key_def, 1, NULL,
- 0, 0, NULL, false,
- false);
+ struct tuple_format *format = vy_stmt_format_new(&stmt_env, &key_def, 1,
+ NULL, 0, 0, NULL);
isnt(format, NULL, "tuple_format_new is not NULL");
tuple_format_ref(format);
@@ -95,7 +94,7 @@ test_basic()
index_def_new(512, 0, "primary", sizeof("primary") - 1, TREE,
&index_opts, key_def, NULL);
- struct vy_lsm *pk = vy_lsm_new(&lsm_env, &cache_env, &mem_env,
+ struct vy_lsm *pk = vy_lsm_new(&lsm_env, &stmt_env, &cache_env, &mem_env,
index_def, format, NULL, 0);
isnt(pk, NULL, "lsm is not NULL")
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 10/12] vinyl: rename key stmt construction routine
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (8 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 09/12] vinyl: introduce statement environment Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:15 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements Vladimir Davydov
` (2 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
Currently, it's called vy_stmt_new_select, but soon a key statement will
be allowed to have any type, not just IPROTO_SELECT. So let's rename it
to vy_stmt_new_key. Let's also rename the wrapper vy_key_from_msgpack to
vy_stmt_new_key_from_array, because its key feature is that in contrast
to vy_stmt_new_key it takes a msgpack array instead of key data + part
count.
---
src/box/vinyl.c | 14 +++++++-------
src/box/vy_lsm.c | 22 +++++++++++-----------
src/box/vy_scheduler.c | 4 ++--
src/box/vy_stmt.c | 19 ++++---------------
src/box/vy_stmt.h | 19 +++++++------------
test/unit/vy_iterators_helper.c | 4 +---
test/unit/vy_mem.c | 2 +-
7 files changed, 33 insertions(+), 51 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index c469077e..925784b0 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1436,8 +1436,8 @@ vy_get_by_raw_key(struct vy_lsm *lsm, struct vy_tx *tx,
const char *key_raw, uint32_t part_count,
struct tuple **result)
{
- struct tuple *key = vy_stmt_new_select(lsm->env->key_format,
- key_raw, part_count);
+ struct tuple *key = vy_stmt_new_key(lsm->env->key_format,
+ key_raw, part_count);
if (key == NULL)
return -1;
int rc = vy_get(lsm, tx, rv, key, result);
@@ -2914,14 +2914,14 @@ vy_prepare_send_slice(struct vy_join_ctx *ctx,
goto out;
if (slice_info->begin != NULL) {
- begin = vy_key_from_msgpack(ctx->env->lsm_env.key_format,
- slice_info->begin);
+ begin = vy_stmt_new_key_from_array(ctx->env->lsm_env.key_format,
+ slice_info->begin);
if (begin == NULL)
goto out;
}
if (slice_info->end != NULL) {
- end = vy_key_from_msgpack(ctx->env->lsm_env.key_format,
- slice_info->end);
+ end = vy_stmt_new_key_from_array(ctx->env->lsm_env.key_format,
+ slice_info->end);
if (end == NULL)
goto out;
}
@@ -3793,7 +3793,7 @@ vinyl_index_create_iterator(struct index *base, enum iterator_type type,
"mempool", "struct vinyl_iterator");
return NULL;
}
- it->key = vy_stmt_new_select(lsm->env->key_format, key, part_count);
+ it->key = vy_stmt_new_key(lsm->env->key_format, key, part_count);
if (it->key == NULL) {
mempool_free(&env->iterator_pool, it);
return NULL;
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index eca8a334..2bb56d87 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -74,7 +74,7 @@ vy_lsm_env_create(struct vy_lsm_env *env, const char *path,
vy_upsert_thresh_cb upsert_thresh_cb,
void *upsert_thresh_arg)
{
- env->empty_key = vy_stmt_new_select(key_format, NULL, 0);
+ env->empty_key = vy_stmt_new_key(key_format, NULL, 0);
if (env->empty_key == NULL)
return -1;
env->path = path;
@@ -387,14 +387,14 @@ vy_lsm_recover_slice(struct vy_lsm *lsm, struct vy_range *range,
struct vy_run *run;
if (slice_info->begin != NULL) {
- begin = vy_key_from_msgpack(lsm->env->key_format,
- slice_info->begin);
+ begin = vy_stmt_new_key_from_array(lsm->env->key_format,
+ slice_info->begin);
if (begin == NULL)
goto out;
}
if (slice_info->end != NULL) {
- end = vy_key_from_msgpack(lsm->env->key_format,
- slice_info->end);
+ end = vy_stmt_new_key_from_array(lsm->env->key_format,
+ slice_info->end);
if (end == NULL)
goto out;
}
@@ -433,14 +433,14 @@ vy_lsm_recover_range(struct vy_lsm *lsm,
struct vy_range *range = NULL;
if (range_info->begin != NULL) {
- begin = vy_key_from_msgpack(lsm->env->key_format,
- range_info->begin);
+ begin = vy_stmt_new_key_from_array(lsm->env->key_format,
+ range_info->begin);
if (begin == NULL)
goto out;
}
if (range_info->end != NULL) {
- end = vy_key_from_msgpack(lsm->env->key_format,
- range_info->end);
+ end = vy_stmt_new_key_from_array(lsm->env->key_format,
+ range_info->end);
if (end == NULL)
goto out;
}
@@ -1069,8 +1069,8 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range)
/*
* Determine new ranges' boundaries.
*/
- struct tuple *split_key = vy_key_from_msgpack(key_format,
- split_key_raw);
+ struct tuple *split_key = vy_stmt_new_key_from_array(key_format,
+ split_key_raw);
if (split_key == NULL)
goto fail;
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index f0e47fce..ad28712b 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -1150,10 +1150,10 @@ vy_task_dump_complete(struct vy_task *task)
* intersecting the run or NULL if the run itersects all
* ranges.
*/
- min_key = vy_key_from_msgpack(key_format, new_run->info.min_key);
+ min_key = vy_stmt_new_key_from_array(key_format, new_run->info.min_key);
if (min_key == NULL)
goto fail;
- max_key = vy_key_from_msgpack(key_format, new_run->info.max_key);
+ max_key = vy_stmt_new_key_from_array(key_format, new_run->info.max_key);
if (max_key == NULL) {
tuple_unref(min_key);
goto fail;
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 57abad23..5ac7d833 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -236,20 +236,9 @@ vy_stmt_dup_lsregion(const struct tuple *stmt, struct lsregion *lsregion,
return mem_stmt;
}
-/**
- * Create the key statement from raw MessagePack data.
- * @param format Format of an index.
- * @param key MessagePack data that contain an array of
- * fields WITHOUT the array header.
- * @param part_count Count of the key fields that will be saved as
- * result.
- *
- * @retval not NULL Success.
- * @retval NULL Memory allocation error.
- */
struct tuple *
-vy_stmt_new_select(struct tuple_format *format, const char *key,
- uint32_t part_count)
+vy_stmt_new_key(struct tuple_format *format, const char *key,
+ uint32_t part_count)
{
assert(part_count == 0 || key != NULL);
/* Key don't have field map */
@@ -629,7 +618,7 @@ vy_stmt_extract_key(const struct tuple *stmt, struct key_def *key_def,
return NULL;
uint32_t part_count = mp_decode_array(&key_raw);
assert(part_count == key_def->part_count);
- struct tuple *key = vy_stmt_new_select(format, key_raw, part_count);
+ struct tuple *key = vy_stmt_new_key(format, key_raw, part_count);
/* Cleanup memory allocated by tuple_extract_key(). */
region_truncate(region, region_svp);
return key;
@@ -648,7 +637,7 @@ vy_stmt_extract_key_raw(const char *data, const char *data_end,
return NULL;
uint32_t part_count = mp_decode_array(&key_raw);
assert(part_count == key_def->part_count);
- struct tuple *key = vy_stmt_new_select(format, key_raw, part_count);
+ struct tuple *key = vy_stmt_new_key(format, key_raw, part_count);
/* Cleanup memory allocated by tuple_extract_key_raw(). */
region_truncate(region, region_svp);
return key;
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 3881a2f4..6c834aee 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -382,7 +382,7 @@ vy_stmt_compare_with_raw_key(const struct tuple *stmt, const char *key,
}
/**
- * Create the SELECT statement from raw MessagePack data.
+ * Create a key statement from raw MessagePack data.
* @param format Format of an index.
* @param key MessagePack data that contain an array of
* fields WITHOUT the array header.
@@ -393,8 +393,8 @@ vy_stmt_compare_with_raw_key(const struct tuple *stmt, const char *key,
* @retval not NULL Success.
*/
struct tuple *
-vy_stmt_new_select(struct tuple_format *format, const char *key,
- uint32_t part_count);
+vy_stmt_new_key(struct tuple_format *format, const char *key,
+ uint32_t part_count);
/**
* Create a new surrogate DELETE from @a key using format.
@@ -541,7 +541,7 @@ vy_stmt_upsert_ops(const struct tuple *tuple, uint32_t *mp_size)
}
/**
- * Create the SELECT statement from MessagePack array.
+ * Create a key statement from MessagePack array.
* @param format Format of an index.
* @param key MessagePack array of key fields.
*
@@ -549,15 +549,10 @@ vy_stmt_upsert_ops(const struct tuple *tuple, uint32_t *mp_size)
* @retval NULL Memory error.
*/
static inline struct tuple *
-vy_key_from_msgpack(struct tuple_format *format, const char *key)
+vy_stmt_new_key_from_array(struct tuple_format *format, const char *key)
{
- uint32_t part_count;
- /*
- * The statement already is a key, so simply copy it in
- * the new struct vy_stmt as SELECT.
- */
- part_count = mp_decode_array(&key);
- return vy_stmt_new_select(format, key, part_count);
+ uint32_t part_count = mp_decode_array(&key);
+ return vy_stmt_new_key(format, key, part_count);
}
/**
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index bcb60001..1e61ec97 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -117,9 +117,7 @@ vy_new_simple_stmt(struct tuple_format *format,
break;
}
case IPROTO_SELECT: {
- const char *key = buf;
- uint part_count = mp_decode_array(&key);
- ret = vy_stmt_new_select(stmt_env.key_format, key, part_count);
+ ret = vy_stmt_new_key_from_array(stmt_env.key_format, buf);
fail_if(ret == NULL);
break;
}
diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c
index 0226a62a..da4117c5 100644
--- a/test/unit/vy_mem.c
+++ b/test/unit/vy_mem.c
@@ -86,7 +86,7 @@ test_iterator_restore_after_insertion()
struct slab_cache *slab_cache = cord_slab_cache();
lsregion_create(&lsregion, slab_cache->arena);
- struct tuple *select_key = vy_stmt_new_select(format, "", 0);
+ struct tuple *select_key = vy_stmt_new_key(format, NULL, 0);
struct mempool history_node_pool;
mempool_create(&history_node_pool, cord_slab_cache(),
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 10/12] vinyl: rename key stmt construction routine
2019-02-21 10:26 ` [PATCH 10/12] vinyl: rename key stmt construction routine Vladimir Davydov
@ 2019-02-21 11:15 ` Konstantin Osipov
2019-02-21 12:14 ` Vladimir Davydov
0 siblings, 1 reply; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:15 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> Currently, it's called vy_stmt_new_select, but soon a key statement will
> be allowed to have any type, not just IPROTO_SELECT. So let's rename it
> to vy_stmt_new_key. Let's also rename the wrapper vy_key_from_msgpack to
> vy_stmt_new_key_from_array, because its key feature is that in contrast
> to vy_stmt_new_key it takes a msgpack array instead of key data + part
> count.
I like the new names, vy_stmt_new_key_from_array - maybe
vy_stmt_new_key_from_msgpack after all?
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [tarantool-patches] Re: [PATCH 10/12] vinyl: rename key stmt construction routine
2019-02-21 11:15 ` [tarantool-patches] " Konstantin Osipov
@ 2019-02-21 12:14 ` Vladimir Davydov
0 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 12:14 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 02:15:30PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> > Currently, it's called vy_stmt_new_select, but soon a key statement will
> > be allowed to have any type, not just IPROTO_SELECT. So let's rename it
> > to vy_stmt_new_key. Let's also rename the wrapper vy_key_from_msgpack to
> > vy_stmt_new_key_from_array, because its key feature is that in contrast
> > to vy_stmt_new_key it takes a msgpack array instead of key data + part
> > count.
>
> I like the new names, vy_stmt_new_key_from_array - maybe
> vy_stmt_new_key_from_msgpack after all?
But vy_stmt_new_key also takes msgpack so we'd have two functions both
taking msgpack, called vy_stmt_new_key and vy_stmt_new_key_from_msgpack.
Looks confusing IMO. Not that it really matters to me. I can rename it
if you think it's OK.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (9 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 10/12] vinyl: rename key stmt construction routine Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 11:16 ` [tarantool-patches] " Konstantin Osipov
2019-02-21 10:26 ` [PATCH 12/12] vinyl: do not fill secondary tuples with nulls when decoded Vladimir Davydov
2019-02-21 15:39 ` [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
12 siblings, 1 reply; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
To differentiate between key and tuple statements in comparators, we set
IPROTO_SELECT type for key statements. As a result, we can't use key
statements in the run iterator directly although secondary index runs do
store statements in key format. Instead we create surrogate tuples
filling missing fields with NULLs. This won't play nicely with multikey
indexes so we need to teach iterators to deal with statements in key
format. The first step in this direction is dropping IPROTO_SELECT and
starting to identify key statements by format instead.
---
src/box/vy_run.c | 2 +-
src/box/vy_stmt.c | 6 +++++-
src/box/vy_stmt.h | 38 +++++++++++++++++++++++++-------------
test/unit/vy_mem.c | 2 +-
4 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 06ef299e..e3470c09 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1250,7 +1250,7 @@ vy_run_iterator_do_seek(struct vy_run_iterator *itr,
struct key_def *key_def = itr->key_def;
if (iterator_type == ITER_EQ && bloom != NULL) {
bool need_lookup;
- if (vy_stmt_type(key) == IPROTO_SELECT) {
+ if (vy_stmt_is_key(key)) {
const char *data = tuple_data(key);
uint32_t part_count = mp_decode_array(&data);
need_lookup = tuple_bloom_maybe_has_key(bloom, data,
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 5ac7d833..5a76bfe4 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -240,6 +240,7 @@ struct tuple *
vy_stmt_new_key(struct tuple_format *format, const char *key,
uint32_t part_count)
{
+ assert(vy_stmt_is_key_format(format));
assert(part_count == 0 || key != NULL);
/* Key don't have field map */
assert(format->field_map_size == 0);
@@ -260,7 +261,6 @@ vy_stmt_new_key(struct tuple_format *format, const char *key,
char *data = mp_encode_array(raw, part_count);
memcpy(data, key, key_size);
assert(data + key_size == raw + bsize);
- vy_stmt_set_type(stmt, IPROTO_SELECT);
return stmt;
}
@@ -839,6 +839,10 @@ vy_stmt_snprint(char *buf, int size, const struct tuple *stmt)
SNPRINT(total, snprintf, buf, size, "<NULL>");
return total;
}
+ if (vy_stmt_type(stmt) == 0) {
+ SNPRINT(total, mp_snprint, buf, size, tuple_data(stmt));
+ return total;
+ }
SNPRINT(total, snprintf, buf, size, "%s(",
iproto_type_name(vy_stmt_type(stmt)));
SNPRINT(total, mp_snprint, buf, size, tuple_data(stmt));
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index 6c834aee..88d74de7 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -133,12 +133,9 @@ enum {
};
/**
- * There are two groups of statements:
+ * A vinyl statement can have either key or tuple format.
*
- * - SELECT is "key" statement.
- * - DELETE, UPSERT and REPLACE are "tuple" statements.
- *
- * REPLACE/UPSERT/DELETE statements structure:
+ * Tuple statement structure:
* data_offset
* ^
* +----------------------------------+
@@ -154,7 +151,7 @@ enum {
* indexed then before MessagePack data are stored offsets only for field 3 and
* field 5.
*
- * SELECT statements structure.
+ * Key statement structure:
* +--------------+-----------------+
* | array header | part1 ... partN | - MessagePack data
* +--------------+-----------------+
@@ -164,7 +161,7 @@ enum {
struct vy_stmt {
struct tuple base;
int64_t lsn;
- uint8_t type; /* IPROTO_SELECT/REPLACE/UPSERT/DELETE */
+ uint8_t type; /* IPROTO_INSERT/REPLACE/UPSERT/DELETE */
uint8_t flags;
/**
* Offsets array concatenated with MessagePack fields
@@ -239,6 +236,21 @@ vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
*((uint8_t *)stmt - 1) = n;
}
+/** Return true if the given format is a key format. */
+static inline bool
+vy_stmt_is_key_format(const struct tuple_format *format)
+{
+ struct vy_stmt_env *env = format->engine;
+ return env->key_format == format;
+}
+
+/** Return true if the vinyl statement has key format. */
+static inline bool
+vy_stmt_is_key(const struct tuple *stmt)
+{
+ return vy_stmt_is_key_format(tuple_format(stmt));
+}
+
/**
* Return the number of key parts defined in the given vinyl
* statement.
@@ -249,7 +261,7 @@ vy_stmt_set_n_upserts(struct tuple *stmt, uint8_t n)
static inline uint32_t
vy_stmt_key_part_count(const struct tuple *stmt, struct key_def *key_def)
{
- if (vy_stmt_type(stmt) == IPROTO_SELECT) {
+ if (vy_stmt_is_key(stmt)) {
uint32_t part_count = tuple_field_count(stmt);
assert(part_count <= key_def->part_count);
return part_count;
@@ -348,8 +360,8 @@ static inline int
vy_stmt_compare(const struct tuple *a, const struct tuple *b,
struct key_def *key_def)
{
- bool a_is_tuple = vy_stmt_type(a) != IPROTO_SELECT;
- bool b_is_tuple = vy_stmt_type(b) != IPROTO_SELECT;
+ 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) {
@@ -374,7 +386,7 @@ static inline int
vy_stmt_compare_with_raw_key(const struct tuple *stmt, const char *key,
struct key_def *key_def)
{
- if (vy_stmt_type(stmt) != IPROTO_SELECT) {
+ 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);
}
@@ -557,7 +569,7 @@ vy_stmt_new_key_from_array(struct tuple_format *format, const char *key)
/**
* Extract the key from a tuple by the given key definition
- * and store the result in a SELECT statement allocated with
+ * and store the result in a key statement allocated with
* malloc().
*/
struct tuple *
@@ -566,7 +578,7 @@ vy_stmt_extract_key(const struct tuple *stmt, struct key_def *key_def,
/**
* Extract the key from msgpack by the given key definition
- * and store the result in a SELECT statement allocated with
+ * and store the result in a key statement allocated with
* malloc().
*/
struct tuple *
diff --git a/test/unit/vy_mem.c b/test/unit/vy_mem.c
index da4117c5..b672ee91 100644
--- a/test/unit/vy_mem.c
+++ b/test/unit/vy_mem.c
@@ -86,7 +86,7 @@ test_iterator_restore_after_insertion()
struct slab_cache *slab_cache = cord_slab_cache();
lsregion_create(&lsregion, slab_cache->arena);
- struct tuple *select_key = vy_stmt_new_key(format, NULL, 0);
+ struct tuple *select_key = vy_stmt_new_key(stmt_env.key_format, NULL, 0);
struct mempool history_node_pool;
mempool_create(&history_node_pool, cord_slab_cache(),
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* [tarantool-patches] Re: [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements
2019-02-21 10:26 ` [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements Vladimir Davydov
@ 2019-02-21 11:16 ` Konstantin Osipov
0 siblings, 0 replies; 30+ messages in thread
From: Konstantin Osipov @ 2019-02-21 11:16 UTC (permalink / raw)
To: tarantool-patches
* Vladimir Davydov <vdavydov.dev@gmail.com> [19/02/21 13:31]:
> To differentiate between key and tuple statements in comparators, we set
> IPROTO_SELECT type for key statements. As a result, we can't use key
> statements in the run iterator directly although secondary index runs do
> store statements in key format. Instead we create surrogate tuples
> filling missing fields with NULLs. This won't play nicely with multikey
> indexes so we need to teach iterators to deal with statements in key
> format. The first step in this direction is dropping IPROTO_SELECT and
> starting to identify key statements by format instead.
It's amazing how simple this patch is, it's OK to push provided we
decide to push the final patch.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 12/12] vinyl: do not fill secondary tuples with nulls when decoded
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (10 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements Vladimir Davydov
@ 2019-02-21 10:26 ` Vladimir Davydov
2019-02-21 15:39 ` [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
12 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 10:26 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
In contrast to a primary index, which stores full tuples, secondary
indexes only store extended (secondary + primary) keys on disk. To make
them look like tuples, we fill missing fields with nulls (aka tuple
surrogate). This isn't going to work nicely with multikey indexes
though: how would you make a surrogate array from a key? We could
special-case multikey index handling, but that would look cumbersome.
So this patch removes nulls from secondary tuples restored from disk
altogether. To achieve that, it's enough to use key_format for them -
then the comparators will detect that it's actually a key, not a tuple
and use the appropriate primitive.
---
src/box/key_def.c | 36 +++++++++++++++++++++
src/box/key_def.h | 24 ++++++++++++++
src/box/tuple_bloom.c | 64 +++++++++++++++++++++++++++++--------
src/box/tuple_bloom.h | 15 +++++++++
src/box/tuple_compare.cc | 24 ++++++++++++++
src/box/vinyl.c | 39 ++++++++++++++++-------
src/box/vy_lsm.c | 42 +++++++++++++-----------
src/box/vy_lsm.h | 13 +++++---
src/box/vy_run.c | 78 ++++++++++++++++++++++++++++++---------------
src/box/vy_scheduler.c | 2 +-
src/box/vy_stmt.c | 40 +++++++++++++----------
test/unit/vy_point_lookup.c | 2 +-
12 files changed, 285 insertions(+), 94 deletions(-)
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 92b2586d..9a12322f 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -690,6 +690,42 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
return new_def;
}
+struct key_def *
+key_def_extract(const struct key_def *cmp_def, const struct key_def *pk_def,
+ struct region *region)
+{
+ struct key_def *extracted_def = NULL;
+ size_t region_svp = region_used(region);
+
+ /* First, dump primary key parts as is. */
+ struct key_part_def *parts = region_alloc(region,
+ pk_def->part_count * sizeof(*parts));
+ if (parts == NULL) {
+ diag_set(OutOfMemory, pk_def->part_count * sizeof(*parts),
+ "region", "key def parts");
+ goto out;
+ }
+ if (key_def_dump_parts(pk_def, parts, region) != 0)
+ goto out;
+ /*
+ * Second, update field numbers to match the primary key
+ * parts in a secondary key.
+ */
+ for (uint32_t i = 0; i < pk_def->part_count; i++) {
+ const struct key_part *part = key_def_find(cmp_def,
+ &pk_def->parts[i]);
+ assert(part != NULL);
+ parts[i].fieldno = part - cmp_def->parts;
+ parts[i].path = NULL;
+ }
+
+ /* Finally, allocate the new key definition. */
+ extracted_def = key_def_new(parts, pk_def->part_count);
+out:
+ region_truncate(region, region_svp);
+ return extracted_def;
+}
+
int
key_validate_parts(const struct key_def *key_def, const char *key,
uint32_t part_count, bool allow_nullable)
diff --git a/src/box/key_def.h b/src/box/key_def.h
index dd62f6a3..62f4cb21 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -375,6 +375,20 @@ key_def_contains(const struct key_def *first, const struct key_def *second);
struct key_def *
key_def_merge(const struct key_def *first, const struct key_def *second);
+/**
+ * Create a key definition suitable for extracting primary key
+ * parts from an extended secondary key.
+ * @param cmp_def Extended secondary key definition
+ * (must include primary key parts).
+ * @param pk_def Primary key definition.
+ * @param region Region used for temporary allocations.
+ * @retval not NULL Pointer to the extracted key definition.
+ * @retval NULL Memory allocation error.
+ */
+struct key_def *
+key_def_extract(const struct key_def *cmp_def, const struct key_def *pk_def,
+ struct region *region);
+
/*
* Check that parts of the key match with the key definition.
* @param key_def Key definition.
@@ -523,6 +537,16 @@ tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
struct key_def *key_def);
/**
+ * Return the length of the longest common prefix of two keys.
+ * @param key_a first key
+ * @param key_b second key
+ * @param key_def key defintion
+ * @return number of key parts the two keys have in common
+ */
+uint32_t
+key_common_parts(const char *key_a, const char *key_b, struct key_def *key_def);
+
+/**
* 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
diff --git a/src/box/tuple_bloom.c b/src/box/tuple_bloom.c
index cf887c7b..60a7a8fd 100644
--- a/src/box/tuple_bloom.c
+++ b/src/box/tuple_bloom.c
@@ -70,6 +70,25 @@ tuple_bloom_builder_delete(struct tuple_bloom_builder *builder)
free(builder);
}
+static int
+tuple_hash_array_add(struct tuple_hash_array *hash_arr, uint32_t hash)
+{
+ if (hash_arr->count >= hash_arr->capacity) {
+ uint32_t capacity = MAX(hash_arr->capacity * 2, 1024U);
+ uint32_t *values = realloc(hash_arr->values,
+ capacity * sizeof(*values));
+ if (values == NULL) {
+ diag_set(OutOfMemory, capacity * sizeof(*values),
+ "malloc", "tuple hash array");
+ return -1;
+ }
+ hash_arr->capacity = capacity;
+ hash_arr->values = values;
+ }
+ hash_arr->values[hash_arr->count++] = hash;
+ return 0;
+}
+
int
tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
const struct tuple *tuple, struct key_def *key_def,
@@ -92,21 +111,38 @@ tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
*/
continue;
}
- struct tuple_hash_array *hash_arr = &builder->parts[i];
- if (hash_arr->count >= hash_arr->capacity) {
- uint32_t capacity = MAX(hash_arr->capacity * 2, 1024U);
- uint32_t *values = realloc(hash_arr->values,
- capacity * sizeof(*values));
- if (values == NULL) {
- diag_set(OutOfMemory, capacity * sizeof(*values),
- "malloc", "tuple hash array");
- return -1;
- }
- hash_arr->capacity = capacity;
- hash_arr->values = values;
+ tuple_hash_array_add(&builder->parts[i],
+ PMurHash32_Result(h, carry, total_size));
+ }
+ return 0;
+}
+
+int
+tuple_bloom_builder_add_key(struct tuple_bloom_builder *builder,
+ const char *key, uint32_t part_count,
+ struct key_def *key_def, uint32_t hashed_parts)
+{
+ (void)part_count;
+ assert(part_count >= key_def->part_count);
+ assert(builder->part_count == key_def->part_count);
+
+ uint32_t h = HASH_SEED;
+ uint32_t carry = 0;
+ uint32_t total_size = 0;
+
+ for (uint32_t i = 0; i < key_def->part_count; i++) {
+ total_size += tuple_hash_field(&h, &carry, &key,
+ key_def->parts[i].coll);
+ if (i < hashed_parts) {
+ /*
+ * This part is already in the bloom, proceed
+ * to the next one. Note, we can't skip to
+ * hashed_parts, as we need to compute the hash.
+ */
+ continue;
}
- uint32_t hash = PMurHash32_Result(h, carry, total_size);
- hash_arr->values[hash_arr->count++] = hash;
+ tuple_hash_array_add(&builder->parts[i],
+ PMurHash32_Result(h, carry, total_size));
}
return 0;
}
diff --git a/src/box/tuple_bloom.h b/src/box/tuple_bloom.h
index b05fee18..c45bc30d 100644
--- a/src/box/tuple_bloom.h
+++ b/src/box/tuple_bloom.h
@@ -121,6 +121,21 @@ tuple_bloom_builder_add(struct tuple_bloom_builder *builder,
uint32_t hashed_parts);
/**
+ * Add a key hash to a tuple bloom filter builder.
+ * @param builder - bloom filter builder
+ * @param key - key to add
+ * @param part_count - number of parts in the key
+ * @param key_def - key definition
+ * @param hashed_parts - number of key parts that have already
+ * been added to the builder
+ * @return 0 on success, -1 on OOM
+ */
+int
+tuple_bloom_builder_add_key(struct tuple_bloom_builder *builder,
+ const char *key, uint32_t part_count,
+ struct key_def *key_def, uint32_t hashed_parts);
+
+/**
* Create a new tuple bloom filter.
* @param builder - bloom filter builder
* @param fpr - desired false positive rate
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index b8dc350f..b617d6d8 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -458,6 +458,30 @@ tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
return i;
}
+uint32_t
+key_common_parts(const char *key_a, const char *key_b, struct key_def *key_def)
+{
+ uint32_t i;
+ uint32_t part_count_a = mp_decode_array(&key_a);
+ uint32_t part_count_b = mp_decode_array(&key_b);
+ uint32_t part_count = MIN(part_count_a, part_count_b);
+ part_count = MIN(part_count, key_def->part_count);
+ for (i = 0; i < part_count; i++) {
+ struct key_part *part = &key_def->parts[i];
+ enum mp_type a_type = mp_typeof(*key_a);
+ enum mp_type b_type = mp_typeof(*key_b);
+ if (a_type == MP_NIL && b_type == MP_NIL)
+ continue;
+ if (a_type == MP_NIL || b_type == MP_NIL ||
+ tuple_compare_field_with_hint(key_a, a_type,
+ key_b, b_type, part->type, part->coll) != 0)
+ break;
+ mp_next(&key_a);
+ mp_next(&key_b);
+ }
+ return i;
+}
+
template<bool is_nullable, bool has_optional_parts, bool has_json_paths>
static inline int
tuple_compare_slowpath(const struct tuple *tuple_a, const struct tuple *tuple_b,
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 925784b0..ce0368ab 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -720,10 +720,9 @@ vinyl_space_create_index(struct space *space, struct index_def *index_def)
pk = vy_lsm(space_index(space, 0));
assert(pk != NULL);
}
- struct vy_lsm *lsm = vy_lsm_new(&env->lsm_env, &env->stmt_env,
- &env->cache_env, &env->mem_env,
- index_def, space->format, pk,
- space_group_id(space));
+ struct vy_lsm *lsm = vy_lsm_new(&env->lsm_env, &env->cache_env,
+ &env->mem_env, index_def, space->format,
+ pk, space_group_id(space));
if (lsm == NULL) {
free(index);
return NULL;
@@ -1302,13 +1301,27 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
const struct vy_read_view **rv,
struct tuple *tuple, struct tuple **result)
{
+ int rc = 0;
assert(lsm->index_id > 0);
- if (vy_point_lookup(lsm->pk, tx, rv, tuple, result) != 0)
- return -1;
+ struct tuple *key;
+ if (vy_stmt_is_key(tuple)) {
+ key = vy_stmt_extract_key(tuple, lsm->pk_extractor,
+ lsm->env->key_format);
+ if (key == NULL)
+ return -1;
+ } else {
+ key = tuple;
+ tuple_ref(key);
+ }
+
+ if (vy_point_lookup(lsm->pk, tx, rv, key, result) != 0) {
+ rc = -1;
+ goto out;
+ }
if (*result == NULL ||
- vy_stmt_compare(*result, tuple, lsm->key_def) != 0) {
+ vy_stmt_compare(*result, tuple, lsm->cmp_def) != 0) {
/*
* If a tuple read from a secondary index doesn't
* match the tuple corresponding to it in the
@@ -1328,7 +1341,7 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
* the tuple cache implementation.
*/
vy_cache_on_write(&lsm->cache, tuple, NULL);
- return 0;
+ goto out;
}
/*
@@ -1341,13 +1354,15 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx,
*/
if (tx != NULL && vy_tx_track_point(tx, lsm->pk, *result) != 0) {
tuple_unref(*result);
- return -1;
+ rc = -1;
+ goto out;
}
if ((*rv)->vlsn == INT64_MAX)
- vy_cache_add(&lsm->pk->cache, *result, NULL, tuple, ITER_EQ);
-
- return 0;
+ vy_cache_add(&lsm->pk->cache, *result, NULL, key, ITER_EQ);
+out:
+ tuple_unref(key);
+ return rc;
}
/**
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index 2bb56d87..43f8dba8 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -118,10 +118,9 @@ vy_lsm_mem_tree_size(struct vy_lsm *lsm)
}
struct vy_lsm *
-vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
- struct vy_cache_env *cache_env, struct vy_mem_env *mem_env,
- struct index_def *index_def, struct tuple_format *format,
- struct vy_lsm *pk, uint32_t group_id)
+vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
+ struct vy_mem_env *mem_env, struct index_def *index_def,
+ struct tuple_format *format, struct vy_lsm *pk, uint32_t group_id)
{
static int64_t run_buckets[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 15, 20, 25, 50, 100,
@@ -138,16 +137,14 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
}
lsm->env = lsm_env;
- struct key_def *key_def = key_def_dup(index_def->key_def);
- if (key_def == NULL)
+ lsm->key_def = key_def_dup(index_def->key_def);
+ if (lsm->key_def == NULL)
goto fail_key_def;
- struct key_def *cmp_def = key_def_dup(index_def->cmp_def);
- if (cmp_def == NULL)
+ lsm->cmp_def = key_def_dup(index_def->cmp_def);
+ if (lsm->cmp_def == NULL)
goto fail_cmp_def;
- lsm->cmp_def = cmp_def;
- lsm->key_def = key_def;
if (index_def->iid == 0) {
/*
* Disk tuples can be returned to an user from a
@@ -156,10 +153,12 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
*/
lsm->disk_format = format;
} else {
- lsm->disk_format = vy_stmt_format_new(stmt_env, &cmp_def, 1,
- NULL, 0, 0, NULL);
- if (lsm->disk_format == NULL)
- goto fail_format;
+ lsm->disk_format = lsm_env->key_format;
+
+ lsm->pk_extractor = key_def_extract(lsm->cmp_def, pk->key_def,
+ &fiber()->gc);
+ if (lsm->pk_extractor == NULL)
+ goto fail_pk_extractor;
}
tuple_format_ref(lsm->disk_format);
@@ -170,7 +169,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
if (lsm->run_hist == NULL)
goto fail_run_hist;
- lsm->mem = vy_mem_new(mem_env, cmp_def, format,
+ lsm->mem = vy_mem_new(mem_env, lsm->cmp_def, format,
*lsm->env->p_generation,
space_cache_version);
if (lsm->mem == NULL)
@@ -180,7 +179,8 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
lsm->refs = 1;
lsm->dump_lsn = -1;
lsm->commit_lsn = -1;
- vy_cache_create(&lsm->cache, cache_env, cmp_def, index_def->iid == 0);
+ vy_cache_create(&lsm->cache, cache_env, lsm->cmp_def,
+ index_def->iid == 0);
rlist_create(&lsm->sealed);
vy_range_tree_new(&lsm->range_tree);
vy_range_heap_create(&lsm->range_heap);
@@ -208,10 +208,12 @@ fail_run_hist:
vy_lsm_stat_destroy(&lsm->stat);
fail_stat:
tuple_format_unref(lsm->disk_format);
-fail_format:
- key_def_delete(cmp_def);
+ if (lsm->pk_extractor != NULL)
+ key_def_delete(lsm->pk_extractor);
+fail_pk_extractor:
+ key_def_delete(lsm->cmp_def);
fail_cmp_def:
- key_def_delete(key_def);
+ key_def_delete(lsm->key_def);
fail_key_def:
free(lsm);
fail:
@@ -262,6 +264,8 @@ vy_lsm_delete(struct vy_lsm *lsm)
tuple_format_unref(lsm->disk_format);
key_def_delete(lsm->cmp_def);
key_def_delete(lsm->key_def);
+ if (lsm->pk_extractor != NULL)
+ key_def_delete(lsm->pk_extractor);
histogram_delete(lsm->run_hist);
vy_lsm_stat_destroy(&lsm->stat);
vy_cache_destroy(&lsm->cache);
diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h
index b94e7a43..981893ec 100644
--- a/src/box/vy_lsm.h
+++ b/src/box/vy_lsm.h
@@ -200,6 +200,12 @@ struct vy_lsm {
/** Key definition passed by the user. */
struct key_def *key_def;
/**
+ * Key definition to extract primary key parts from
+ * a secondary key. NULL if this LSM tree corresponds
+ * to a primary index.
+ */
+ struct key_def *pk_extractor;
+ /**
* If the following flag is set, the index this LSM tree
* is created for is unique and it must be checked for
* duplicates on INSERT. Otherwise, the check can be skipped,
@@ -333,10 +339,9 @@ vy_lsm_mem_tree_size(struct vy_lsm *lsm);
/** Allocate a new LSM tree object. */
struct vy_lsm *
-vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_stmt_env *stmt_env,
- struct vy_cache_env *cache_env, struct vy_mem_env *mem_env,
- struct index_def *index_def, struct tuple_format *format,
- struct vy_lsm *pk, uint32_t group_id);
+vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
+ struct vy_mem_env *mem_env, struct index_def *index_def,
+ struct tuple_format *format, struct vy_lsm *pk, uint32_t group_id);
/** Free an LSM tree object. */
void
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index e3470c09..01257144 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1412,8 +1412,7 @@ vy_run_iterator_open(struct vy_run_iterator *itr,
struct vy_slice *slice, enum iterator_type iterator_type,
const struct tuple *key, const struct vy_read_view **rv,
struct key_def *cmp_def, struct key_def *key_def,
- struct tuple_format *format,
- bool is_primary)
+ struct tuple_format *format, bool is_primary)
{
itr->stat = stat;
itr->cmp_def = cmp_def;
@@ -2149,7 +2148,8 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
if (run->info.page_count >= writer->page_info_capacity &&
vy_run_alloc_page_info(run, &writer->page_info_capacity) != 0)
return -1;
- const char *key = tuple_extract_key(first_stmt, writer->cmp_def, NULL);
+ const char *key = vy_stmt_is_key(first_stmt) ? tuple_data(first_stmt) :
+ tuple_extract_key(first_stmt, writer->cmp_def, NULL);
if (key == NULL)
return -1;
if (run->info.page_count == 0) {
@@ -2165,6 +2165,27 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
return 0;
}
+static int
+vy_tuple_bloom_add(struct tuple_bloom_builder *bloom, struct key_def *key_def,
+ struct tuple *stmt, struct tuple *last_stmt)
+{
+ if (vy_stmt_is_key(stmt)) {
+ assert(last_stmt == NULL || vy_stmt_is_key(last_stmt));
+ const char *key = tuple_data(stmt);
+ uint32_t hashed_parts = last_stmt == NULL ? 0 :
+ key_common_parts(key, tuple_data(last_stmt), key_def);
+ uint32_t part_count = mp_decode_array(&key);
+ return tuple_bloom_builder_add_key(bloom, key, part_count,
+ key_def, hashed_parts);
+ } else {
+ assert(last_stmt == NULL || !vy_stmt_is_key(last_stmt));
+ uint32_t hashed_parts = last_stmt == NULL ? 0 :
+ tuple_common_key_parts(stmt, last_stmt, key_def);
+ return tuple_bloom_builder_add(bloom, stmt, key_def,
+ hashed_parts);
+ }
+}
+
/**
* Write @a stmt into a current page.
* @param writer Run writer.
@@ -2176,13 +2197,10 @@ vy_run_writer_start_page(struct vy_run_writer *writer,
static int
vy_run_writer_write_to_page(struct vy_run_writer *writer, struct tuple *stmt)
{
- if (writer->bloom != NULL) {
- uint32_t hashed_parts = writer->last_stmt == NULL ? 0 :
- tuple_common_key_parts(stmt, writer->last_stmt,
- writer->key_def);
- tuple_bloom_builder_add(writer->bloom, stmt,
- writer->key_def, hashed_parts);
- }
+ if (writer->bloom != NULL &&
+ vy_tuple_bloom_add(writer->bloom, writer->key_def, stmt,
+ writer->last_stmt) != 0)
+ return -1;
if (writer->last_stmt != NULL)
vy_stmt_unref_if_possible(writer->last_stmt);
writer->last_stmt = stmt;
@@ -2302,7 +2320,9 @@ vy_run_writer_commit(struct vy_run_writer *writer)
}
assert(writer->last_stmt != NULL);
- const char *key = tuple_extract_key(writer->last_stmt,
+ 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);
if (key == NULL)
goto out;
@@ -2370,6 +2390,7 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
uint32_t page_info_capacity = 0;
const char *key = NULL;
+ char *page_min_key = NULL;
int64_t max_lsn = 0;
int64_t min_lsn = INT64_MAX;
struct tuple *prev_tuple = NULL;
@@ -2390,7 +2411,6 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
if (run->info.page_count == page_info_capacity &&
vy_run_alloc_page_info(run, &page_info_capacity) != 0)
goto close_err;
- const char *page_min_key = NULL;
uint32_t page_row_count = 0;
uint64_t page_row_index_offset = 0;
uint64_t row_offset = xlog_cursor_tx_pos(&cursor);
@@ -2407,14 +2427,14 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
format, iid == 0);
if (tuple == NULL)
goto close_err;
- if (bloom_builder != NULL) {
- uint32_t hashed_parts = prev_tuple == NULL ? 0 :
- tuple_common_key_parts(prev_tuple,
- tuple, key_def);
- tuple_bloom_builder_add(bloom_builder, tuple,
- key_def, hashed_parts);
+ if (bloom_builder != NULL &&
+ vy_tuple_bloom_add(bloom_builder, key_def,
+ tuple, prev_tuple) != 0) {
+ tuple_unref(tuple);
+ goto close_err;
}
- key = tuple_extract_key(tuple, cmp_def, NULL);
+ key = vy_stmt_is_key(tuple) ? tuple_data(tuple) :
+ tuple_extract_key(tuple, cmp_def, NULL);
if (prev_tuple != NULL)
tuple_unref(prev_tuple);
prev_tuple = tuple;
@@ -2425,8 +2445,11 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
if (run->info.min_key == NULL)
goto close_err;
}
- if (page_min_key == NULL)
- page_min_key = key;
+ if (page_min_key == NULL) {
+ page_min_key = key_dup(key);
+ if (page_min_key == NULL)
+ goto close_err;
+ }
if (xrow.lsn > max_lsn)
max_lsn = xrow.lsn;
if (xrow.lsn < min_lsn)
@@ -2443,11 +2466,8 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
info->row_index_offset = page_row_index_offset;
++run->info.page_count;
vy_run_acct_page(run, info);
- }
-
- if (prev_tuple != NULL) {
- tuple_unref(prev_tuple);
- prev_tuple = NULL;
+ free(page_min_key);
+ page_min_key = NULL;
}
if (key != NULL) {
@@ -2455,6 +2475,10 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
if (run->info.max_key == NULL)
goto close_err;
}
+ if (prev_tuple != NULL) {
+ tuple_unref(prev_tuple);
+ prev_tuple = NULL;
+ }
run->info.max_lsn = max_lsn;
run->info.min_lsn = min_lsn;
@@ -2487,6 +2511,8 @@ close_err:
region_truncate(region, mem_used);
if (prev_tuple != NULL)
tuple_unref(prev_tuple);
+ if (page_min_key != NULL)
+ free(page_min_key);
if (bloom_builder != NULL)
tuple_bloom_builder_delete(bloom_builder);
if (xlog_cursor_is_open(&cursor))
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index ad28712b..48e739af 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -1407,7 +1407,7 @@ vy_task_dump_new(struct vy_scheduler *scheduler, struct vy_worker *worker,
*/
struct vy_stmt_stream *wi;
bool is_last_level = (lsm->run_count == 0);
- wi = vy_write_iterator_new(task->cmp_def, lsm->disk_format,
+ wi = vy_write_iterator_new(task->cmp_def, lsm->mem_format,
lsm->index_id == 0, is_last_level,
scheduler->read_views, NULL);
if (wi == NULL)
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 5a76bfe4..29accf37 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -699,6 +699,8 @@ int
vy_stmt_encode_primary(const struct tuple *value, struct key_def *key_def,
uint32_t space_id, struct xrow_header *xrow)
{
+ assert(!vy_stmt_is_key(value));
+
memset(xrow, 0, sizeof(*xrow));
enum iproto_type type = vy_stmt_type(value);
xrow->type = type;
@@ -755,9 +757,14 @@ vy_stmt_encode_secondary(const struct tuple *value, struct key_def *cmp_def,
memset(&request, 0, sizeof(request));
request.type = type;
uint32_t size;
- const char *extracted = tuple_extract_key(value, cmp_def, &size);
- if (extracted == NULL)
- return -1;
+ const char *extracted;
+ if (!vy_stmt_is_key(value)) {
+ extracted = tuple_extract_key(value, cmp_def, &size);
+ if (extracted == NULL)
+ return -1;
+ } else {
+ extracted = tuple_data_range(value, &size);
+ }
if (type == IPROTO_REPLACE || type == IPROTO_INSERT) {
request.tuple = extracted;
request.tuple_end = extracted + size;
@@ -791,21 +798,20 @@ vy_stmt_decode(struct xrow_header *xrow, const struct key_def *key_def,
switch (request.type) {
case IPROTO_DELETE:
/* extract key */
- stmt = vy_stmt_new_surrogate_from_key(request.key,
- IPROTO_DELETE,
- key_def, format);
- break;
- case IPROTO_INSERT:
- case IPROTO_REPLACE:
- if (is_primary) {
- stmt = vy_stmt_new_with_ops(format, request.tuple,
- request.tuple_end,
- NULL, 0, request.type);
- } else {
- stmt = vy_stmt_new_surrogate_from_key(request.tuple,
- request.type,
+ if (is_primary)
+ stmt = vy_stmt_new_surrogate_from_key(request.key,
+ IPROTO_DELETE,
key_def, format);
- }
+ else
+ stmt = vy_stmt_new_with_ops(format, request.key,
+ request.key_end,
+ NULL, 0, IPROTO_DELETE);
+ break;
+ case IPROTO_INSERT:
+ case IPROTO_REPLACE:
+ stmt = vy_stmt_new_with_ops(format, request.tuple,
+ request.tuple_end,
+ NULL, 0, request.type);
break;
case IPROTO_UPSERT:
ops.iov_base = (char *)request.ops;
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index 5f20d09b..6268d2d2 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -94,7 +94,7 @@ test_basic()
index_def_new(512, 0, "primary", sizeof("primary") - 1, TREE,
&index_opts, key_def, NULL);
- struct vy_lsm *pk = vy_lsm_new(&lsm_env, &stmt_env, &cache_env, &mem_env,
+ struct vy_lsm *pk = vy_lsm_new(&lsm_env, &cache_env, &mem_env,
index_def, format, NULL, 0);
isnt(pk, NULL, "lsm is not NULL")
--
2.11.0
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 00/12] vinyl: do not fill secondary tuples with nulls
2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
` (11 preceding siblings ...)
2019-02-21 10:26 ` [PATCH 12/12] vinyl: do not fill secondary tuples with nulls when decoded Vladimir Davydov
@ 2019-02-21 15:39 ` Vladimir Davydov
12 siblings, 0 replies; 30+ messages in thread
From: Vladimir Davydov @ 2019-02-21 15:39 UTC (permalink / raw)
To: kostja; +Cc: tarantool-patches
On Thu, Feb 21, 2019 at 01:26:00PM +0300, Vladimir Davydov wrote:
> vinyl: use vy_lsm_env::empty_key where appropriate
> vinyl: make vy_tuple_delete static
> key_def: cleanup virtual function initialization
> key_def: move cmp and hash functions declarations to key_def.h
> vinyl: move vy_tuple_key_contains_null to generic code
Pushed patches 1-5 to 2.1 (see above).
^ permalink raw reply [flat|nested] 30+ messages in thread