From 0126e55b2f5f75342c549ae225f92d04c510944b Mon Sep 17 00:00:00 2001
From: Magomed Kostoev <m.kostoev@tarantool.org>
Date: Thu, 31 Aug 2023 15:05:02 +0300
Subject: [PATCH] box: get rid of the slowpath comparator unreachable branch
The tuple_compare_slowpath comparator had unreachable
branch under this condition: `key_def->part_count == 1
&& part->fieldno == 0 && (!has_json_paths || part->path
== NULL)`. The condition will never be true in the
function context.
It has been introduced in the commit
c8b87dc7696bff484981c55dff9dad0118ea662c ("Speed up
tuple_compare()."), when there was no sqeuential
comparators, and so it was reasonable at that moment.
But since the sequential comparators had been introduced
in the commit
78102868cd35c0b5dc6fadb614cb7a5c9b074f82 ("Don't store
offsets for sequential multi-parts keys") the condition
became permanently falsy.
There're two ways it can be true:
1. `key_def->part_count == 1 && part->fieldno == 0 &&
!has_json_paths`
2. `key_def->part_count == 1 && part->fieldno == 0 &&
has_json_paths && part->path == NULL`
Condition 1 will never happen because if we have a key
starting from `fieldno = 0` with any part count
following and without JSON paths, then it is compared
using `tuple_compare_sequential` instead.
Proof:
1. The key is sequential if and only if it does not have
JSON paths and for all key parts
`index_def->parts[i].fieldno == i`.
2. The `key_def->part_count == 1 && part->fieldno == 0
&& !has_json_paths` condition fully satisfies this
condition.
3. The `tuple_compare_slowpath` is only set as a
comparator if the key is not sequential. Proof:
The only places the comparator is set are:
- `key_def_set_compare_func_fast` under the
`!is_sequential` condition.
- `key_def_set_compare_func_plain` under the
`!key_def_is_sequential` condition.
- `key_def_set_compare_func_json`, which is only
called under `def->has_json_paths` condition, which
conflicts with the `!has_json_paths` condition.
Condition 2: has JSON path means we have `path`
parameter in the index definition, but the following
condition requires the path to be `NULL`, which is
impossible if the part count is 1.
Proof:
1. A key has JSON paths if and only if one of its parts'
path does not equal NULL.
2. If key part count is one and the only part has path,
then the `part->path == NULL` part fails.
3. If key part count is one and the only part does not
have JSON path then the key has no JSON paths, goto
Condition 1.
Closes #8900
NO_DOC=dead code elimination
NO_TEST=dead code elimination
NO_CHANGELOG=dead code elimination
---
src/box/tuple_compare.cc | 26 +-------------------------
1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index b727751847..3cc60f63da 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -589,34 +589,10 @@ tuple_compare_slowpath(struct tuple *tuple_a, hint_t tuple_a_hint,
int rc = 0;
if (!is_multikey && (rc = hint_cmp(tuple_a_hint, tuple_b_hint)) != 0)
return rc;
+ bool was_null_met = false;
struct key_part *part = key_def->parts;
const char *tuple_a_raw = tuple_data(tuple_a);
const char *tuple_b_raw = tuple_data(tuple_b);
- if (key_def->part_count == 1 && part->fieldno == 0 &&
- (!has_json_paths || part->path == NULL)) {
- /*
- * First field can not be optional - empty tuples
- * can not exist.
- */
- assert(!has_optional_parts);
- mp_decode_array(&tuple_a_raw);
- mp_decode_array(&tuple_b_raw);
- if (! is_nullable) {
- return tuple_compare_field(tuple_a_raw, tuple_b_raw,
- part->type, part->coll);
- }
- enum mp_type a_type = mp_typeof(*tuple_a_raw);
- enum mp_type b_type = mp_typeof(*tuple_b_raw);
- if (a_type == MP_NIL)
- return b_type == MP_NIL ? 0 : -1;
- else if (b_type == MP_NIL)
- return 1;
- return tuple_compare_field_with_type(tuple_a_raw, a_type,
- tuple_b_raw, b_type,
- part->type, part->coll);
- }
-
- bool was_null_met = false;
struct tuple_format *format_a = tuple_format(tuple_a);
struct tuple_format *format_b = tuple_format(tuple_b);
const uint32_t *field_map_a = tuple_field_map(tuple_a);
--
GitLab