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 690036EC40; Wed, 18 Aug 2021 17:11:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 690036EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629295865; bh=I5dFdxiYtiYw2UuZuQGFJhJskGPk2GutesRPuCLgBSQ=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=DOHOHmqwtpkXawCWbxuPouo9u89A/ahcCZXO//Z1r/IhXM9XE5IxzRS+V+VS9dDBr DIrU67IP7BdwXlUKT4D4kZ9CcOm/I+HUmDAYLcDOYkj6ocyATfftO9JmsEmIyMkUAc DYfyGL/YxSbbMAo+zcc0K+I8ROVJgyP4N5PO/riQ= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (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 D08E96EC40 for ; Wed, 18 Aug 2021 17:11:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D08E96EC40 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1mGMHW-00074Y-Kv; Wed, 18 Aug 2021 17:11:03 +0300 To: Vladimir Davydov , Timur Safin via Tarantool-patches Cc: v.shpilevoy@tarantool.org References: <8dccf7ee1d6db0f8fd194ab45ae16978b74ec99b.1629071531.git.tsafin@tarantool.org> <20210817170637.x3ilanywpmgbugj5@esperanza> Message-ID: Date: Wed, 18 Aug 2021 17:10:50 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20210817170637.x3ilanywpmgbugj5@esperanza> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD972FF4A7D76DB5E242D14FEF1BD8BF4AC182A05F53808504092F691ED3D10B8FB8CA19757EF8E8CB00F5E3196F4EDAB3B569ACA1C9A7F4386 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7D6964C9E324ABA58EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F97367C191A19EB28638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8EF6C4105D7CA7B7A4F95176C0A6247A5117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE7B96B19DC40933219935A1E27F592749D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE389DDFE3E282F3DD103F1AB874ED89028C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637870CFFD37CCFDD3AEFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458F0AFF96BAACF4158235E5A14AD4A4A4625E192CAD1D9E79D0B18DC6AC13D9A1C243D3E3733263F9F X-C1DE0DAB: 0D63561A33F958A5F3569BF5C29FB94D717F03C29839E1AF3B096E23C8C8EA99D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA757E10A58996CBD514410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3472E5ECC12A9739C15E9A9D9C86280B6C5C2EC0F73F982D6B806CBA18468EA262232D94745411CDAC1D7E09C32AA3244C0BA72751B154B8B34CE41EF09638D05F60759606DA2E136AFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tqCpTYcQPf7Yg== X-Mailru-Sender: 6CA451E36783D721CBEA96CEA26D325D599317AD5D363ACA853729CE847CB34CB7CBEF92542CD7C82F97C478340294DCC77752E0C033A69E0F0C7111264B8915FF1320A92A5534336C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 3/8] lua, datetime: display 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: Safin Timur via Tarantool-patches Reply-To: Safin Timur Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On 17.08.2021 20:06, Vladimir Davydov via Tarantool-patches wrote: > On Mon, Aug 16, 2021 at 02:59:37AM +0300, Timur Safin via > Tarantool-patches wrote: >> * introduced output routine for converting datetime >> to their default output format. >> >> * use this routine for tostring() in datetime.lua >> >> Part of #5941 >> --- >> extra/exports | 1 + >> src/lib/core/datetime.c | 71 ++++++++++++++++++ >> src/lib/core/datetime.h | 9 +++ >> src/lua/datetime.lua | 35 +++++++++ >> test/app-tap/datetime.test.lua | 131 ++++++++++++++++++--------------- >> test/unit/CMakeLists.txt | 2 +- >> test/unit/datetime.c | 61 +++++++++++---- >> 7 files changed, 236 insertions(+), 74 deletions(-) >> >> diff --git a/extra/exports b/extra/exports >> index 80eb92abd..2437e175c 100644 >> --- a/extra/exports >> +++ b/extra/exports >> @@ -152,6 +152,7 @@ datetime_asctime >> datetime_ctime >> datetime_now >> datetime_strftime >> +datetime_to_string >> decimal_unpack >> decimal_from_string >> decimal_unpack >> diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c >> index c48295a6f..c24a0df82 100644 >> --- a/src/lib/core/datetime.c >> +++ b/src/lib/core/datetime.c >> @@ -29,6 +29,8 @@ >> * SUCH DAMAGE. >> */ >> >> +#include >> +#include >> #include >> #include >> >> @@ -94,3 +96,72 @@ datetime_strftime(const struct datetime *date, const > char *fmt, char *buf, >> struct tm *p_tm = datetime_to_tm(date); >> return strftime(buf, len, fmt, p_tm); >> } >> + >> +#define SECS_EPOCH_1970_OFFSET ((int64_t)DT_EPOCH_1970_OFFSET * > SECS_PER_DAY) >> + >> +/* NB! buf may be NULL, and we should handle it gracefully, returning > > /** > * ... > > (according to our coding style) AFAIK external comments (in headers) should be doxygen, but internal comments (in the implementation) may be not in doxygen format. That's internal comment. > >> + * calculated length of output string >> + */ >> +int >> +datetime_to_string(const struct datetime *date, char *buf, uint32_t > len) >> +{ >> +#define ADVANCE(sz) \ >> + if (buf != NULL) { \ >> + buf += sz; \ >> + len -= sz; \ >> + } \ >> + ret += sz; > > Please use SNPRINT helper. Yes, already has have it reimplemented after Serge Petrenko comment. It's a little bit shorter now. Nice! > >> + >> + int offset = date->offset; >> + /* for negative offsets around Epoch date we could get >> + * negative secs value, which should be attributed to >> + * 1969-12-31, not 1970-01-01, thus we first shift >> + * epoch to Rata Die then divide by seconds per day, >> + * not in reverse >> + */ >> + int64_t secs = (int64_t)date->secs + offset * 60 + > SECS_EPOCH_1970_OFFSET; >> + assert((secs / SECS_PER_DAY) <= INT_MAX); >> + dt_t dt = dt_from_rdn(secs / SECS_PER_DAY); >> + >> + int year, month, day, sec, ns, sign; >> + dt_to_ymd(dt, &year, &month, &day); >> + >> + int hour = (secs / 3600) % 24, >> + minute = (secs / 60) % 60; > > Please define on a separate line: > > int hour = ...; > int minute = ...; Done. > >> + sec = secs % 60; > > sec, secs, oh > > Please use better names. Renamed. > >> + ns = date->nsec; >> + >> + int ret = 0; >> + uint32_t sz = snprintf(buf, len, "%04d-%02d-%02dT%02d:%02d", >> + year, month, day, hour, minute); >> + ADVANCE(sz); >> + if (sec || ns) { >> + sz = snprintf(buf, len, ":%02d", sec); >> + ADVANCE(sz); >> + if (ns) { >> + if ((ns % 1000000) == 0) >> + sz = snprintf(buf, len, ".%03d", ns / > 1000000); >> + else if ((ns % 1000) == 0) >> + sz = snprintf(buf, len, ".%06d", ns / > 1000); >> + else >> + sz = snprintf(buf, len, ".%09d", ns); >> + ADVANCE(sz); >> + } >> + } >> + if (offset == 0) { >> + sz = snprintf(buf, len, "Z"); >> + ADVANCE(sz); >> + } >> + else { > > Bad formatting. Should be '} else {'. Fixed. > >> + if (offset < 0) >> + sign = '-', offset = -offset; > > Please don't abuse ','. > >> + else >> + sign = '+'; >> + >> + sz = snprintf(buf, len, "%c%02d:%02d", sign, offset / 60, > offset % 60); >> + ADVANCE(sz); >> + } >> + return ret; >> +} >> +#undef ADVANCE >> + >> diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h >> index 1a8d7e34f..964e76fcc 100644 >> --- a/src/lib/core/datetime.h >> +++ b/src/lib/core/datetime.h >> @@ -70,6 +70,15 @@ struct datetime_interval { >> int32_t nsec; >> }; >> >> +/** >> + * Convert datetime to string using default format >> + * @param date source datetime value >> + * @param buf output character buffer >> + * @param len size ofoutput buffer >> + */ >> +int >> +datetime_to_string(const struct datetime *date, char *buf, uint32_t > len); >> + > > This should be an snprint-like function: buf and len should go first. > This would allow us to use it in conjunction with SNPRINT. Will reorder. [BTW, if we keep argument at the same order then for x64 ABI, with fastcall and upto 6 arguments passed via registers RDI, RSI, RDX, RCX, R8, R9, then compiler may keep arguments in the same registers without any spilling to the stack] > >> /** >> * Convert datetime to string using default asctime format >> * "Sun Sep 16 01:03:52 1973\n\0" >> diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua >> index ce579828f..4d946f194 100644 >> --- a/src/lua/datetime.lua >> +++ b/src/lua/datetime.lua >> @@ -249,6 +249,37 @@ local function datetime_new(obj) >> return datetime_new_dt(dt, secs, frac, offset) >> end >> >> +local function datetime_tostring(o) > > Please add a comment to this function with example output. Documented. > >> + if ffi.typeof(o) == datetime_t then >> + local sz = 48 >> + local buff = ffi.new('char[?]', sz) >> + local len = builtin.datetime_to_string(o, buff, sz) >> + assert(len < sz) >> + return ffi.string(buff) >> + elseif ffi.typeof(o) == interval_t then > > Please define separate functions for interval and datetime. Yes, that will simplify. Part of a corresponding diff below... ---------------------------------------- @@ -373,9 +366,16 @@ local function datetime_new(obj) secs = h * 3600 + m * 60 + s end - return datetime_new_dt(dt, secs, frac, offset) + return datetime_new_dt(dt, secs, nsec, offset) end +--[[ + Convert to text datetime values + + - datetime will use ISO-8601 forat: + 1970-01-01T00:00Z + 2021-08-18T16:57:08.981725+03:00 +]] local function datetime_tostring(o) if ffi.typeof(o) == datetime_t then local sz = 48 @@ -383,7 +383,25 @@ local function datetime_tostring(o) local len = builtin.datetime_to_string(o, buff, sz) assert(len < sz) return ffi.string(buff) - elseif ffi.typeof(o) == interval_years_t then + end +end + +--[[ + Convert to text interval values of different types + + - depending on a values stored there generic interval + values may display in following format: + +12 secs + -23 minutes, 0 seconds + +12 hours, 23 minutes, 1 seconds + -7 days, 23 hours, 23 minutes, 1 seconds + - years will be displayed as + +10 years + - months will be displayed as: + +2 months +]] +local function interval_tostring(o) + if ffi.typeof(o) == interval_years_t then return ('%+d years'):format(o.y) elseif ffi.typeof(o) == interval_months_t then return ('%+d months'):format(o.m) @@ -828,7 +901,7 @@ local datetime_mt = { } local interval_mt = { - __tostring = datetime_tostring, + __tostring = interval_tostring, __serialize = interval_serialize, __eq = datetime_eq, __lt = datetime_lt, @@ -839,7 +912,7 @@ local interval_mt = { } local interval_tiny_mt = { - __tostring = datetime_tostring, + __tostring = interval_tostring, __serialize = interval_serialize, __sub = datetime_sub, __add = datetime_add, ---------------------------------------- > >> + local ts = o.timestamp >> + local sign = '+' >> + >> + if ts < 0 then >> + ts = -ts >> + sign = '-' >> + end >> + >> + if ts < 60 then >> + return ('%s%s secs'):format(sign, ts) >> + elseif ts < 60 * 60 then >> + return ('%+d minutes, %s seconds'):format(o.minutes, ts % > 60) >> + elseif ts < 24 * 60 * 60 then >> + return ('%+d hours, %d minutes, %s seconds'):format( >> + o.hours, o.minutes % 60, ts % 60) >> + else >> + return ('%+d days, %d hours, %d minutes, %s > seconds'):format( >> + o.days, o.hours % 24, o.minutes % 60, ts % 60) >> + end >> + end >> +end >> + >> + >> --[[ >> Basic Extended >> 20121224 2012-12-24 Calendar date (ISO 8601) >> @@ -457,6 +488,7 @@ local function strftime(fmt, o) >> end >> >> local datetime_mt = { >> + __tostring = datetime_tostring, >> __serialize = datetime_serialize, >> __eq = datetime_eq, >> __lt = datetime_lt, >> @@ -466,6 +498,7 @@ local datetime_mt = { >> } >> >> local interval_mt = { >> + __tostring = datetime_tostring, >> __serialize = interval_serialize, >> __eq = datetime_eq, >> __lt = datetime_lt, >> @@ -487,6 +520,8 @@ return setmetatable( >> parse_time = parse_time, >> parse_zone = parse_zone, >> >> + tostring = datetime_tostring, >> + > > I don't think we need this function in the module. Global tostring() is > enough. > ----------------------------------------- @@ -862,15 +935,12 @@ return setmetatable( hours = interval_hours_new, minutes = interval_minutes_new, seconds = interval_seconds_new, - interval = interval_new, parse = parse, parse_date = parse_date, parse_time = parse_time, parse_zone = parse_zone, - tostring = datetime_tostring, - now = local_now, strftime = strftime, asctime = asctime, ----------------------------------------- Deleted. Thanks, Timur