From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id D4A316EC40; Tue, 17 Aug 2021 21:36:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D4A316EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629225372; bh=CFWJCGyV5nrdhzXX1+i8BD6NmZHAjPC4idmG3Xrsg6Y=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=lj8ErgtFXhBL6i1qySm1YGQA2NlIEQ07QiMCljD4XIjdMq2XX+N3dHfgx/q2XmOEF LSRTOpf8sgrO/PdWMAeaFi9L5jH50qZgvE0924oWxAd/XnVwwDi/GIRsw7QIH2bbrm MuKvaGbbbR3KuhJfQZZ7w19TwbOkh9yAPiiMdLkk= Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 745E36EC40 for ; Tue, 17 Aug 2021 21:36:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 745E36EC40 Received: by smtp59.i.mail.ru with esmtpa (envelope-from ) id 1mG3wX-0004eu-5K; Tue, 17 Aug 2021 21:36:09 +0300 Date: Tue, 17 Aug 2021 21:36:07 +0300 To: Timur Safin via Tarantool-patches Message-ID: <20210817183607.akhqcqos2dmbkbol@esperanza> References: <5e5afbb7249c1dfa56f2695fc6a7140be98fa39d.1629071531.git.tsafin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5e5afbb7249c1dfa56f2695fc6a7140be98fa39d.1629071531.git.tsafin@tarantool.org> X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9D5AC6413C25DCF08CC98B8FCC5CD86F3182A05F538085040B8AC7405BE108DAA573EAC0BCC647E9FF81DFB4667CB5334D36BC93209A843EA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE701173C01F417A2A6EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378629C75C8EFA8C148638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D82F858387D9BCBB2DFF2C7C8922BEE12C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FCF06FF5FC0997E3F8D81D268191BDAD3DBD4B6F7A4D31EC0BEA7A3FFF5B025636D81D268191BDAD3D78DA827A17800CE751275A15C7BC67C5EC76A7562686271EEC990983EF5C03292E808ACE2090B5E14AD6D5ED66289B5259CC434672EE63711DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C3C9BE88FFEDFA497A35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458F0AFF96BAACF4158235E5A14AD4A4A4625E192CAD1D9E79D0B18DC6AC13D9A1CBAD7C56FBD66EB3A X-C1DE0DAB: 0D63561A33F958A5C7D9B27BC4F9F8FFB3B6D7F502EB1F0BC69A71CABA7E4063D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7567C209D01CC1E34B410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D041FB2E16F174C4C7EA9F6DF98329473949A2D523EB88A8C2C250A8BC2CCA24C4C3DC505B2F0F471D7E09C32AA3244C5A9C565F0BEE3C736F106508D085413155E75C8D0ED9F6EE83B48618A63566E0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tr0AI1sJ1HdUA== X-Mailru-Sender: EAC3DA4C40F0802FC8DB1BBF04B7D9F2CA1CBFB9E2A12DBBFA65C0351847EA935B2BEBD93A05268FF0798C444D2DF93A342CD0BA774DB6A9B195EA2AC6D0D44E902A426E4A0C259E9437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 4/8] box, datetime: messagepack support for datetime X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladimir Davydov via Tarantool-patches Reply-To: Vladimir Davydov Cc: v.shpilevoy@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Mon, Aug 16, 2021 at 02:59:38AM +0300, Timur Safin via Tarantool-patches wrote: > Serialize datetime_t as newly introduced MP_EXT type. > It saves 1 required integer field and upto 2 optional > unsigned fields in very compact fashion. > - secs is required field; > - but nsec, offset are both optional; > > * json, yaml serialization formats, lua output mode > supported; > * exported symbols for datetime messagepack size calculations > so they are available for usage on Lua side. > > Part of #5941 > Part of #5946 > --- > extra/exports | 5 +- > src/box/field_def.c | 35 +++--- > src/box/field_def.h | 1 + > src/box/lua/serialize_lua.c | 7 +- > src/box/msgpack.c | 7 +- > src/box/tuple_compare.cc | 20 ++++ > src/lib/core/CMakeLists.txt | 4 +- > src/lib/core/datetime.c | 9 ++ > src/lib/core/datetime.h | 11 ++ > src/lib/core/mp_datetime.c | 189 ++++++++++++++++++++++++++++++ > src/lib/core/mp_datetime.h | 89 ++++++++++++++ > src/lib/core/mp_extension_types.h | 1 + > src/lib/mpstream/mpstream.c | 11 ++ > src/lib/mpstream/mpstream.h | 4 + > src/lua/msgpack.c | 12 ++ > src/lua/msgpackffi.lua | 18 +++ > src/lua/serializer.c | 4 + > src/lua/serializer.h | 2 + > src/lua/utils.c | 1 - > test/unit/datetime.c | 125 +++++++++++++++++++- > test/unit/datetime.result | 115 +++++++++++++++++- > third_party/lua-cjson/lua_cjson.c | 8 ++ > third_party/lua-yaml/lyaml.cc | 6 +- Please add a Lua test checking that serialization (msgpack, yaml, json) works fine. Should be added in this patch. > 23 files changed, 661 insertions(+), 23 deletions(-) > create mode 100644 src/lib/core/mp_datetime.c > create mode 100644 src/lib/core/mp_datetime.h > > diff --git a/src/box/field_def.c b/src/box/field_def.c > index 51acb8025..2682a42ee 100644 > --- a/src/box/field_def.c > +++ b/src/box/field_def.c > @@ -72,6 +72,7 @@ const uint32_t field_mp_type[] = { > /* [FIELD_TYPE_UUID] = */ 0, /* only MP_UUID is supported */ > /* [FIELD_TYPE_ARRAY] = */ 1U << MP_ARRAY, > /* [FIELD_TYPE_MAP] = */ (1U << MP_MAP), > + /* [FIELD_TYPE_DATETIME] = */ 0, /* only MP_DATETIME is supported */ > }; > > const uint32_t field_ext_type[] = { > @@ -83,11 +84,13 @@ 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_ARRAY] = */ 0, > /* [FIELD_TYPE_MAP] = */ 0, > + /* [FIELD_TYPE_DATETIME] = */ 1U << MP_DATETIME, > }; > > const char *field_type_strs[] = { > @@ -104,6 +107,7 @@ const char *field_type_strs[] = { > /* [FIELD_TYPE_UUID] = */ "uuid", > /* [FIELD_TYPE_ARRAY] = */ "array", > /* [FIELD_TYPE_MAP] = */ "map", > + /* [FIELD_TYPE_DATETIME] = */ "datetime", > }; This doesn't belong to this patch. Please split in two: - encoding datetime as msgpack/yaml/json - this patch - indexing of datetime - next patch > > const char *on_conflict_action_strs[] = { > @@ -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 ARRAY MAP DATETIME */ > +/* 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, > +/* ARRAY */ true, false, false, false, false, false, false, false, false, false, false, true, false, false, > +/* MAP */ true, false, false, false, false, false, false, false, false, false, false, false, true, false, > +/* DATETIME */ true, false, false, false, false, false, false, false, true, false, false, false, false, true, > }; > > bool > diff --git a/src/box/field_def.h b/src/box/field_def.h > index c5cfe5e86..120b2a93d 100644 > --- a/src/box/field_def.h > +++ b/src/box/field_def.h > @@ -63,6 +63,7 @@ enum field_type { > FIELD_TYPE_UUID, > FIELD_TYPE_ARRAY, > FIELD_TYPE_MAP, > + FIELD_TYPE_DATETIME, > field_type_MAX > }; > > diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c > index 1f791980f..51855011b 100644 > --- a/src/box/lua/serialize_lua.c > +++ b/src/box/lua/serialize_lua.c > @@ -768,7 +768,7 @@ static int > dump_node(struct lua_dumper *d, struct node *nd, int indent) > { > struct luaL_field *field = &nd->field; > - char buf[FPCONV_G_FMT_BUFSIZE]; > + char buf[FPCONV_G_FMT_BUFSIZE + 8]; Why +8? Better use max(FPCONV_G_FMT_BUFSIZE, ). > int ltype = lua_type(d->L, -1); > const char *str = NULL; > size_t len = 0; > @@ -861,6 +861,11 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent) > str = tt_uuid_str(field->uuidval); > len = UUID_STR_LEN; > break; > + case MP_DATETIME: > + nd->mask |= NODE_QUOTE; > + str = buf; > + len = datetime_to_string(field->dateval, buf, sizeof buf); sizeof(buf) Please fix here and in other places. > + break; > default: > d->err = EINVAL; > snprintf(d->err_msg, sizeof(d->err_msg), > diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc > index 43cd29ce9..9a69f2a72 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); > +} > + Comparators is a part of indexing hence belong to the next patch. > 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; > diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c > index c24a0df82..baf9cc8ae 100644 > --- a/src/lib/core/datetime.c > +++ b/src/lib/core/datetime.c > @@ -165,3 +165,12 @@ datetime_to_string(const struct datetime *date, char *buf, uint32_t len) > } > #undef ADVANCE > > +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); What about offset? Shouldn't you take it into account (convert all dates to UTC)? > +} > diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h > index 964e76fcc..5122e422e 100644 > --- a/src/lib/core/datetime.h > +++ b/src/lib/core/datetime.h > @@ -70,6 +70,17 @@ struct datetime_interval { > int32_t nsec; > }; > > +/** > + * 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/src/lib/core/mp_datetime.c b/src/lib/core/mp_datetime.c > new file mode 100644 > index 000000000..d0a3e562c > --- /dev/null > +++ b/src/lib/core/mp_datetime.c > @@ -0,0 +1,189 @@ > +/* > + * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include "mp_datetime.h" > +#include "msgpuck.h" > +#include "mp_extension_types.h" > + > +/* > + Datetime MessagePack serialization schema is MP_EXT (0xC7 for 1 byte length) > + extension, which creates container of 1 to 3 integers. > + > + +----+---+-----------+====~~~~~~~====+-----~~~~~~~~-------+....~~~~~~~....+ > + |0xC7| 4 |len (uint8)| seconds (int) | nanoseconds (uint) | offset (uint) | > + +----+---+-----------+====~~~~~~~====+-----~~~~~~~~-------+....~~~~~~~....+ > + > + MessagePack extension MP_EXT (0xC7), after 1-byte length, contains: > + > + - signed integer seconds part (required). Depending on the value of > + seconds it may be from 1 to 8 bytes positive or negative integer number; > + > + - [optional] fraction time in nanoseconds as unsigned integer. > + If this value is 0 then it's not saved (unless there is offset field, > + as below); > + > + - [optional] timzeone offset in minutes as unsigned integer. > + If this field is 0 then it's not saved. > + */ > + > +static inline uint32_t > +mp_sizeof_Xint(int64_t n) mp_sizeof_xint - Let's not mix camel-case and underscores. I think this should be put somewhere in a public header so that everyone can use them: mp_utils.h? > +{ > + return n < 0 ? mp_sizeof_int(n) : mp_sizeof_uint(n); > +} > + > +static inline char * > +mp_encode_Xint(char *data, int64_t v) > +{ > + return v < 0 ? mp_encode_int(data, v) : mp_encode_uint(data, v); > +} > + > +static inline int64_t > +mp_decode_Xint(const char **data) > +{ > + switch (mp_typeof(**data)) { > + case MP_UINT: > + return (int64_t)mp_decode_uint(data); assert(value <= INT64_MAX) ? > + case MP_INT: > + return mp_decode_int(data); > + default: > + mp_unreachable(); > + } > + return 0; > +} > + > +static inline uint32_t > +mp_sizeof_datetime_raw(const struct datetime *date) > +{ > + uint32_t sz = mp_sizeof_Xint(date->secs); > + > + // even if nanosecs == 0 we need to output anything > + // if we have non-null tz offset > + if (date->nsec != 0 || date->offset != 0) > + sz += mp_sizeof_Xint(date->nsec); > + if (date->offset) if (date->offset != 0) Please be consistent.