[tarantool-patches] [PATCH v8 6/6] box: introduce has_json_paths flag in templates
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Jan 23 17:15:10 MSK 2019
On Wed, Jan 16, 2019 at 06:35:56PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index 7ab6e3bf6..dff3aebbd 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -458,11 +458,12 @@ tuple_common_key_parts(const struct tuple *tuple_a, const struct tuple *tuple_b,
> return i;
> }
>
> -template<bool is_nullable, bool has_optional_parts>
> +template<bool is_nullable, bool has_optional_parts, bool has_json_path>
> static inline int
> tuple_compare_slowpath(const struct tuple *tuple_a, const struct tuple *tuple_b,
> struct key_def *key_def)
> {
> + assert(has_json_path == key_def->has_json_paths);
Sometimes you use 'has_json_paths', sometimes 'has_json_path'
(without 's'). Please be consistent.
> assert(!has_optional_parts || is_nullable);
> assert(is_nullable == key_def->is_nullable);
> assert(has_optional_parts == key_def->has_optional_parts);
> @@ -508,10 +509,19 @@ tuple_compare_slowpath(const struct tuple *tuple_a, const struct tuple *tuple_b,
> end = part + key_def->part_count;
>
> for (; part < end; part++) {
> - field_a = tuple_field_by_part_raw(format_a, tuple_a_raw,
> - field_map_a, part);
> - field_b = tuple_field_by_part_raw(format_b, tuple_b_raw,
> - field_map_b, part);
> + if (!has_json_path) {
> + field_a = tuple_field_raw(format_a, tuple_a_raw,
> + field_map_a,
> + part->fieldno);
No need to carry part->fieldno to the next line - it fits.
> + field_b = tuple_field_raw(format_b, tuple_b_raw,
> + field_map_b,
> + part->fieldno);
Ditto.
> + } else {
> + field_a = tuple_field_by_part_raw(format_a, tuple_a_raw,
> + field_map_a, part);
> + field_b = tuple_field_by_part_raw(format_b, tuple_b_raw,
> + field_map_b, part);
> + }
> assert(has_optional_parts ||
> (field_a != NULL && field_b != NULL));
> if (! is_nullable) {
> diff --git a/src/box/tuple_extract_key.cc b/src/box/tuple_extract_key.cc
> index c10ee8ce4..78462908d 100644
> --- a/src/box/tuple_extract_key.cc
> +++ b/src/box/tuple_extract_key.cc
> @@ -133,7 +145,8 @@ tuple_extract_key_slowpath(const struct tuple *tuple,
> * minimize tuple_field_raw() calls.
> */
> for (; i < part_count - 1; i++) {
> - if (!key_def_parts_are_sequential(key_def, i)) {
> + if (!key_def_parts_are_sequential
> + <has_json_paths>(key_def, i)) {
Bad alignment - looks like <has_json_paths>(key_def, i) isn't a part of
'if' condition. Please align by the parenthesis. Here and everywhere
else in the patch.
> diff --git a/src/box/tuple_hash.cc b/src/box/tuple_hash.cc
> index 3486ce11c..8ede29045 100644
> --- a/src/box/tuple_hash.cc
> +++ b/src/box/tuple_hash.cc
> @@ -347,9 +359,18 @@ tuple_hash_slowpath(const struct tuple *tuple, struct key_def *key_def)
> * need of tuple_field
> */
> if (prev_fieldno + 1 != key_def->parts[part_id].fieldno) {
> - struct key_part *part = &key_def->parts[part_id];
> - field = tuple_field_by_part_raw(format, tuple_raw,
> - field_map, part);
> + if (!has_json_paths) {
> + field = tuple_field(tuple,
> + key_def->parts[part_id].
> + fieldno);
Please don't carry .fieldno over to the next line - it looks confusing.
If you didn't remove local variable 'part', you would avoid that.
> + } else {
> + struct key_part *part =
> + &key_def->parts[part_id];
> + field = tuple_field_by_part_raw(format,
> + tuple_raw,
> + field_map,
> + part);
> + }
> }
> if (has_optional_parts && (field == NULL || field >= end)) {
> total_size += tuple_hash_null(&h, &carry);
More information about the Tarantool-patches
mailing list