[Tarantool-patches] [PATCH v6 3/5] box, datetime: datetime comparison for indices
unera at tarantool.org
unera at tarantool.org
Thu Aug 19 14:18:17 MSK 2021
Hi!
I looked trhough the patchset.
I also compiled the bracnch and tested by hand.
Example:
tarantool> datetime = require('datetime')
---
...
tarantool> datetime.new('2020-01-02 02:00')
---
- 1970-01-01T00:00Z
...
tarantool> datetime.new('2020-01-02 02:00+0300')
---
- 1970-01-01T00:00Z
...
tarantool> datetime.new('2020-01-02 02:00:11+0300')
---
- 1970-01-01T00:00Z
...
tarantool> datetime.new('2020-01-02T02:00:11+0300')
---
- 1970-01-01T00:00Z
...
The code looks like as broken.
Also my notes:
* tostring has to return datetime string with seconds and timezone
* also the other databases try to avoid to use «Z» symbol in datetime-format, so I thing we should use «+00:00» instead «Z»
Yesterday we approved the following API:
T:add{year=XXX, month=YYY, ...}
T:sub{year=XXX, month=YYY, ...}
T:set{year=XXX, …, tz=’+03:00’}
T:strftime(‘%F %D %z’)
I want to see a test set for the API.
Also we approved the following methods:
T:sec() — 35
T:min() — 12
T:day() — 19
T:wday() — 5
T:yday() — 231
T:year() — 2021
T:month() — 8
T:hour() — 23
T:tz() — ‘+03:00’
It would be nice to see test set for each of the functions.
Also it would be nice to rename ambigouos items «secs» to «epoch» or «timestamp»
Datetime object must have methods to return integer and float timestamp:
T:epoch()
T:timestamp()
>Четверг, 19 августа 2021, 5:58 +03:00 от Timur Safin via Tarantool-patches <tarantool-patches at dev.tarantool.org>:
>
>* storage hints implemented for datetime_t values;
>* proper comparison for indices of datetime type.
>
>Part of #5941
>Part of #5946
>
>@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 | 35 +++++++++-------
> src/box/field_def.h | 1 +
> src/box/tuple_compare.cc | 77 +++++++++++++++++++++++++++++++++++
> src/lib/core/datetime.c | 9 ++++
> src/lib/core/datetime.h | 11 +++++
> test/engine/datetime.result | 77 +++++++++++++++++++++++++++++++++++
> test/engine/datetime.test.lua | 35 ++++++++++++++++
> test/unit/datetime.c | 3 +-
> 8 files changed, 232 insertions(+), 16 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 51acb8025..cd5db942e 100644
>--- a/src/box/field_def.c
>+++ b/src/box/field_def.c
>@@ -70,6 +70,7 @@ const uint32_t field_mp_type[] = {
> (1U << MP_BIN) | (1U << MP_BOOL),
> /* [FIELD_TYPE_DECIMAL] = */ 0, /* only MP_DECIMAL is supported */
> /* [FIELD_TYPE_UUID] = */ 0, /* only MP_UUID is supported */
>+ /* [FIELD_TYPE_DATETIME] = */ 0, /* only MP_DATETIME is supported */
> /* [FIELD_TYPE_ARRAY] = */ 1U << MP_ARRAY,
> /* [FIELD_TYPE_MAP] = */ (1U << MP_MAP),
> };
>@@ -83,9 +84,11 @@ const uint32_t field_ext_type[] = {
> /* [FIELD_TYPE_INTEGER] = */ 0,
> /* [FIELD_TYPE_BOOLEAN] = */ 0,
> /* [FIELD_TYPE_VARBINARY] = */ 0,
>- /* [FIELD_TYPE_SCALAR] = */ (1U << MP_DECIMAL) | (1U << MP_UUID),
>+ /* [FIELD_TYPE_SCALAR] = */ (1U << MP_DECIMAL) | (1U << MP_UUID) |
>+ (1U << MP_DATETIME),
> /* [FIELD_TYPE_DECIMAL] = */ 1U << MP_DECIMAL,
> /* [FIELD_TYPE_UUID] = */ 1U << MP_UUID,
>+ /* [FIELD_TYPE_DATETIME] = */ 1U << MP_DATETIME,
> /* [FIELD_TYPE_ARRAY] = */ 0,
> /* [FIELD_TYPE_MAP] = */ 0,
> };
>@@ -102,6 +105,7 @@ const char *field_type_strs[] = {
> /* [FIELD_TYPE_SCALAR] = */ "scalar",
> /* [FIELD_TYPE_DECIMAL] = */ "decimal",
> /* [FIELD_TYPE_UUID] = */ "uuid",
>+ /* [FIELD_TYPE_DATETIME] = */ "datetime",
> /* [FIELD_TYPE_ARRAY] = */ "array",
> /* [FIELD_TYPE_MAP] = */ "map",
> };
>@@ -128,20 +132,21 @@ field_type_by_name_wrapper(const char *str, uint32_t len)
> * values can be stored in the j type.
> */
> static const bool field_type_compatibility[] = {
>- /* ANY UNSIGNED STRING NUMBER DOUBLE INTEGER BOOLEAN VARBINARY SCALAR DECIMAL UUID ARRAY MAP */
>-/* ANY */ true, false, false, false, false, false, false, false, false, false, false, false, false,
>-/* UNSIGNED */ true, true, false, true, false, true, false, false, true, false, false, false, false,
>-/* STRING */ true, false, true, false, false, false, false, false, true, false, false, false, false,
>-/* NUMBER */ true, false, false, true, false, false, false, false, true, false, false, false, false,
>-/* DOUBLE */ true, false, false, true, true, false, false, false, true, false, false, false, false,
>-/* INTEGER */ true, false, false, true, false, true, false, false, true, false, false, false, false,
>-/* BOOLEAN */ true, false, false, false, false, false, true, false, true, false, false, false, false,
>-/* VARBINARY*/ true, false, false, false, false, false, false, true, true, false, false, false, false,
>-/* SCALAR */ true, false, false, false, false, false, false, false, true, false, false, false, false,
>-/* DECIMAL */ true, false, false, true, false, false, false, false, true, true, false, false, false,
>-/* UUID */ true, false, false, false, false, false, false, false, false, false, true, false, false,
>-/* ARRAY */ true, false, false, false, false, false, false, false, false, false, false, true, false,
>-/* MAP */ true, false, false, false, false, false, false, false, false, false, false, false, true,
>+ /* ANY UNSIGNED STRING NUMBER DOUBLE INTEGER BOOLEAN VARBINARY SCALAR DECIMAL UUID DATETIME ARRAY MAP */
>+/* ANY */ true, false, false, false, false, false, false, false, false, false, false, false, false, false,
>+/* UNSIGNED */ true, true, false, true, false, true, false, false, true, false, false, false, false, false,
>+/* STRING */ true, false, true, false, false, false, false, false, true, false, false, false, false, false,
>+/* NUMBER */ true, false, false, true, false, false, false, false, true, false, false, false, false, false,
>+/* DOUBLE */ true, false, false, true, true, false, false, false, true, false, false, false, false, false,
>+/* INTEGER */ true, false, false, true, false, true, false, false, true, false, false, false, false, false,
>+/* BOOLEAN */ true, false, false, false, false, false, true, false, true, false, false, false, false, false,
>+/* VARBINARY*/ true, false, false, false, false, false, false, true, true, false, false, false, false, false,
>+/* SCALAR */ true, false, false, false, false, false, false, false, true, false, false, false, false, false,
>+/* DECIMAL */ true, false, false, true, false, false, false, false, true, true, false, false, false, false,
>+/* UUID */ true, false, false, false, false, false, false, false, false, false, true, false, false, false,
>+/* DATETIME */ true, false, false, false, false, false, false, false, true, false, false, true, false, false,
>+/* ARRAY */ true, false, false, false, false, false, false, false, false, false, false, false, true, false,
>+/* MAP */ true, false, false, false, false, false, false, false, false, false, false, false, false, true,
> };
>
> bool
>diff --git a/src/box/field_def.h b/src/box/field_def.h
>index c5cfe5e86..619ea19fe 100644
>--- a/src/box/field_def.h
>+++ b/src/box/field_def.h
>@@ -61,6 +61,7 @@ enum field_type {
> FIELD_TYPE_SCALAR,
> FIELD_TYPE_DECIMAL,
> FIELD_TYPE_UUID,
>+ FIELD_TYPE_DATETIME,
> FIELD_TYPE_ARRAY,
> FIELD_TYPE_MAP,
> field_type_MAX
>diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
>index 43cd29ce9..2478498ba 100644
>--- a/src/box/tuple_compare.cc
>+++ b/src/box/tuple_compare.cc
>@@ -36,6 +36,7 @@
> #include "lib/core/decimal.h"
> #include "lib/core/mp_decimal.h"
> #include "uuid/mp_uuid.h"
>+#include "lib/core/mp_datetime.h"
> #include "lib/core/mp_extension_types.h"
>
> /* {{{ tuple_compare */
>@@ -76,6 +77,7 @@ enum mp_class {
> MP_CLASS_STR,
> MP_CLASS_BIN,
> MP_CLASS_UUID,
>+ MP_CLASS_DATETIME,
> MP_CLASS_ARRAY,
> MP_CLASS_MAP,
> mp_class_max,
>@@ -99,6 +101,8 @@ static enum mp_class mp_ext_classes[] = {
> /* .MP_UNKNOWN_EXTENSION = */ mp_class_max, /* unsupported */
> /* .MP_DECIMAL = */ MP_CLASS_NUMBER,
> /* .MP_UUID = */ MP_CLASS_UUID,
>+ /* .MP_ERROR = */ mp_class_max,
>+ /* .MP_DATETIME = */ MP_CLASS_DATETIME,
> };
>
> static enum mp_class
>@@ -390,6 +394,19 @@ mp_compare_uuid(const char *field_a, const char *field_b)
> return memcmp(field_a + 2, field_b + 2, UUID_PACKED_LEN);
> }
>
>+static int
>+mp_compare_datetime(const char *lhs, const char *rhs)
>+{
>+ struct datetime lhs_dt, rhs_dt;
>+ struct datetime *ret;
>+ ret = mp_decode_datetime(&lhs, &lhs_dt);
>+ assert(ret != NULL);
>+ ret = mp_decode_datetime(&rhs, &rhs_dt);
>+ assert(ret != NULL);
>+ (void)ret;
>+ return datetime_compare(&lhs_dt, &rhs_dt);
>+}
>+
> typedef int (*mp_compare_f)(const char *, const char *);
> static mp_compare_f mp_class_comparators[] = {
> /* .MP_CLASS_NIL = */ NULL,
>@@ -398,6 +415,7 @@ static mp_compare_f mp_class_comparators[] = {
> /* .MP_CLASS_STR = */ mp_compare_str,
> /* .MP_CLASS_BIN = */ mp_compare_bin,
> /* .MP_CLASS_UUID = */ mp_compare_uuid,
>+ /* .MP_CLASS_DATETIME=*/ mp_compare_datetime,
> /* .MP_CLASS_ARRAY = */ NULL,
> /* .MP_CLASS_MAP = */ NULL,
> };
>@@ -478,6 +496,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
> return mp_compare_decimal(field_a, field_b);
> 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;
>@@ -518,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;
>@@ -1518,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)) - 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)
>+
> /*
> * HINT_CLASS_BITS should be big enough to store any mp_class value.
> * Note, ((1 << HINT_CLASS_BITS) - 1) is reserved for HINT_NONE.
>@@ -1610,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);
>+}
>+
> static inline uint64_t
> hint_str_raw(const char *s, uint32_t len)
> {
>@@ -1741,6 +1797,17 @@ field_hint_uuid(const char *field)
> return hint_uuid_raw(data);
> }
>
>+static inline hint_t
>+field_hint_datetime(const char *field)
>+{
>+ assert(mp_typeof(*field) == MP_EXT);
>+ int8_t ext_type;
>+ uint32_t len = mp_decode_extl(&field, &ext_type);
>+ assert(ext_type == MP_DATETIME);
>+ struct datetime date;
>+ return hint_datetime(datetime_unpack(&field, len, &date));
>+}
>+
> static inline hint_t
> field_hint_string(const char *field, struct coll *coll)
> {
>@@ -1792,6 +1859,11 @@ field_hint_scalar(const char *field, struct coll *coll)
> }
> case MP_UUID:
> return hint_uuid_raw(field);
>+ case MP_DATETIME:
>+ {
>+ struct datetime date;
>+ return hint_datetime(datetime_unpack(&field, len, &date));
>+ }
> default:
> unreachable();
> }
>@@ -1829,6 +1901,8 @@ field_hint(const char *field, struct coll *coll)
> return field_hint_decimal(field);
> case FIELD_TYPE_UUID:
> return field_hint_uuid(field);
>+ case FIELD_TYPE_DATETIME:
>+ return field_hint_datetime(field);
> default:
> unreachable();
> }
>@@ -1943,6 +2017,9 @@ key_def_set_hint_func(struct key_def *def)
> case FIELD_TYPE_UUID:
> key_def_set_hint_func<FIELD_TYPE_UUID>(def);
> break;
>+ case FIELD_TYPE_DATETIME:
>+ key_def_set_hint_func<FIELD_TYPE_DATETIME>(def);
>+ break;
> default:
> /* Invalid key definition. */
> def->key_hint = NULL;
>diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c
>index 7125090e6..ebc05e29c 100644
>--- a/src/lib/core/datetime.c
>+++ b/src/lib/core/datetime.c
>@@ -123,3 +123,12 @@ datetime_to_string(const struct datetime *date, char *buf, int len)
> return sz;
> }
>
>+int
>+datetime_compare(const struct datetime *lhs, const struct datetime *rhs)
>+{
>+ int result = COMPARE_RESULT(lhs->secs, rhs->secs);
>+ if (result != 0)
>+ return result;
>+
>+ return COMPARE_RESULT(lhs->nsec, rhs->nsec);
>+}
>diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
>index fb537e372..fc5717115 100644
>--- a/src/lib/core/datetime.h
>+++ b/src/lib/core/datetime.h
>@@ -81,6 +81,17 @@ struct datetime_interval {
> */
> #define DT_TO_STRING_BUFSIZE 48
>
>+/*
>+ * Compare arguments of a datetime type
>+ * @param lhs left datetime argument
>+ * @param rhs right datetime argument
>+ * @retval < 0 if lhs less than rhs
>+ * @retval = 0 if lhs and rhs equal
>+ * @retval > 0 if lhs greater than rhs
>+ */
>+int
>+datetime_compare(const struct datetime *lhs, const struct datetime *rhs);
>+
> /**
> * Convert datetime to string using default format
> * @param date source datetime value
>diff --git a/test/engine/datetime.result b/test/engine/datetime.result
>new file mode 100644
>index 000000000..848a0aaec
>--- /dev/null
>+++ b/test/engine/datetime.result
>@@ -0,0 +1,77 @@
>+-- test-run result file version 2
>+env = require('test_run')
>+ | ---
>+ | ...
>+test_run = env.new()
>+ | ---
>+ | ...
>+engine = test_run:get_cfg('engine')
>+ | ---
>+ | ...
>+
>+date = require('datetime')
>+ | ---
>+ | ...
>+
>+_ = box.schema.space.create('T', {engine = engine})
>+ | ---
>+ | ...
>+_ = box.space.T:create_index('pk', {parts={1,'datetime'}})
>+ | ---
>+ | ...
>+
>+box.space.T:insert{date('1970-01-01')}\
>+box.space.T:insert{date('1970-01-02')}\
>+box.space.T:insert{date('1970-01-03')}\
>+box.space.T:insert{date('2000-01-01')}
>+ | ---
>+ | ...
>+
>+o = box.space.T:select{}
>+ | ---
>+ | ...
>+assert(tostring(o[1][1]) == '1970-01-01T00:00Z')
>+ | ---
>+ | - true
>+ | ...
>+assert(tostring(o[2][1]) == '1970-01-02T00:00Z')
>+ | ---
>+ | - true
>+ | ...
>+assert(tostring(o[3][1]) == '1970-01-03T00:00Z')
>+ | ---
>+ | - true
>+ | ...
>+assert(tostring(o[4][1]) == '2000-01-01T00:00Z')
>+ | ---
>+ | - true
>+ | ...
>+
>+for i = 1,16 do\
>+ box.space.T:insert{date.now()}\
>+end
>+ | ---
>+ | ...
>+
>+a = box.space.T:select{}
>+ | ---
>+ | ...
>+err = {}
>+ | ---
>+ | ...
>+for i = 1, #a - 1 do\
>+ if a[i][1] >= a[i+1][1] then\
>+ table.insert(err, {a[i][1], a[i+1][1]})\
>+ break\
>+ end\
>+end
>+ | ---
>+ | ...
>+
>+err
>+ | ---
>+ | - []
>+ | ...
>+box.space.T:drop()
>+ | ---
>+ | ...
>diff --git a/test/engine/datetime.test.lua b/test/engine/datetime.test.lua
>new file mode 100644
>index 000000000..3685e4d4b
>--- /dev/null
>+++ b/test/engine/datetime.test.lua
>@@ -0,0 +1,35 @@
>+env = require('test_run')
>+test_run = env.new()
>+engine = test_run:get_cfg('engine')
>+
>+date = require('datetime')
>+
>+_ = box.schema.space.create('T', {engine = engine})
>+_ = box.space.T:create_index('pk', {parts={1,'datetime'}})
>+
>+box.space.T:insert{date('1970-01-01')}\
>+box.space.T:insert{date('1970-01-02')}\
>+box.space.T:insert{date('1970-01-03')}\
>+box.space.T:insert{date('2000-01-01')}
>+
>+o = box.space.T:select{}
>+assert(tostring(o[1][1]) == '1970-01-01T00:00Z')
>+assert(tostring(o[2][1]) == '1970-01-02T00:00Z')
>+assert(tostring(o[3][1]) == '1970-01-03T00:00Z')
>+assert(tostring(o[4][1]) == '2000-01-01T00:00Z')
>+
>+for i = 1,16 do\
>+ box.space.T:insert{date.now()}\
>+end
>+
>+a = box.space.T:select{}
>+err = {}
>+for i = 1, #a - 1 do\
>+ if a[i][1] >= a[i+1][1] then\
>+ table.insert(err, {a[i][1], a[i+1][1]})\
>+ break\
>+ end\
>+end
>+
>+err
>+box.space.T:drop()
>diff --git a/test/unit/datetime.c b/test/unit/datetime.c
>index c6ba444f5..61ebfe76b 100644
>--- a/test/unit/datetime.c
>+++ b/test/unit/datetime.c
>@@ -281,7 +281,7 @@ mp_datetime_test()
> };
> size_t index;
>
>- plan(68);
>+ plan(85);
> for (index = 0; index < lengthof(tests); index++) {
> struct datetime date = {
> tests[index].secs,
>@@ -303,6 +303,7 @@ mp_datetime_test()
> struct datetime *rc = mp_decode_datetime(&data1, &ret);
> is(rc, &ret, "mp_decode_datetime() return code");
> is(data1, end, "mp_sizeof_uuid() == decoded length");
>+ is(datetime_compare(&date, &ret), 0, "datetime_compare(&date, &ret)");
> }
> check_plan();
> }
>--
>2.29.2
--
Dmitry E. Oboukhov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20210819/6831b40b/attachment.htm>
More information about the Tarantool-patches
mailing list