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 BAAB26EC40; Wed, 18 Aug 2021 13:06:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BAAB26EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629281207; bh=8T9OWVBsLfP0ncfPlaYPjuqS3An8ZC2BBIAqAQLiVzU=; 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=l8Chjb+29J6yGsyFFWnyC0jlgxW5ZVn1LX1oruHMuLu52HEeUdZ/O/EpjF3VgwsDJ NeJF4evqWGrCjzBNXVMZMeujQEzhse15QoLxS/Z8tZ/If4PtPFo2uwbS1FOjOslypE Wlu7nnZoowc0zn1S7phBaM4gpuCIFlntRds+WPiU= Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 686666EC40 for ; Wed, 18 Aug 2021 13:06:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 686666EC40 Received: by smtp63.i.mail.ru with esmtpa (envelope-from ) id 1mGIT6-0002wv-JD; Wed, 18 Aug 2021 13:06:45 +0300 To: Vladimir Davydov , Timur Safin via Tarantool-patches Cc: v.shpilevoy@tarantool.org References: <20210817165243.kumsj3x2ia5pijme@esperanza> <512da66c-1b3f-c671-b653-96726293a5f2@tarantool.org> Message-ID: <45ceb01f-a9d4-063f-ddbb-40a48434ff7f@tarantool.org> Date: Wed, 18 Aug 2021 13:06:31 +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: <512da66c-1b3f-c671-b653-96726293a5f2@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD91BCCB18F2C129F87F36E61E9E4584E9D182A05F5380850406EF254B83DDC7FD1B46436B633329F8B8D14E9CEA58225D0F3F1D7BA938659B3 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE716FAD50E497B9C14EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379C6BD13E0D9523298638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8A25797BC7D86E14E0F53F43897B61505117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE4B6963042765DA4B7C6FCE95544A9834D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE3A7DFDF579AB090EF03F1AB874ED89028C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637870CFFD37CCFDD3AEFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505412D5E1ACC0BA668BBEEB48C39E9B5BD X-C1DE0DAB: 0D63561A33F958A5A6385F0A5F02A62E71C87350FFE5516B5897F745BB35B9A9D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA757E10A58996CBD514410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346E67BC984B8ABCB509E87B8F0B109DC6E536E403039BE5995E7A5D1C1815EA71F92A1CEE0425CCCF1D7E09C32AA3244C0966E8609E3CC36A8B925DB83AE6EC56725D5B54B2FE4575FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tpQreQH7nziCQ== X-Mailru-Sender: B5B6A6EBBD94DAD872610222F811036BE96112C9F4C4BCE6DB89C94ABC6E04EEE2EA530725DD0AA61EC9E4A2C82A33BC8C24925A86E657CE0C70AEE3C9A96FBAB3D7EE8ED63280BE112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 2/8] lua: built-in module 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" Forgot to insert an inlined incremental changes introduced: ----------------------------------------------------------- diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c index 6d92645f7..4cd93b264 100644 --- a/src/lib/core/datetime.c +++ b/src/lib/core/datetime.c @@ -12,6 +12,11 @@ #include "trivia/util.h" #include "datetime.h" +/* + * Given the seconds from Epoch (1970-01-01) we calculate date + * since Rata Die (0001-01-01). + * DT_EPOCH_1970_OFFSET is the distance in days from Rata Die to Epoch. + */ static int local_dt(int64_t secs) { diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h index b8d179600..278ae6a87 100644 --- a/src/lib/core/datetime.h +++ b/src/lib/core/datetime.h @@ -111,9 +111,25 @@ datetime_to_string(const struct datetime *date, char *buf, int len); char * datetime_asctime(const struct datetime *date, char *buf); +/** + * Convert datetime to string using default ctime format, using + * local timezone for representation. + * "Sun Sep 16 01:03:52 1973\n\0" + * Wrapper around reenterable ctime_r() version of POSIX function + * @param date source datetime value + * @sa datetime_asctime + */ char * datetime_ctime(const struct datetime *date, char *buf); +/** + * Convert datetime to string using default format provided + * Wrapper around standard strftime() function + * @param date source datetime value + * @param fmt format + * @param buf output buffer + * @param len size of output buffer + */ size_t datetime_strftime(const struct datetime *date, const char *fmt, char *buf, uint32_t len); diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua index ae22c598d..a0fa29167 100644 --- a/src/lua/datetime.lua +++ b/src/lua/datetime.lua @@ -16,51 +16,49 @@ local ffi = require('ffi') -- dt_core.h definitions ffi.cdef [[ - typedef int dt_t; - dt_t tnt_dt_from_rdn (int n); - dt_t tnt_dt_from_ymd (int y, int m, int d); +typedef int dt_t; + +dt_t tnt_dt_from_rdn (int n); +dt_t tnt_dt_from_ymd (int y, int m, int d); +int tnt_dt_rdn (dt_t dt); - int tnt_dt_rdn (dt_t dt); ]] -- dt_arithmetic.h definitions ffi.cdef [[ - typedef enum { - DT_EXCESS, - DT_LIMIT, - DT_SNAP - } dt_adjust_t; - - dt_t tnt_dt_add_years (dt_t dt, int delta, dt_adjust_t adjust); - dt_t tnt_dt_add_quarters (dt_t dt, int delta, dt_adjust_t adjust); - dt_t tnt_dt_add_months (dt_t dt, int delta, dt_adjust_t adjust); + +typedef enum { + DT_EXCESS, + DT_LIMIT, + DT_SNAP +} dt_adjust_t; + +dt_t tnt_dt_add_years (dt_t dt, int delta, dt_adjust_t adjust); +dt_t tnt_dt_add_quarters (dt_t dt, int delta, dt_adjust_t adjust); +dt_t tnt_dt_add_months (dt_t dt, int delta, dt_adjust_t adjust); + ]] -- dt_parse_iso.h definitions ffi.cdef [[ - size_t tnt_dt_parse_iso_date (const char *str, size_t len, dt_t *dt); - size_t tnt_dt_parse_iso_time (const char *str, size_t len, int *sod, int *nsec); - size_t tnt_dt_parse_iso_zone_lenient (const char *str, size_t len, int *offset); + +size_t tnt_dt_parse_iso_date (const char *str, size_t len, dt_t *dt); +size_t tnt_dt_parse_iso_time (const char *str, size_t len, int *sod, int *nsec); +size_t tnt_dt_parse_iso_zone_lenient(const char *str, size_t len, int *offset); + ]] -- Tarantool functions - datetime.c ffi.cdef [[ - int - datetime_to_string(const struct datetime * date, char *buf, int len); - - char * - datetime_asctime(const struct datetime *date, char *buf); - char * - datetime_ctime(const struct datetime *date, char *buf); +int datetime_to_string(const struct datetime * date, char *buf, int len); +char *datetime_asctime(const struct datetime *date, char *buf); +char *datetime_ctime(const struct datetime *date, char *buf); +size_t datetime_strftime(const struct datetime *date, const char *fmt, char *buf, + uint32_t len); +void datetime_now(struct datetime *now); - size_t - datetime_strftime(const struct datetime *date, const char *fmt, char *buf, - uint32_t len); - - void - datetime_now(struct datetime * now); ]] local builtin = ffi.C @@ -92,22 +90,17 @@ local interval_months_t = ffi.typeof('struct interval_months') local interval_years_t = ffi.typeof('struct interval_years') local function is_interval(o) - return type(o) == 'cdata' and - (ffi.istype(interval_t, o) or - ffi.istype(interval_months_t, o) or - ffi.istype(interval_years_t, o)) + return ffi.istype(interval_t, o) or + ffi.istype(interval_months_t, o) or + ffi.istype(interval_years_t, o) end local function is_datetime(o) - return type(o) == 'cdata' and ffi.istype(datetime_t, o) + return ffi.istype(datetime_t, o) end local function is_date_interval(o) - return type(o) == 'cdata' and - (ffi.istype(datetime_t, o) or - ffi.istype(interval_t, o) or - ffi.istype(interval_months_t, o) or - ffi.istype(interval_years_t, o)) + return is_datetime(o) or is_interval(o) end local function interval_new() @@ -230,9 +223,8 @@ local function normalize_nsec(secs, nsec) end local function datetime_cmp(lhs, rhs) - if not is_date_interval(lhs) or - not is_date_interval(rhs) then - return nil + if not is_date_interval(lhs) or not is_date_interval(rhs) then + return nil end local sdiff = lhs.secs - rhs.secs return sdiff ~= 0 and sdiff or (lhs.nsec - rhs.nsec) @@ -271,12 +263,12 @@ local function datetime_new_raw(secs, nsec, offset) return dt_obj end -local function datetime_new_dt(dt, secs, frac, offset) +local function datetime_new_dt(dt, secs, fraction, offset) local epochV = dt ~= nil and (builtin.tnt_dt_rdn(dt) - DT_EPOCH_1970_OFFSET) * SECS_PER_DAY or 0 - local secsV = secs ~= nil and secs or 0 - local fracV = frac ~= nil and frac or 0 - local ofsV = offset ~= nil and offset or 0 + local secsV = secs or 0 + local fracV = fraction or 0 + local ofsV = offset or 0 return datetime_new_raw(epochV + secsV - ofsV * 60, fracV, ofsV) end @@ -319,7 +311,7 @@ local function datetime_new(obj) local h = 0 local m = 0 local s = 0 - local frac = 0 + local nsec = 0 local hms = false local offset = 0 @@ -348,14 +340,15 @@ local function datetime_new(obj) hms = true elseif key == 'sec' or key == 'second' then check_range(value, {0, 60}, key) - s, frac = math_modf(value) - frac = frac * 1e9 -- convert fraction to nanoseconds + s, nsec = math_modf(value) + nsec = nsec * 1e9 -- convert fraction to nanoseconds hms = true elseif key == 'tz' then - -- tz offset in minutes + -- tz offset in minutes check_range(value, {0, 720}, key) offset = value - elseif key == 'isdst' or key == 'wday' or key =='yday' then -- luacheck: ignore 542 + elseif key == 'isdst' or key == 'wday' or + key == 'yday' then -- luacheck: ignore 542 -- ignore unused os.date attributes else error(('unknown attribute %s'):format(key), 2) @@ -373,7 +366,7 @@ 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 local function datetime_tostring(o) @@ -537,6 +530,9 @@ end 2012359 2012-359 Ordinal date (ISO 8601) 2012W521 2012-W52-1 Week date (ISO 8601) 2012Q485 2012-Q4-85 Quarter date + + Returns pair of constructed datetime object, and length of string + which has been accepted by parser. ]] local function parse_date(str) @@ -555,6 +551,9 @@ end T123045,123456789 T12:30:45,123456789 The time designator [T] may be omitted. + + Returns pair of constructed datetime object, and length of string + which has been accepted by parser. ]] local function parse_time(str) check_str("datetime.parse_time()") @@ -572,6 +571,9 @@ end -hh N/A +hhmm +hh:mm -hhmm -hh:mm + + Returns pair of constructed datetime object, and length of string + which has been accepted by parser. ]] local function parse_zone(str) check_str("datetime.parse_zone()") @@ -581,13 +583,14 @@ local function parse_zone(str) tonumber(len) end - --[[ aggregated parse functions assumes to deal with date T time time_zone at once date [T] time [ ] time_zone + + Returns constructed datetime object. ]] local function parse(str) check_str("datetime.parse()") @@ -601,7 +604,7 @@ local function parse(str) str = str:sub(tonumber(n) + 1) - local ch = str:sub(1,1) + local ch = str:sub(1, 1) if ch:match('[Tt ]') == nil then return datetime_new_dt(dt_) end @@ -623,7 +626,7 @@ local function parse(str) str = str:sub(tonumber(n) + 1) - if str:sub(1,1) == ' ' then + if str:sub(1, 1) == ' ' then str = str:sub(2) end @@ -637,6 +640,11 @@ local function parse(str) return datetime_new_dt(dt_, sp_, fp_, offset[0]) end +--[[ + Dispatch function to create datetime from string or table. + Creates default timeobject (pointing to Epoch date) if + called without arguments. +]] local function datetime_from(o) if o == nil or type(o) == 'table' then return datetime_new(o) @@ -645,6 +653,10 @@ local function datetime_from(o) end end +--[[ + Create datetime object representing current time using microseconds + platform timer and local timezone information. +]] local function local_now() local d = datetime_new_raw(0, 0, 0) builtin.datetime_now(d) @@ -720,9 +732,14 @@ local function interval_increment(self, o, direction) return self end --- Change the time-zone to the provided target_offset --- Time `.secs`/`.nsec` are always UTC normalized, we need only to --- reattribute object with different `.offset` +--[[ + Create new object with modified to target_offset time-zone. + If target timezone does not differ to the original one - we + return original object unmodified. + + Time `.secs`/`.nsec` are always UTC normalized, we need only to + reattribute object with different `.offset` +]] local function datetime_to_tz(self, tgt_ofs) if self.offset == tgt_ofs then return self @@ -738,6 +755,29 @@ local function datetime_to_tz(self, tgt_ofs) return datetime_new_raw(self.secs, self.nsec, tgt_ofs) end +--[[ + Provide a set of accessor to datetime attributes: + - .epoch or .unixtime - return timestamp as seconds represented as double + typed value, and measured since Epoch; + - .ts or .timestamp - the same timestamp, but with extended precision and + fraction part; + + All accessors below convert time distance from Epoch date to different + units: + - .ns or .nanoseconds - seconds and fraction converted to nanoseconds; + - .us or .microseconds - seconds and fraction converted to microseconds; + - .ms or .milliseconds - seconds and fraction converted to milliseconds; + - .s or .seconds - seconds with extended precision and fraction part; + - .m or .min or minutes - minutes with extended precision; + - .hr or .hours - hours with extended precision; + - .d or .days - days with extended precision; + + .add or .sub methods provide friendlier way for interval arithmetics; + + .to_utc and .to_tz allows to create equivalent datetime object but in + particular timezone or as UTC. Time moment remains the same, only time-zone + information changed, thus textual representation will be impacted also. +]] local function datetime_index(self, key) if key == 'epoch' or key == 'unixtime' then return self.secs @@ -778,6 +818,13 @@ local function datetime_index(self, key) end end +--[[ + Allow to change datetime attributes via: + - .epoch or .unixtime - change datetime via provided interval to Unix Epoch; + - .ts or .timestamp - change both seconds and nanoseconds fields via given + timestamp with extended precision. Timestamp fraction part changes + nanoseconds information. +]] local function datetime_newindex(self, key, value) if key == 'epoch' or key == 'unixtime' then self.secs = value @@ -794,17 +841,18 @@ end -- sizeof("Wed Jun 30 21:49:08 1993\n") local buf_len = 26 +local asctime_buffer = ffi.new('char[?]', buf_len) local function asctime(o) check_date(o, "datetime:asctime()") - local buf = ffi.new('char[?]', buf_len) - return ffi.string(builtin.datetime_asctime(o, buf)) + return ffi.string(builtin.datetime_asctime(o, asctime_buffer)) end +local ctime_buffer = ffi.new('char[?]', buf_len) + local function ctime(o) check_date(o, "datetime:ctime()") - local buf = ffi.new('char[?]', buf_len) - return ffi.string(builtin.datetime_ctime(o, buf)) + return ffi.string(builtin.datetime_ctime(o, ctime_buffer)) end local function strftime(fmt, o) ----------------------------------------------------------- On 18.08.2021 13:03, Safin Timur wrote: > On 17.08.2021 19:52, Vladimir Davydov via Tarantool-patches wrote: >> On Mon, Aug 16, 2021 at 02:59:36AM +0300, Timur Safin via >> Tarantool-patches wrote: >>> diff --git a/extra/exports b/extra/exports >>> index 9eaba1282..80eb92abd 100644 >>> --- a/extra/exports >>> +++ b/extra/exports >>> @@ -148,8 +148,34 @@ csv_feed >>>   csv_iterator_create >>>   csv_next >>>   csv_setopt >>> +datetime_asctime >>> +datetime_ctime >>> +datetime_now >>> +datetime_strftime >>> +decimal_unpack >> >> decimal_unpack? > > Yes, bad copy paste. Supposed to become datetime_unpack. Has been > changed later in the patchset. But now, after Serge complain, I've > reshuffled and fixed original patch, not later. > >> >>>   decimal_from_string >>>   decimal_unpack >>> +tnt_dt_dow >>> +tnt_dt_from_rdn >>> +tnt_dt_from_struct_tm >>> +tnt_dt_from_yd >>> +tnt_dt_from_ymd >>> +tnt_dt_from_yqd >>> +tnt_dt_from_ywd >>> +tnt_dt_parse_iso_date >>> +tnt_dt_parse_iso_time_basic >>> +tnt_dt_parse_iso_time_extended >>> +tnt_dt_parse_iso_time >>> +tnt_dt_parse_iso_zone_basic >>> +tnt_dt_parse_iso_zone_extended >>> +tnt_dt_parse_iso_zone_lenient >>> +tnt_dt_parse_iso_zone >>> +tnt_dt_rdn >>> +tnt_dt_to_struct_tm >>> +tnt_dt_to_yd >>> +tnt_dt_to_ymd >>> +tnt_dt_to_yqd >>> +tnt_dt_to_ywd >>>   error_ref >>>   error_set_prev >>>   error_unref > > ... > >>> new file mode 100644 >>> index 000000000..c48295a6f >>> --- /dev/null >>> +++ b/src/lib/core/datetime.c >>> @@ -0,0 +1,96 @@ >>> +/* >>> + * 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 >>> +#include >>> + >>> +#include "trivia/util.h" >>> +#include "datetime.h" >>> + >>> +static int >>> +local_dt(int64_t secs) >>> +{ >>> +    return dt_from_rdn((int)(secs / SECS_PER_DAY) + >> DT_EPOCH_1970_OFFSET); >>> +} >> >> I don't understand what this function does. Please add a comment. > > Good point. Added comment. > >> >>> + >>> +static struct tm * >>> +datetime_to_tm(const struct datetime *date) >>> +{ >>> +    static struct tm tm; >>> + >>> +    memset(&tm, 0, sizeof(tm)); >>> +    int64_t secs = date->secs; >>> +    dt_to_struct_tm(local_dt(secs), &tm); >>> + >>> +    int seconds_of_day = (int64_t)date->secs % SECS_PER_DAY; >>> +    tm.tm_hour = (seconds_of_day / 3600) % 24; >>> +    tm.tm_min = (seconds_of_day / 60) % 60; >>> +    tm.tm_sec = seconds_of_day % 60; >> >> To make the code easier for understanding, please define and use >> constants here and everywhere else in this patch: HOURS_PER_DAY, >> MINUTES_PER_HOUR, NSECS_PER_USEC, etc. >> >>> + >>> +    return &tm; >>> +} >>> + >>> +void >>> +datetime_now(struct datetime *now) >>> +{ >>> +    struct timeval tv; >>> +    gettimeofday(&tv, NULL); >>> +    now->secs = tv.tv_sec; >>> +    now->nsec = tv.tv_usec * 1000; >>> + >>> +    time_t now_seconds; >>> +    time(&now_seconds); >> >> Can't you use tv.tv_sec here? > > Nope. :) > > Actually I don't want to know value returned from time() - all I need is > to eventually call localtime[_r]() to get local timezone. If you know > simpler way to determine local timezone without time, then please give > me know. > > >>> new file mode 100644 >>> index 000000000..1a8d7e34f >>> --- /dev/null >>> +++ b/src/lib/core/datetime.h >>> @@ -0,0 +1,95 @@ >>> +#pragma once >>> +/* >>> + * 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 >>> +#include >>> +#include >>> +#include "c-dt/dt.h" >>> + >>> +#if defined(__cplusplus) >>> +extern "C" >>> +{ >>> +#endif /* defined(__cplusplus) */ >>> + >>> +#ifndef SECS_PER_DAY >>> +#define SECS_PER_DAY          86400 >>> +#define DT_EPOCH_1970_OFFSET  719163 >> >> I don't understand what this constant stores. Please add a comment. > > I've documented them after Serge request this way: > > ------------------------------------------------------ > /** >  * We count dates since so called "Rata Die" date >  * January 1, 0001, Monday (as Day 1). >  * But datetime structure keeps seconds since >  * Unix "Epoch" date: >  * Unix, January 1, 1970, Thursday >  * >  * The difference between Epoch (1970-01-01) >  * and Rata Die (0001-01-01) is 719163 days. >  */ > > #ifndef SECS_PER_DAY > #define SECS_PER_DAY          86400 > #define DT_EPOCH_1970_OFFSET  719163 > #endif > > /** >  * c-dt library uses int as type for dt value, which >  * represents the number of days since Rata Die date. >  * This implies limits to the number of seconds we >  * could safely store in our structures and then safely >  * pass to c-dt functions. >  * >  * So supported ranges will be >  * - for seconds [-185604722870400 .. 185480451417600] >  * - for dates   [-5879610-06-22T00:00Z .. 5879611-07-11T00:00Z] >  */ > #define MAX_DT_DAY_VALUE (int64_t)INT_MAX > #define MIN_DT_DAY_VALUE (int64_t)INT_MIN > #define SECS_EPOCH_1970_OFFSET     \ >     ((int64_t)DT_EPOCH_1970_OFFSET * SECS_PER_DAY) > #define MAX_EPOCH_SECS_VALUE    \ >     (MAX_DT_DAY_VALUE * SECS_PER_DAY - SECS_EPOCH_1970_OFFSET) > #define MIN_EPOCH_SECS_VALUE    \ >     (MIN_DT_DAY_VALUE * SECS_PER_DAY - SECS_EPOCH_1970_OFFSET) > ------------------------------------------------------ > > This is more than you probably were asking for at the moment, but bear > with me - we will need those MAX_EPOCH_SECS_VALUE/MIN_EPOCH_SECS_VALUE > in the messagepack serialization/deserialization code. > >> >>> +#endif >>> + >>> +/** >>> + * Full datetime structure representing moments >>> + * since Unix Epoch (1970-01-01). >>> + * Time is kept normalized to UTC, time-zone offset >>> + * is informative only. >>> + */ >>> +struct datetime { >>> +    /** seconds since epoch */ >>> +    double secs; >> >> Please add a comment explaining why you use 'double' instead of >> an integer type. >> >>> +    /** nanoseconds if any */ >>> +    int32_t nsec; >> >> Why 'nsec', but 'secs'? This is inconsistent. Should be 'nsec' and 'sec' >> or 'nsecs' and 'secs'. > > Good point, will rname to use 'secs' and 'nsecs'. But rename will be > massive, so no incremental patch so far. > >> >>> +    /** offset in minutes from UTC */ >>> +    int32_t offset; >> >> Why do you use int32_t instead of int for these two members? > > Because I need signed integer larger than 16-bit, but less or equal to > 32-bit signed value. Simple int is not exactly what I want, depending on > platfrom it could be 4byte or 8bytes long. For example, ILP32, and LP64 > do have 4-bytes long int, but there are(were) ILP64 hardware platforms > (like Cray) where int was as 64-bit as long or pointer. > > int is not as specific as int32_t in this particular case. > >> >> Same comments for the datetime_interval struct. >> >>> +}; >>> + >>> +/** >>> + * Date/time interval structure >>> + */ >>> +struct datetime_interval { >>> +    /** relative seconds delta */ >>> +    double secs; >>> +    /** nanoseconds delta */ >>> +    int32_t nsec; >>> +}; >>> + >>> +/** >>> + * Convert datetime to string using default asctime format >>> + * "Sun Sep 16 01:03:52 1973\n\0" >>> + * Wrapper around reenterable asctime_r() version of POSIX function >>> + * @param date source datetime value >>> + * @sa datetime_ctime >>> + */ >>> +char * >>> +datetime_asctime(const struct datetime *date, char *buf); >>> + >>> +char * >>> +datetime_ctime(const struct datetime *date, char *buf); >>> + >>> +size_t >>> +datetime_strftime(const struct datetime *date, const char *fmt, char >> *buf, >>> +          uint32_t len); >>> + >>> +void >>> +datetime_now(struct datetime * now); >> >> Extra space after '*'. > > Fixed. > >> >> Please add comments to all these functions, like you did for >> datetime_asctime. > > Added. > >> >>> + >>> +#if defined(__cplusplus) >>> +} /* extern "C" */ >>> +#endif /* defined(__cplusplus) */ >>> diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua >>> new file mode 100644 >>> index 000000000..ce579828f >>> --- /dev/null >>> +++ b/src/lua/datetime.lua >>> @@ -0,0 +1,500 @@ >>> +local ffi = require('ffi') >>> + >>> +ffi.cdef [[ >>> + >>> +    /* >>> +    `c-dt` library functions handles properly both positive and >> negative `dt` >>> +    values, where `dt` is a number of dates since Rata Die date >> (0001-01-01). >>> + >>> +    For better compactness of our typical data in MessagePack stream we >> shift >>> +    root of our time to the Unix Epoch date (1970-01-01), thus our 0 is >>> +    actually dt = 719163. >>> + >>> +    So here is a simple formula how convert our epoch-based seconds to >> dt values >>> +        dt = (secs / 86400) + 719163 >>> +    Where 719163 is an offset of Unix Epoch (1970-01-01) since Rata Die >>> +    (0001-01-01) in dates. >>> + >>> +    */ >>> +    typedef int dt_t; >>> + >>> +    // dt_core.h >>> +    dt_t     tnt_dt_from_rdn     (int n); >>> +    dt_t     tnt_dt_from_ymd     (int y, int m, int d); >>> + >>> +    int      tnt_dt_rdn          (dt_t dt); >>> + >>> +    // dt_parse_iso.h >>> +    size_t tnt_dt_parse_iso_date          (const char *str, size_t len, >> dt_t *dt); >>> +    size_t tnt_dt_parse_iso_time          (const char *str, size_t len, >> int *sod, int *nsec); >>> +    size_t tnt_dt_parse_iso_zone_lenient  (const char *str, size_t len, >> int *offset); >> >> The line is too long. Please ensure that all lines are <= 80 characters >> long. > > Reformatted. > >> >>> + >>> +    // datetime.c >>> +    int >>> +    datetime_to_string(const struct datetime * date, char *buf, >> uint32_t len); >>> + >>> +    char * >>> +    datetime_asctime(const struct datetime *date, char *buf); >>> + >>> +    char * >>> +    datetime_ctime(const struct datetime *date, char *buf); >>> + >>> +    size_t >>> +    datetime_strftime(const struct datetime *date, const char *fmt, >> char *buf, >>> +                      uint32_t len); >>> + >>> +    void >>> +    datetime_now(struct datetime * now); >> >> Extra space after '*'. > > Removed > >> >>> + >>> +]] >>> + >>> +local builtin = ffi.C >>> +local math_modf = math.modf >>> + >>> +local SECS_PER_DAY     = 86400 >>> + >>> +-- c-dt/dt_config.h >>> + >>> +-- Unix, January 1, 1970, Thursday >>> +local DT_EPOCH_1970_OFFSET = 719163 >>> + >>> + >>> +local datetime_t = ffi.typeof('struct datetime') >>> +local interval_t = ffi.typeof('struct datetime_interval') >>> + >>> +local function is_interval(o) >>> +    return type(o) == 'cdata' and ffi.istype(interval_t, o) >> >> The check for 'cdata' is redundant. ffi.istype alone should be enough >> AFAIK. > > Wanted to object, but have verified and indeed, in that particular order > (first ffi metatype reference, then value) it's accepting every type. > Apparently I'd got wrong impression when I passed arguments in the wrong > order. Updated. > > >> >>> +end >>> + >>> +local function is_datetime(o) >>> +    return type(o) == 'cdata' and ffi.istype(datetime_t, o) >>> +end >>> + >>> +local function is_date_interval(o) >>> +    return type(o) == 'cdata' and >>> +           (ffi.istype(interval_t, o) or ffi.istype(datetime_t, o)) >>> +end >>> + >>> +local function interval_new() >>> +    local interval = ffi.new(interval_t) >>> +    return interval >>> +end >>> + >>> +local function check_date(o, message) >>> +    if not is_datetime(o) then >>> +        return error(("%s: expected datetime, but received %s"): >>> +                     format(message, o), 2) >>> +    end >>> +end >>> + >>> +local function check_str(s, message) >>> +    if not type(s) == 'string' then >>> +        return error(("%s: expected string, but received %s"): >>> +                     format(message, s), 2) >>> +    end >>> +end >>> + >>> +local function check_range(v, range, txt) >>> +    assert(#range == 2) >>> +    if v < range[1] or v > range[2] then >>> +        error(('value %d of %s is out of allowed range [%d, %d]'): >>> +              format(v, txt, range[1], range[2]), 4) >>> +    end >>> +end >>> + >>> +local function datetime_cmp(lhs, rhs) >>> +    if not is_date_interval(lhs) or >>> +       not is_date_interval(rhs) then >>> +       return nil >> >> Bad indentation. > > Fixed. > >> >>> +    end >>> +    local sdiff = lhs.secs - rhs.secs >>> +    return sdiff ~= 0 and sdiff or (lhs.nsec - rhs.nsec) >>> +end >>> + >>> +local function datetime_eq(lhs, rhs) >>> +    local rc = datetime_cmp(lhs, rhs) >>> +    return rc ~= nil and rc == 0 >>> +end >>> + >>> +local function datetime_lt(lhs, rhs) >>> +    local rc = datetime_cmp(lhs, rhs) >>> +    return rc == nil and error('incompatible types for comparison', 2) >> or >>> +           rc < 0 >>> +end >>> + >>> +local function datetime_le(lhs, rhs) >>> +    local rc = datetime_cmp(lhs, rhs) >>> +    return rc == nil and error('incompatible types for comparison', 2) >> or >>> +           rc <= 0 >>> +end >>> + >>> +local function datetime_serialize(self) >>> +    return { secs = self.secs, nsec = self.nsec, offset = self.offset } >>> +end >>> + >>> +local function interval_serialize(self) >>> +    return { secs = self.secs, nsec = self.nsec } >>> +end >>> + >>> +local function datetime_new_raw(secs, nsec, offset) >>> +    local dt_obj = ffi.new(datetime_t) >>> +    dt_obj.secs = secs >>> +    dt_obj.nsec = nsec >>> +    dt_obj.offset = offset >>> +    return dt_obj >>> +end >>> + >>> +local function datetime_new_dt(dt, secs, frac, offset) >> >> What's 'frac'? Please either add a comment or give it a better name. > > fraction part. Renamed to full fraction. > >> >>> +    local epochV = dt ~= nil and (builtin.tnt_dt_rdn(dt) - >> DT_EPOCH_1970_OFFSET) * >>> +                   SECS_PER_DAY or 0 >> >> AFAIK we don't use camel-case for naming local variables in Lua code. >> >>> +    local secsV = secs ~= nil and secs or 0 >> >> This is equivalent to 'secs = secs or 0' > > Indeed. Fixed. > >> >>> +    local fracV = frac ~= nil and frac or 0 >>> +    local ofsV = offset ~= nil and offset or 0 >>> +    return datetime_new_raw(epochV + secsV - ofsV * 60, fracV, ofsV) >>> +end >>> + >>> +-- create datetime given attribute values from obj >>> +-- in the "easy mode", providing builder with >>> +-- .secs, .nsec, .offset >>> +local function datetime_new_obj(obj, ...) >>> +    if obj == nil or type(obj) ~= 'table' then >> >> type(obj) ~= 'table' implies 'obj' == nil > > I meant that obj may be simple number, not builder object. > >> >>> +        return datetime_new_raw(obj, ...) >>> +    end >>> +    local secs = 0 >>> +    local nsec = 0 >>> +    local offset = 0 >>> + >>> +    for key, value in pairs(obj) do >>> +        if key == 'secs' then >>> +            secs = value >>> +        elseif key == 'nsec' then >>> +            nsec = value >>> +        elseif key == 'offset' then >>> +            offset = value >>> +        else >>> +            error(('unknown attribute %s'):format(key), 2) >>> +        end >> >> Hmm, do we really need this check? IMO this is less clear and probably >> slower than accessing the table directly: >> >> local secs = obj.secs >> local nsecs = obj.nsecs >> ... > > Ok, let me put you to the context, where we have been iteration before. > Here we see smaller part of larger code which used to be emulating named > arguments for creating datetime using attributes from passed. It's a > standard idiom for emulating named arguments in a languages where there > is no such syntax, but there is json-like table/object/hash facilities. > > https://www.lua.org/pil/5.3.html > > The idea is to allow constructions like below (with any set of > attributes passed) > >     date = require 'datetime' >     T = date.new{ year = 20201, month = 11, day = 15, >               hour = 1, minute = 10, second = 01, tz = 180} > > The problem was that there is set of attributes, which we want to use > for initialization via this same named attributes idiom, but directly > using .secs, .nsec, .offset attributes when we know what we are doing. > >     T = date.new_raw{ secs = 0, nsec = 0, offset = 180} > > That's why this silly separate version has been created which strangely > parsing set of attributes accessible in the cdata object. (If there is > passed cdata object, but it's not) > > [Worth to note, that at the moment in addition to initialization object > `date.new_raw` now handles gracefully this case of calling it without > creating initialization object, but passing arguments: > >     T = date.new_raw(0, 0, 180) > > This is why there is check for object in obj `type(obj) ~= 'table'` > ] > > > Now you have asked that I realized that ffi.new could do the > initialization magic for us, and instead of this for-loop we could > simply pass an object to the 2nd argument of ffi.new. > >     tarantool> o = { secs = 0, nsec = 0, offset = 180} >     --- >     ... > >     tarantool> ffi = require 'ffi' >     --- >     ... > >     tarantool> T = ffi.new('struct datetime', o) >     --- >     ... > >     tarantool> T >     --- >     - 1970-01-01T03:00+03:00 >     ... > > The problem is - it will be silently ignoring bogus attributes we may > pass to this initialization object, which do not map directly to the set > of known fields in a structure initialized. > > Which is not a problem for the code above, where we verify set of > attributes in an object, and bail out if there is any unknown attribute. > > Performance-wise I do not expect such fancy initialization of objects > would be on a critical path. The expected scenario for average scenario > - creating datetime objects using textual strings (from logs). And this > is very fast, due to c-dt parsing speed. > > > So here is dilema: > - we could directly pass initialization object to the ffi.new. But allow > all kinds of unknown attributes; > - or we could check all attributes in a slightly slower loop, but > provide nice user visible diagnostics for an error. > > What would you recommend? (I prefer implementation with checks and > human-understandable errors) > > >> >> I never saw we do anything like this in Lua code. >> >> Same comment for datetime_new and other places where you use pairs() >> like this. >> > > It's still same approach to use initialization object for "named > arguments" idiom. > >>> +    end >>> + >>> +    return datetime_new_raw(secs, nsec, offset) >>> +end >>> + >>> +-- create datetime given attribute values from obj >> >> Bad comment - what's 'obj'? > > Yes, probably the confusion created because I was too terse here, and > not explained that obj is initialization object for "named arguments" > approach. > >> >>> +local function datetime_new(obj) >>> +    if obj == nil or type(obj) ~= 'table' then >>> +        return datetime_new_raw(0, 0, 0) >>> +    end >>> +    local y = 0 >>> +    local M = 0 >>> +    local d = 0 >>> +    local ymd = false >>> + >>> +    local h = 0 >>> +    local m = 0 >>> +    local s = 0 >>> +    local frac = 0 >>> +    local hms = false >>> +    local offset = 0 >>> + >>> +    local dt = 0 >>> + >>> +    for key, value in pairs(obj) do >>> +        if key == 'year' then >>> +            check_range(value, {1, 9999}, key) >>> +            y = value >>> +            ymd = true >>> +        elseif key == 'month' then >>> +            check_range(value, {1, 12}, key) >>> +            M = value >>> +            ymd = true >>> +        elseif key == 'day' then >>> +            check_range(value, {1, 31}, key) >>> +            d = value >>> +            ymd = true >>> +        elseif key == 'hour' then >>> +            check_range(value, {0, 23}, key) >>> +            h = value >>> +            hms = true >>> +        elseif key == 'min' or key == 'minute' then >>> +            check_range(value, {0, 59}, key) >>> +            m = value >>> +            hms = true >>> +        elseif key == 'sec' or key == 'second' then >> >> I don't think we should support both 'sec'/'min' and 'second'/'minute' >> here. Since you want to be consistent with os.date() output, I would >> leave 'sec'/'min' only. > > Originally, there were only human-readable names like 'second' or > 'minute', os.date compatibility added only lately. Because, it costs > nothing (comparing to all other overhead). > > I'd prefer to have ability to use full readable names, not only those > from set of os.date. It costs nothing, but make it's friendlier. > >> >>> +            check_range(value, {0, 60}, key) >>> +            s, frac = math_modf(value) >>> +            frac = frac * 1e9 -- convert fraction to nanoseconds >> >> So 'frac' actually stores nanoseconds. Please rename accordingly. > > Yes, this is fractional part of seconds, but represented as integers in > nanoseconds units. Renamed. > >>> +            hms = true >>> +        elseif key == 'tz' then >>> +        -- tz offset in minutes >> >> Bad indentation. > > Fixed. > >> >>> +            check_range(value, {0, 720}, key) >>> +            offset = value >>> +        elseif key == 'isdst' or key == 'wday' or key =='yday' then -- >> luacheck: ignore 542 >> >> Missing space between '==' and 'yday'. > > Fixed. > >> >>> +            -- ignore unused os.date attributes >>> +        else >>> +            error(('unknown attribute %s'):format(key), 2) >>> +        end >>> +    end >>> + >>> +    -- .year, .month, .day >>> +    if ymd then >>> +        dt = builtin.tnt_dt_from_ymd(y, M, d) >>> +    end >>> + >>> +    -- .hour, .minute, .second >>> +    local secs = 0 >>> +    if hms then >>> +        secs = h * 3600 + m * 60 + s >>> +    end >>> + >>> +    return datetime_new_dt(dt, secs, frac, offset) >>> +end >>> + >>> +--[[ >>> +    Basic      Extended >>> +    20121224   2012-12-24   Calendar date   (ISO 8601) >>> +    2012359    2012-359     Ordinal date    (ISO 8601) >>> +    2012W521   2012-W52-1   Week date       (ISO 8601) >>> +    2012Q485   2012-Q4-85   Quarter date >>> +]] >> >> Please mention in the comment what this function returns. >> Same for other 'parse' functions. > > Good point. Clarified. > >> >>> + >>> +local function parse_date(str) >>> +    check_str("datetime.parse_date()") >> >> check_str(str, ...) >> >> Here and everywhere else. >> >>> +    local dt = ffi.new('dt_t[1]') >>> +    local len = builtin.tnt_dt_parse_iso_date(str, #str, dt) >>> +    return len > 0 and datetime_new_dt(dt[0]) or nil, tonumber(len) >> >> Is this tonumber() really necessary? > > Yup, size_t is boxed in cdata. > ``` > tarantool> ffi = require 'ffi' > --- > ... > > tarantool> ffi.cdef [[ size_t tnt_dt_parse_iso_date (const char *str, > size_t len, dt_t *dt); ]] > --- > ... > > tarantool> ffi.cdef [[ typedef int dt_t; ]] > --- > ... > > > tarantool> dt = ffi.new 'dt_t[1]' > --- > ... > > tarantool> r = ffi.C.tnt_dt_parse_iso_date('1970-01-01', 10, dt) > --- > ... > > tarantool> dt[0] > --- > - 719163 > ... > > tarantool> type(r) > --- > - cdata > ... > > tarantool> ffi.typeof(r) > --- > - ctype > ... > ``` > >> >>> +end >>> + >>> +--[[ >>> +    Basic               Extended >>> +    T12                 N/A >>> +    T1230               T12:30 >>> +    T123045             T12:30:45 >>> +    T123045.123456789   T12:30:45.123456789 >>> +    T123045,123456789   T12:30:45,123456789 >>> + >>> +    The time designator [T] may be omitted. >>> +]] >>> +local function parse_time(str) >>> +    check_str("datetime.parse_time()") >>> +    local sp = ffi.new('int[1]') >>> +    local fp = ffi.new('int[1]') >>> +    local len = builtin.tnt_dt_parse_iso_time(str, #str, sp, fp) >>> +    return len > 0 and datetime_new_dt(nil, sp[0], fp[0]) or nil, >>> +           tonumber(len) >>> +end >>> + >>> +--[[ >>> +    Basic    Extended >>> +    Z        N/A >>> +    +hh      N/A >>> +    -hh      N/A >>> +    +hhmm    +hh:mm >>> +    -hhmm    -hh:mm >>> +]] >>> +local function parse_zone(str) >>> +    check_str("datetime.parse_zone()") >>> +    local offset = ffi.new('int[1]') >>> +    local len = builtin.tnt_dt_parse_iso_zone_lenient(str, #str, >> offset) >>> +    return len > 0 and datetime_new_dt(nil, nil, nil, offset[0]) or >> nil, >>> +           tonumber(len) >>> +end >>> + >> >> Extra new line. > > Removed > >> >>> + >>> +--[[ >>> +    aggregated parse functions >>> +    assumes to deal with date T time time_zone >>> +    at once >>> + >>> +    date [T] time [ ] time_zone >>> +]] >>> +local function parse(str) >>> +    check_str("datetime.parse()") >>> +    local dt = ffi.new('dt_t[1]') >>> +    local len = #str >>> +    local n = builtin.tnt_dt_parse_iso_date(str, len, dt) >>> +    local dt_ = dt[0] >>> +    if n == 0 or len == n then >>> +        return datetime_new_dt(dt_) >>> +    end >>> + >>> +    str = str:sub(tonumber(n) + 1) >>> + >>> +    local ch = str:sub(1,1) >> >> Missing space after ','. > > Fixed > >> >>> +    if ch:match('[Tt ]') == nil then >>> +        return datetime_new_dt(dt_) >>> +    end >>> + >>> +    str = str:sub(2) >>> +    len = #str >>> + >>> +    local sp = ffi.new('int[1]') >>> +    local fp = ffi.new('int[1]') >>> +    local n = builtin.tnt_dt_parse_iso_time(str, len, sp, fp) >>> +    if n == 0 then >>> +        return datetime_new_dt(dt_) >>> +    end >>> +    local sp_ = sp[0] >>> +    local fp_ = fp[0] >>> +    if len == n then >>> +        return datetime_new_dt(dt_, sp_, fp_) >>> +    end >>> + >>> +    str = str:sub(tonumber(n) + 1) >>> + >>> +    if str:sub(1,1) == ' ' then >> >> Missing space after ','. > > Fixed > >> >>> +        str = str:sub(2) >>> +    end >>> + >>> +    len = #str >>> + >>> +    local offset = ffi.new('int[1]') >>> +    n = builtin.tnt_dt_parse_iso_zone_lenient(str, len, offset) >>> +    if n == 0 then >>> +        return datetime_new_dt(dt_, sp_, fp_) >>> +    end >>> +    return datetime_new_dt(dt_, sp_, fp_, offset[0]) >>> +end >>> + >>> +local function datetime_from(o) >> >> Please add a comment explaining what this function does. > > Added. > >> >>> +    if o == nil or type(o) == 'table' then >>> +        return datetime_new(o) >>> +    elseif type(o) == 'string' then >>> +        return parse(o) >>> +    end >>> +end >>> + >>> +local function local_now() >> >> Please add a comment explaining what this function does. > > Added. > >> >>> +    local d = datetime_new_raw(0, 0, 0) >>> +    builtin.datetime_now(d) >>> +    return d >>> +end >>> + >>> +-- Change the time-zone to the provided target_offset >>> +-- Time `.secs`/`.nsec` are always UTC normalized, we need only to >>> +-- reattribute object with different `.offset` >> >> It doesn't change the time-zone of the given object. It creates a new >> object with the new timezone. Please fix the comment to avoid confusion. > > Indeed. Implementation has been changed to make original object immune > to the timezone changes, but comment has not been updated. Updated now. > >> >>> +local function datetime_to_tz(self, tgt_ofs) >>> +    if self.offset == tgt_ofs then >>> +        return self >>> +    end >>> +    if type(tgt_ofs) == 'string' then >>> +        local obj = parse_zone(tgt_ofs) >>> +        if obj == nil then >>> +            error(('%s: invalid time-zone format %s'):format(self, >> tgt_ofs), 2) >>> +        else >>> +            tgt_ofs = obj.offset >> >> target_offset. Please don't use confusing abbreviations. >> >>> +        end >>> +    end >>> +    return datetime_new_raw(self.secs, self.nsec, tgt_ofs) >>> +end >>> + >>> +local function datetime_index(self, key) >>> +    if key == 'epoch' or key == 'unixtime' then >>> +        return self.secs >>> +    elseif key == 'ts' or key == 'timestamp' then >>> +        return self.secs + self.nsec / 1e9 >>> +    elseif key == 'ns' or key == 'nanoseconds' then >>> +        return self.secs * 1e9 + self.nsec >>> +    elseif key == 'us' or key == 'microseconds' then >>> +        return self.secs * 1e6 + self.nsec / 1e3 >>> +    elseif key == 'ms' or key == 'milliseconds' then >>> +        return self.secs * 1e3 + self.nsec / 1e6 >>> +    elseif key == 's' or key == 'seconds' then >>> +        return self.secs + self.nsec / 1e9 >>> +    elseif key == 'm' or key == 'min' or key == 'minutes' then >>> +        return (self.secs + self.nsec / 1e9) / 60 >>> +    elseif key == 'hr' or key == 'hours' then >>> +        return (self.secs + self.nsec / 1e9) / (60 * 60) >>> +    elseif key == 'd' or key == 'days' then >>> +        return (self.secs + self.nsec / 1e9) / (24 * 60 * 60) >> >>   1. There's so many ways to get the same information, for example m, >>      min, minutes. There should be exactly one way. > > Disagreed. I prefer friendlier approach. > >> >>   2. I'd expect datetime.hour return the number of hours in the current >>      day, like os.date(), but it returns the number of hours since the >>      Unix epoch instead. This is confusing and useless. > > Well, at least this is consistent with seconds and their fractions of > different units. If there would be demand for os.date compatible table > format, we might extend interface with corresponding accessor. But I do > not foresee it, because os.date is totally broken if we need correct > handling of timezone information. AFAIK it's always using local time > context. >> >>   3. Please document the behavior somewhere in the comments to the code. > > Documented. > >> >>   4. These if-else's are inefficient AFAIU. > > Quite the contrary (if we compare to the idiomatic closure handlers > case). If/ifelse is the fastest way to handle such cases in Lua. > Here is earlier bench we have discussed with Vlad and Oleg Babin, and > after which I've changed from using hash based handlers to the series of > ifs. > > https://gist.github.com/tsafin/31cc9b0872b6015904fcc90d97740770 > > >> >>> +    elseif key == 'to_utc' then >>> +        return function(self) >>> +            return datetime_to_tz(self, 0) >>> +        end >>> +    elseif key == 'to_tz' then >>> +        return function(self, offset) >>> +            return datetime_to_tz(self, offset) >>> +        end >>> +    else >>> +        error(('unknown attribute %s'):format(key), 2) >>> +    end >>> +end >>> + >>> +local function datetime_newindex(self, key, value) >>> +    if key == 'epoch' or key == 'unixtime' then >>> +        self.secs = value >>> +        self.nsec, self.offset = 0, 0 >>> +    elseif key == 'ts' or key == 'timestamp' then >>> +        local secs, frac = math_modf(value) >>> +        self.secs = secs >>> +        self.nsec = frac * 1e9 >>> +        self.offset = 0 >> >> Do we really want the datetime object to be mutable? If so, allowing to >> set its value only to the time since the unix epoch doesn't look >> particularly user-friendly. > > I do want. That was request from customer - to have ability to modify by > timestamp from Epoch. > >> >>> +    else >>> +        error(('assigning to unknown attribute %s'):format(key), 2) >>> +    end >>> +end >>> + >>> +-- sizeof("Wed Jun 30 21:49:08 1993\n") >>> +local buf_len = 26 >> >> What if year > 9999? > > Good point. IIRC both BSD libc and glibc have hardcoded length limit at > around this size and not ready to handle such huge dates properly. > >> >>> + >>> +local function asctime(o) >>> +    check_date(o, "datetime:asctime()") >>> +    local buf = ffi.new('char[?]', buf_len) >> >> I think it would be more efficient to define the buffer in a global >> variable - we don't yield in this code so this should be fine. Also, >> please give the buffer an appropriate name that would say that it is >> used for datetime formatting. > > Good point. Changed to using outer variables in asctime and ctime > functions. > >> >>> +    return ffi.string(builtin.datetime_asctime(o, buf)) >>> +end >>> + >>> +local function ctime(o) >>> +    check_date(o, "datetime:ctime()") >>> +    local buf = ffi.new('char[?]', buf_len) >>> +    return ffi.string(builtin.datetime_ctime(o, buf)) >>> +end >>> + >>> +local function strftime(fmt, o) >>> +    check_date(o, "datetime.strftime()") >>> +    local sz = 128 >>> +    local buff = ffi.new('char[?]', sz) >>> +    builtin.datetime_strftime(o, fmt, buff, sz) >>> +    return ffi.string(buff) >>> +end >>> + >>> +local datetime_mt = { >>> +    __serialize = datetime_serialize, >>> +    __eq = datetime_eq, >>> +    __lt = datetime_lt, >>> +    __le = datetime_le, >>> +    __index = datetime_index, >>> +    __newindex = datetime_newindex, >>> +} >>> + >>> +local interval_mt = { >>> +    __serialize = interval_serialize, >>> +    __eq = datetime_eq, >>> +    __lt = datetime_lt, >>> +    __le = datetime_le, >>> +    __index = datetime_index, >> >> Why does datetime_mt has __newindex while interval_mt doesn't? > > I did not foresee the need. Should we? > > And semantically datetime has persistence and life-time, but intervals > have not. [And besides all we already provide a plenty of helpers for > creation of intervals] > >> >>> +} >>> + >>> +ffi.metatype(interval_t, interval_mt) >>> +ffi.metatype(datetime_t, datetime_mt) >>> + >>> +return setmetatable( >>> +    { >>> +        new         = datetime_new, >>> +        new_raw     = datetime_new_obj, >> >> I'm not sure, we need to make the 'raw' function public. > > That's for second kind of constructors using initialization object but > with low level attributes {secs = 0, nsec = 0, offset}. It's not for > everybody, but still provides some ergonomics. > >> >>> +        interval    = interval_new, >> >> Why would anyone want to create a 0 interval? > > Good point, especiall taking into account that there is no way to modify > it, beyond directly modifying .secs and .nsec fields of cdata object. > > Will delete it. We already have a plenty of helpers to construct > different kinds of intervals. > >> >>> + >>> +        parse       = parse, >>> +        parse_date  = parse_date, >> >>> +        parse_time  = parse_time, >>> +        parse_zone  = parse_zone, >> >> Creating a datetime object without a date sounds confusing. > > :) But I do foresee the need to parse parts of datetime string, and I > did not want to establish special type for that. > > Now you have asked it I realized we may better create interval objects > of approapriate time. But in this case we need to add timezone to > interval record. So it will be able to represent timezone shifts. > > Ok, heer is the deal: > - at the moment those partial parsed objects store their data to the > partial datetime object created. That's confusing but do not require > modifications in interval arithmetic. > - we may start to store partial time and timezone information into > generic intervals. But it would require extension of interval arithmetic > to be ready to shift by timezone delta. [Does it even make any sense?] > > What do you think? > >> >>> + >>> +        now         = local_now, >>> +        strftime    = strftime, >>> +        asctime     = asctime, >>> +        ctime       = ctime, >>> + >>> +        is_datetime = is_datetime, >>> +        is_interval = is_interval, >> >> I don't see any point in making these functions public. > > I was asked by customer to introduce such. > >> >>> +    }, { >>> +        __call = function(self, ...) return datetime_from(...) end >>> +    } >>> +) >>> diff --git a/src/lua/init.c b/src/lua/init.c >>> index f9738025d..127e935d7 100644 >>> --- a/src/lua/init.c >>> +++ b/src/lua/init.c >>> @@ -129,7 +129,8 @@ extern char strict_lua[], >>>       parse_lua[], >>>       process_lua[], >>>       humanize_lua[], >>> -    memprof_lua[] >>> +    memprof_lua[], >>> +    datetime_lua[] >>>   ; >>>   static const char *lua_modules[] = { >>> @@ -184,6 +185,7 @@ static const char *lua_modules[] = { >>>       "memprof.process", process_lua, >>>       "memprof.humanize", humanize_lua, >>>       "memprof", memprof_lua, >>> +    "datetime", datetime_lua, >>>       NULL >>>   }; >>> diff --git a/src/lua/utils.c b/src/lua/utils.c >>> index c71cd4857..2c89326f3 100644 >>> --- a/src/lua/utils.c >>> +++ b/src/lua/utils.c >>> @@ -48,6 +48,9 @@ static uint32_t CTID_STRUCT_IBUF_PTR; >>>   uint32_t CTID_CHAR_PTR; >>>   uint32_t CTID_CONST_CHAR_PTR; >>>   uint32_t CTID_UUID; >>> +uint32_t CTID_DATETIME = 0; >>> +uint32_t CTID_INTERVAL = 0; >> >> Should be called CTID_DATETIME_INTERVAL. >> >> Anyway, it's never used. Please remove. > > It's used in later patches. Wil lsquash and rename. > >> >>> + >>>   void * >>>   luaL_pushcdata(struct lua_State *L, uint32_t ctypeid) >>> @@ -120,6 +123,12 @@ luaL_pushuuidstr(struct lua_State *L, const struct >> tt_uuid *uuid) >>>       lua_pushlstring(L, str, UUID_STR_LEN); >>>   } >>> +struct datetime * >>> +luaL_pushdatetime(struct lua_State *L) >>> +{ >>> +    return luaL_pushcdata(L, CTID_DATETIME); >>> +} >>> + >> >> This function isn't used in this patch. Please move it to patch 4. >> > > Looks like it will be used once squashed. > > Thanks, > Timur > > P.S. > > I need to jump to the different location, so the rest of responses will > be send in a couple of hours. Please answer some questions - I 'm > interested in your opinion.