From 68c93246b38e3dcf06df5414fdbcbb6626f616a4 Mon Sep 17 00:00:00 2001
From: Aleksey Demakov <ademakov@gmail.com>
Date: Mon, 23 Jan 2012 22:41:59 +0400
Subject: [PATCH] Make field type conflict a config error and optimize
validate_indexes()
---
mod/box/box.m | 92 +++++++++++++++++++++++----------
test/box/configuration.result | 7 +++
test/box/configuration.test | 14 +++++
test/box/tarantool_bad_type.cfg | 24 +++++++++
4 files changed, 111 insertions(+), 26 deletions(-)
create mode 100644 test/box/tarantool_bad_type.cfg
diff --git a/mod/box/box.m b/mod/box/box.m
index ad926279e0..708830bd68 100644
--- a/mod/box/box.m
+++ b/mod/box/box.m
@@ -141,31 +141,40 @@ tuple_txn_ref(struct box_txn *txn, struct box_tuple *tuple)
static void
validate_indexes(struct box_txn *txn)
{
- int n = index_count(&space[txn->n]);
- if (n <= 1)
- return;
-
- for (int i = 1; i < n; i++) {
- Index *index = space[txn->n].index[i];
-
- for (u32 f = 0; f < index->key_def->part_count; ++f) {
- if (index->key_def->parts[f].fieldno >= txn->tuple->cardinality)
- tnt_raise(IllegalParams, :"tuple must have all indexed fields");
-
- if (index->key_def->parts[f].type == STRING)
- continue;
+ struct space *sp = &space[txn->n];
+ int n = index_count(sp);
- void *field = tuple_field(txn->tuple, index->key_def->parts[f].fieldno);
- u32 len = load_varint32(&field);
+ /* Only secondary indexes are validated here. So check to see
+ if there are any.*/
+ if (n <= 1) {
+ return;
+ }
- if (index->key_def->parts[f].type == NUM && len != sizeof(u32))
+ /* Check to see if the tuple has a sufficient number of fields. */
+ if (txn->tuple->cardinality < sp->field_count)
+ tnt_raise(IllegalParams, :"tuple must have all indexed fields");
+
+ /* Sweep through the tuple and check the field sizes. */
+ u8 *data = txn->tuple->data;
+ for (int f = 0; f < sp->field_count; ++f) {
+ /* Get the size of the current field and advance. */
+ u32 len = load_varint32((void **) &data);
+ data += len;
+
+ /* Check fixed size fields (NUM and NUM64) and skip undefined
+ size fields (STRING and UNKNOWN). */
+ if (sp->field_types[f] == NUM) {
+ if (len != sizeof(u32))
tnt_raise(IllegalParams, :"field must be NUM");
-
- if (index->key_def->parts[f].type == NUM64 && len != sizeof(u64))
+ } else if (sp->field_types[f] == NUM64) {
+ if (len != sizeof(u64))
tnt_raise(IllegalParams, :"field must be NUM64");
}
+ }
- /* Check key uniqueness */
+ /* Check key uniqueness */
+ for (int i = 1; i < n; ++i) {
+ Index *index = space[txn->n].index[i];
if (index->key_def->is_unique) {
struct box_tuple *tuple = [index findByTuple: txn->tuple];
if (tuple != NULL && tuple != txn->old_tuple)
@@ -1062,19 +1071,16 @@ space_init_field_types(struct space *space)
}
}
+#ifndef NDEBUG
/* validate field type info */
for (i = 0; i < key_count; i++) {
struct key_def *def = &key_defs[i];
for (int pi = 0; pi < def->part_count; pi++) {
struct key_part *part = &def->parts[pi];
- if (space->field_types[part->fieldno] != part->type
- && space->field_types[part->fieldno] != UNKNOWN) {
- space->field_types[part->fieldno] = UNKNOWN;
- say_warn("type conflict in space %d for field %d",
- space_n(space), part->fieldno);
- }
+ assert(space->field_types[part->fieldno] == part->type);
}
}
+#endif
}
static void
@@ -1361,6 +1367,8 @@ check_spaces(struct tarantool_cfg *conf)
return -1;
}
+ int max_key_fieldno = -1;
+
/* check spaces indexes */
for (size_t j = 0; space->index[j] != NULL; ++j) {
typeof(space->index[j]) index = space->index[j];
@@ -1404,6 +1412,10 @@ check_spaces(struct tarantool_cfg *conf)
return -1;
}
+ if (max_key_fieldno < key->fieldno) {
+ max_key_fieldno = key->fieldno;
+ }
+
++index_cardinality;
}
@@ -1458,6 +1470,34 @@ check_spaces(struct tarantool_cfg *conf)
assert(false);
}
}
+
+ /* Check for index field type conflicts */
+ if (max_key_fieldno >= 0) {
+ char *types = alloca(max_key_fieldno + 1);
+ memset(types, UNKNOWN, max_key_fieldno + 1);
+ for (size_t j = 0; space->index[j] != NULL; ++j) {
+ typeof(space->index[j]) index = space->index[j];
+ for (size_t k = 0; index->key_field[k] != NULL; ++k) {
+ typeof(index->key_field[k]) key = index->key_field[k];
+ int f = key->fieldno;
+ if (f == -1) {
+ break;
+ }
+ enum field_data_type t = STR2ENUM(field_data_type, key->type);
+ assert(t != field_data_type_MAX);
+ if (types[f] != t) {
+ if (types[f] == UNKNOWN) {
+ types[f] = t;
+ } else {
+ out_warning(0, "(space = %zu fieldno = %zu) "
+ "index field type mismatch", i, f);
+ return -1;
+ }
+ }
+ }
+
+ }
+ }
}
return 0;
@@ -1602,7 +1642,7 @@ mod_init(void)
/* build secondary indexes */
build_indexes();
- /* enable secondary indexes while now */
+ /* enable secondary indexes now */
secondary_indexes_enabled = true;
title("orphan");
diff --git a/test/box/configuration.result b/test/box/configuration.result
index f4c527e1c6..ded392434a 100644
--- a/test/box/configuration.result
+++ b/test/box/configuration.result
@@ -185,3 +185,10 @@ lua box.cfg.wal_fsync_delay
---
- 0.01
...
+
+# Test field type conflict in keys
+
+tarantool_box -c tarantool_bad_type.cfg
+tarantool_box: can't load config:
+ - (space = 0 fieldno = 0) index field type mismatch
+
diff --git a/test/box/configuration.test b/test/box/configuration.test
index e58585a34a..7eb7cb3bd6 100644
--- a/test/box/configuration.test
+++ b/test/box/configuration.test
@@ -1,5 +1,9 @@
# encoding: tarantool
#
+
+import os
+import sys
+
print """
# Bug #708685:
# Addition of required configuration file options broke backward
@@ -37,6 +41,16 @@ server.deploy("box/tarantool_bug876541.cfg")
# check values
exec admin "lua box.cfg.wal_fsync_delay"
+print """
+# Test field type conflict in keys
+"""
+# stop current server
+server.stop()
+# start server with memcached space conflict
+sys.stdout.push_filter("(/\S+)+/tarantool", "tarantool")
+server.test_option("-c " + os.path.join(os.getcwd(), "box/tarantool_bad_type.cfg"))
+sys.stdout.pop_filter()
+
# restore default server
server.stop()
server.deploy(self.suite_ini["config"])
diff --git a/test/box/tarantool_bad_type.cfg b/test/box/tarantool_bad_type.cfg
new file mode 100644
index 0000000000..3fdfd3044a
--- /dev/null
+++ b/test/box/tarantool_bad_type.cfg
@@ -0,0 +1,24 @@
+slab_alloc_arena = 0.1
+
+pid_file = "box.pid"
+
+logger="cat - >> tarantool.log"
+
+primary_port = 33013
+secondary_port = 33014
+admin_port = 33015
+
+rows_per_wal = 50
+# This is one of the few modifiable settings, change it
+too_long_threshold=2
+
+space[0].enabled = 1
+space[0].index[0].type = "HASH"
+space[0].index[0].unique = 1
+space[0].index[0].key_field[0].fieldno = 0
+space[0].index[0].key_field[0].type = "NUM"
+space[0].index[1].type = "TREE"
+space[0].index[1].unique = 1
+space[0].index[1].key_field[0].fieldno = 0
+space[0].index[1].key_field[0].type = "NUM64"
+
--
GitLab