[tarantool-patches] Re: [PATCH v3 3/4] box: introduce JSON indexes
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Sep 3 13:35:52 MSK 2018
Forgot to send diff:
commit deedcb31040cfbe5b93393e54ec55f3cde5631b8
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date: Sat Sep 1 13:18:52 2018 -0300
Review fixes
diff --git a/src/box/key_def.c b/src/box/key_def.c
index cf1169ea0..ea7380b39 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -544,8 +544,6 @@ static int
key_def_normalize_json_path(struct region *region, struct key_part_def *part,
char **path_extra, uint32_t *path_extra_size)
{
- const char *err_msg = NULL;
-
uint32_t allocated_size = *path_extra_size;
char *path = *path_extra;
@@ -589,7 +587,8 @@ key_def_normalize_json_path(struct region *region, struct key_part_def *part,
uint32_t lexemes = 0;
do {
if (node.type == JSON_PATH_NUM) {
- path += sprintf(path, "[%u]", (uint32_t) node.num);
+ path += sprintf(path, "[%llu]",
+ (unsigned long long) node.num);
} else if (node.type == JSON_PATH_STR) {
path += sprintf(path, "[\"%.*s\"]", node.len, node.str);
} else {
@@ -622,10 +621,11 @@ key_def_normalize_json_path(struct region *region, struct key_part_def *part,
}
return 0;
-error_invalid_json:
- err_msg = tt_sprintf("invalid JSON path '%.*s': path has invalid "
- "structure (error at position %d)", parser.src_len,
- parser.src, rc);
+error_invalid_json: ;
+ const char *err_msg =
+ tt_sprintf("invalid JSON path '%.*s': path has invalid "\
+ "structure (error at position %d)", parser.src_len,
+ parser.src, rc);
diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
part->fieldno + TUPLE_INDEX_BASE, err_msg);
return -1;
@@ -704,19 +704,20 @@ key_def_decode_parts_160(struct key_part_def *parts, uint32_t part_count,
return 0;
}
-const struct key_part *
-key_def_find(const struct key_def *key_def, uint32_t fieldno, const char *path,
- uint32_t path_len)
+bool
+key_def_contains_part(const struct key_def *key_def,
+ const struct key_part *to_find)
{
const struct key_part *part = key_def->parts;
const struct key_part *end = part + key_def->part_count;
for (; part != end; part++) {
- if (part->fieldno == fieldno && part->path_len == path_len &&
- (part->path == NULL ||
- memcmp(part->path, path, path_len) == 0))
- return part;
+ if (part->fieldno == to_find->fieldno &&
+ part->path_len == to_find->path_len &&
+ (part->path == NULL || memcmp(part->path, to_find->path,
+ to_find->path_len) == 0))
+ return true;
}
- return NULL;
+ return false;
}
bool
@@ -725,8 +726,7 @@ key_def_contains(const struct key_def *first, const struct key_def *second)
const struct key_part *part = second->parts;
const struct key_part *end = part + second->part_count;
for (; part != end; part++) {
- if (key_def_find(first, part->fieldno, part->path,
- part->path_len) == NULL)
+ if (! key_def_contains_part(first, part))
return false;
}
return true;
@@ -750,8 +750,7 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
part = second->parts;
end = part + second->part_count;
for (; part != end; part++) {
- if (key_def_find(first, part->fieldno, part->path,
- part->path_len) != NULL)
+ if (key_def_contains_part(first, part))
--new_part_count;
else if (part->path != NULL)
sz += part->path_len + 1;
@@ -788,8 +787,7 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
part = second->parts;
end = part + second->part_count;
for (; part != end; part++) {
- if (key_def_find(first, part->fieldno, part->path,
- part->path_len) != NULL)
+ if (key_def_contains_part(first, part))
continue;
if (part->path != NULL) {
new_def->parts[pos].path = data;
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 693dd5dd0..3387cb6be 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -339,13 +339,10 @@ key_def_decode_parts_160(struct key_part_def *parts, uint32_t part_count,
const char **data, const struct field_def *fields,
uint32_t field_count);
-/**
- * Returns the part in index_def->parts for the specified fieldno.
- * If fieldno is not in index_def->parts returns NULL.
- */
-const struct key_part *
-key_def_find(const struct key_def *key_def, uint32_t fieldno, const char *path,
- uint32_t path_len);
+/** Check if @a key_def contains @a to_find part. */
+bool
+key_def_contains_part(const struct key_def *key_def,
+ const struct key_part *to_find);
/**
* Check if key definition @a first contains all parts of
diff --git a/src/box/tuple.c b/src/box/tuple.c
index 70eda36b3..83bdad1a3 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -168,12 +168,13 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple)
field->is_nullable))
return -1;
/* Check all JSON paths. */
- if (field->childs != NULL &&
- tuple_field_bypass_and_init(field, i, tuple, &pos,
- NULL) != 0)
- return -1;
- if (field->childs == NULL)
+ if (field->childs != NULL) {
+ if (tuple_field_bypass_and_init(field, i, tuple, &pos,
+ NULL) != 0)
+ return -1;
+ } else {
mp_next(&pos);
+ }
}
return 0;
}
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 243c81f2c..51624d243 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -295,7 +295,6 @@ tuple_field_bypass_and_init(const struct tuple_field *field, uint32_t idx,
uint32_t *field_map)
{
assert(offset != NULL);
- int rc = 0;
const char *mp_data = *offset;
const char *valid_type_str = NULL;
const char *err = NULL;
@@ -317,9 +316,9 @@ tuple_field_bypass_and_init(const struct tuple_field *field, uint32_t idx,
const char *raw = *offset;
uint32_t map_idx = 0;
- rc = tuple_field_go_to_key(&raw, ht_record->str,
- (int)ht_record->len,
- &map_idx);
+ int rc = tuple_field_go_to_key(&raw, ht_record->str,
+ (int)ht_record->len,
+ &map_idx);
if (rc != 0 && !leaf->is_nullable) {
err = tt_sprintf("map doesn't contain key "
"'%.*s' defined in index",
@@ -392,10 +391,8 @@ tuple_field_bypass_and_init(const struct tuple_field *field, uint32_t idx,
}
assert(offset != NULL);
if (field_map != NULL &&
- field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
- field_map[field->offset_slot] =
- (uint32_t) (*offset - tuple);
- }
+ field->offset_slot != TUPLE_OFFSET_SLOT_NIL)
+ field_map[field->offset_slot] = (uint32_t) (*offset - tuple);
mp_next(offset);
return 0;
@@ -404,15 +401,13 @@ error_type_mistmatch:
mp_type_strs[type], valid_type_str);
error_invalid_document:
assert(err != NULL);
- do {
- char *data_buff = tt_static_buf();
- mp_snprint(data_buff, TT_STATIC_BUF_LEN, mp_data);
- const char *err_msg =
- tt_sprintf("invalid field %d document content '%s': %s",
- idx + TUPLE_INDEX_BASE, data_buff, err);
- diag_set(ClientError, ER_DATA_STRUCTURE_MISMATCH, err_msg);
- return -1;
- } while (0);
+ char *data_buff = tt_static_buf();
+ mp_snprint(data_buff, TT_STATIC_BUF_LEN, mp_data);
+ const char *err_msg =
+ tt_sprintf("invalid field %d document content '%s': %s",
+ idx + TUPLE_INDEX_BASE, data_buff, err);
+ diag_set(ClientError, ER_DATA_STRUCTURE_MISMATCH, err_msg);
+ return -1;
}
/**
@@ -460,7 +455,7 @@ tuple_format_add_json_path(struct tuple_format *format, const char *path,
path_len, path,
field_type_strs[field->type]);
diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
- node.num, err);
+ node.num, err);
return -1;
}
if (field->is_nullable != is_nullable) {
@@ -568,7 +563,7 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
if (part->path != NULL) {
field->is_key_part = true;
- assert(is_sequential == false);
+ assert(! is_sequential);
struct tuple_field *leaf = NULL;
if (tuple_format_add_json_path(format,
part->path,
@@ -593,12 +588,10 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
* used in tuple_format.
*/
if (field_type1_contains_type2(field->type,
- part->type) &&
- part->path == NULL) {
+ part->type)) {
field->type = part->type;
} else if (! field_type1_contains_type2(part->type,
- field->type) &&
- part->path == NULL) {
+ field->type)) {
const char *name;
int fieldno = part->fieldno + TUPLE_INDEX_BASE;
if (part->fieldno >= field_count) {
@@ -626,10 +619,8 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
* so we don't store an offset for it.
*/
if (field->offset_slot == TUPLE_OFFSET_SLOT_NIL &&
- is_sequential == false && part->fieldno > 0) {
-
+ !is_sequential && part->fieldno > 0)
field->offset_slot = --current_slot;
- }
}
}
@@ -708,10 +699,8 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
for (; part < pend; part++) {
if (part->path != NULL &&
json_path_hash_insert(path_hash, part->path,
- part->path_len, NULL) != 0) {
- json_path_hash_delete(path_hash);
- return NULL;
- }
+ part->path_len, NULL) != 0)
+ goto error;
index_field_count = MAX(index_field_count,
part->fieldno + 1);
}
@@ -742,14 +731,14 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
if (format == NULL) {
diag_set(OutOfMemory, sizeof(struct tuple_format), "malloc",
"tuple format");
- return NULL;
+ goto error;
}
if (dict == NULL) {
assert(space_field_count == 0);
format->dict = tuple_dictionary_new(NULL, 0);
if (format->dict == NULL) {
free(format);
- return NULL;
+ goto error;
}
} else {
format->dict = dict;
@@ -768,6 +757,9 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
format->min_field_count = 0;
format->path_hash = path_hash;
return format;
+error:
+ json_path_hash_delete(path_hash);
+ return NULL;
}
/** Free tuple format resources, doesn't unregister. */
@@ -945,10 +937,6 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
return -1;
}
- /*
- * First field is simply accessible, store offset to it
- * only for JSON path.
- */
uint32_t i = 0;
enum mp_type mp_type;
const struct tuple_field *field = &format->fields[0];
@@ -961,6 +949,10 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
format->field_map_size);
}
if (field->childs == NULL) {
+ /*
+ * First field is simply accessible, do not store
+ * offset to it.
+ */
mp_type = mp_typeof(*pos);
if (key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE,
TUPLE_INDEX_BASE, field->is_nullable))
More information about the Tarantool-patches
mailing list