[Tarantool-patches] [PATCH v5 5/8] box, datetime: datetime comparison for indices
Safin Timur
tsafin at tarantool.org
Wed Aug 18 02:43:34 MSK 2021
On 17.08.2021 15:16, Serge Petrenko wrote:
>
>
> 16.08.2021 02:59, Timur Safin via Tarantool-patches пишет:
>> * storage hints implemented for datetime_t values;
>> * proper comparison for indices of datetime type.
>>
>> Part of #5941
>> Part of #5946
>
>
> Please, add a docbot request stating that it's now possible to store
> datetime values in spaces and create indexed datetime fields.
Will use something like that:
@TarantoolBot document
Title: Storage support for datetime values
It's now possible to store datetime values in spaces and create
indexed datetime fields.
Please refer to https://github.com/tarantool/tarantool/discussions/6244
for more detailed description of a storage schema.
>
>
>> ---
>> src/box/field_def.c | 18 ++++++++
>> src/box/field_def.h | 3 ++
>> src/box/memtx_space.c | 3 +-
>> src/box/tuple_compare.cc | 57 ++++++++++++++++++++++++++
>> src/box/vinyl.c | 3 +-
>> test/engine/datetime.result | 77 +++++++++++++++++++++++++++++++++++
>> test/engine/datetime.test.lua | 35 ++++++++++++++++
>> 7 files changed, 192 insertions(+), 4 deletions(-)
>> create mode 100644 test/engine/datetime.result
>> create mode 100644 test/engine/datetime.test.lua
>>
>> diff --git a/src/box/field_def.c b/src/box/field_def.c
>> index 2682a42ee..97033d0bb 100644
>> --- a/src/box/field_def.c
>> +++ b/src/box/field_def.c
>> @@ -194,3 +194,21 @@ field_type_by_name(const char *name, size_t len)
>> return FIELD_TYPE_ANY;
>> return field_type_MAX;
>> }
>> +
>> +const bool field_type_index_allowed[] =
>> + {
>> + /* [FIELD_TYPE_ANY] = */ false,
>> + /* [FIELD_TYPE_UNSIGNED] = */ true,
>> + /* [FIELD_TYPE_STRING] = */ true,
>> + /* [FIELD_TYPE_NUMBER] = */ true,
>> + /* [FIELD_TYPE_DOUBLE] = */ true,
>> + /* [FIELD_TYPE_INTEGER] = */ true,
>> + /* [FIELD_TYPE_BOOLEAN] = */ true,
>> + /* [FIELD_TYPE_VARBINARY]= */ true,
>> + /* [FIELD_TYPE_SCALAR] = */ true,
>> + /* [FIELD_TYPE_DECIMAL] = */ true,
>> + /* [FIELD_TYPE_UUID] = */ true,
>> + /* [FIELD_TYPE_ARRAY] = */ false,
>> + /* [FIELD_TYPE_MAP] = */ false,
>> + /* [FIELD_TYPE_DATETIME] = */ true,
>> +};
>
>
> You wouldn't need that array if you moved
> FIELD_TYPE_DATETIME above FIELD_TYPE_ARRAY
> in the previous commit.
>
> Please, do so.
Yes, will change order and also move all field support code to this
patch (as Vova recommends).
>
>
>> diff --git a/src/box/field_def.h b/src/box/field_def.h
>> index 120b2a93d..bd02418df 100644
>> --- a/src/box/field_def.h
>> +++ b/src/box/field_def.h
>> @@ -120,6 +120,9 @@ extern const uint32_t field_ext_type[];
>> extern const struct opt_def field_def_reg[];
>> extern const struct field_def field_def_default;
>> +/** helper table for checking allowed indices for types */
>> +extern const bool field_type_index_allowed[];
>> +
>> /**
>> * @brief Field definition
>> * Contains information about of one tuple field.
>> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
>> index b71318d24..1ab16122e 100644
>> --- a/src/box/memtx_space.c
>> +++ b/src/box/memtx_space.c
>> @@ -748,8 +748,7 @@ memtx_space_check_index_def(struct space *space,
>> struct index_def *index_def)
>> /* Check that there are no ANY, ARRAY, MAP parts */
>> for (uint32_t i = 0; i < key_def->part_count; i++) {
>> struct key_part *part = &key_def->parts[i];
>> - if (part->type <= FIELD_TYPE_ANY ||
>> - part->type >= FIELD_TYPE_ARRAY) {
>> + if (!field_type_index_allowed[part->type]) {
>> diag_set(ClientError, ER_MODIFY_INDEX,
>> index_def->name, space_name(space),
>> tt_sprintf("field type '%s' is not supported",
>> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
>> index 9a69f2a72..110017853 100644
>> --- a/src/box/tuple_compare.cc
>> +++ b/src/box/tuple_compare.cc
>> @@ -538,6 +538,8 @@ tuple_compare_field_with_type(const char *field_a,
>> enum mp_type a_type,
>> field_b, b_type);
>> case FIELD_TYPE_UUID:
>> return mp_compare_uuid(field_a, field_b);
>> + case FIELD_TYPE_DATETIME:
>> + return mp_compare_datetime(field_a, field_b);
>> default:
>> unreachable();
>> return 0;
>> @@ -1538,6 +1540,21 @@ func_index_compare_with_key(struct tuple
>> *tuple, hint_t tuple_hint,
>> #define HINT_VALUE_DOUBLE_MAX (exp2(HINT_VALUE_BITS - 1) - 1)
>> #define HINT_VALUE_DOUBLE_MIN (-exp2(HINT_VALUE_BITS - 1))
>> +/**
>> + * We need to squeeze 64 bits of seconds and 32 bits of nanoseconds
>> + * into 60 bits of hint value. The idea is to represent wide enough
>> + * years range, and leave the rest of bits occupied from nanoseconds
>> part:
>> + * - 36 bits is enough for time range of [208BC..4147]
>> + * - for nanoseconds there is left 24 bits, which are MSB part of
>> + * 32-bit value
>> + */
>> +#define HINT_VALUE_SECS_BITS 36
>> +#define HINT_VALUE_NSEC_BITS (HINT_VALUE_BITS - HINT_VALUE_SECS_BITS)
>> +#define HINT_VALUE_SECS_MAX ((1LL << HINT_VALUE_SECS_BITS) - 1)
>
> Am I missing something?
> n bits may store values from (-2^(n-1)) to 2^(n-1)-1
>
> should be (1LL << (HINT_VALUE_SECS_BITS -1)) - 1 ?
>
>
>> +#define HINT_VALUE_SECS_MIN (-(1LL << HINT_VALUE_SECS_BITS))
>
>
>
>
> should be
>
> #define HINT_VALUE_SECS_MIN (-(1LL << (HINT_VALUE_SECS_BITS - 1)))
>
> ?
>
Yes, my definition made sense only when used as a mask (in prior version
of a code). Thus did not take into consideration a sign bit. You
absolutely correct that if seconds are signed then we have lesser number
of bits, and your definitions of HINT_VALUE_SECS_MAX/HINT_VALUE_SECS_MIN
should be used.
>
>> +#define HINT_VALUE_NSEC_SHIFT (sizeof(int) * CHAR_BIT -
>> HINT_VALUE_NSEC_BITS)
>> +#define HINT_VALUE_NSEC_MAX ((1ULL << HINT_VALUE_NSEC_BITS) - 1)
>> +
>> /*
>> * HINT_CLASS_BITS should be big enough to store any mp_class value.
>> * Note, ((1 << HINT_CLASS_BITS) - 1) is reserved for HINT_NONE.
>> @@ -1630,6 +1647,25 @@ hint_uuid_raw(const char *data)
>> return hint_create(MP_CLASS_UUID, val);
>> }
>> +static inline hint_t
>> +hint_datetime(struct datetime *date)
>> +{
>> + /*
>> + * Use at most HINT_VALUE_SECS_BITS from datetime
>> + * seconds field as a hint value, and at MSB part
>> + * of HINT_VALUE_NSEC_BITS from nanoseconds.
>> + */
>> + int64_t secs = date->secs;
>> + int32_t nsec = date->nsec;
>> + uint64_t val = secs <= HINT_VALUE_SECS_MIN ? 0 :
>> + secs - HINT_VALUE_SECS_MIN;
>> + if (val >= HINT_VALUE_SECS_MAX)
>> + val = HINT_VALUE_SECS_MAX;
>> + val <<= HINT_VALUE_NSEC_BITS;
>> + val |= (nsec >> HINT_VALUE_NSEC_SHIFT) & HINT_VALUE_NSEC_MAX;
>> + return hint_create(MP_CLASS_DATETIME, val);
>> +}
>> +
>
> <stripped>
>
Patch increment here small (so far)
------------------------------------
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 110017853..2478498ba 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -1550,8 +1550,8 @@ func_index_compare_with_key(struct tuple *tuple,
hint_t tuple_hint,
*/
#define HINT_VALUE_SECS_BITS 36
#define HINT_VALUE_NSEC_BITS (HINT_VALUE_BITS - HINT_VALUE_SECS_BITS)
-#define HINT_VALUE_SECS_MAX ((1LL << HINT_VALUE_SECS_BITS) - 1)
-#define HINT_VALUE_SECS_MIN (-(1LL << HINT_VALUE_SECS_BITS))
+#define HINT_VALUE_SECS_MAX ((1LL << (HINT_VALUE_SECS_BITS - 1)) - 1)
+#define HINT_VALUE_SECS_MIN (-(1LL << (HINT_VALUE_SECS_BITS - 1)))
#define HINT_VALUE_NSEC_SHIFT (sizeof(int) * CHAR_BIT -
HINT_VALUE_NSEC_BITS)
#define HINT_VALUE_NSEC_MAX ((1ULL << HINT_VALUE_NSEC_BITS) - 1)
------------------------------------
But please see code moves which will be done in the next version of a
patchset, so all field and indices changes will become part of a single
patch.
Regards,
Timur
More information about the Tarantool-patches
mailing list