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 229596EC55; Fri, 16 Jul 2021 09:58:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 229596EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626418727; bh=O+ISFOb0qb51FL9o1bnvj57ATyzq7RP8JkH+1HFghdc=; 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=oG7jd7K4uHVxiYtjOHZ1I0fgy+b5XFSbihLJX0iXlLvOjzQu2K/a1yAf9R+TuAIZr 2nrJBzorQ+qYnPH1KqvRGzwxH3lEfh5OQ5Ohd38I0jkS3G40eIadCFmKJB6BaT8vYZ cmN2swXzh1yMg2gHLTNYhR5TACQbXf0ATwfeX2r4= Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (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 2C4C26EC55 for ; Fri, 16 Jul 2021 09:58:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2C4C26EC55 Received: by smtp57.i.mail.ru with esmtpa (envelope-from ) id 1m4Ho5-0007sf-8T; Fri, 16 Jul 2021 09:58:45 +0300 To: Timur Safin Cc: TML References: <5deb9cc2-3a74-9968-0fcc-aa0a1f851fb9@tarantool.org> <129501d779cd$9e83bfd0$db8b3f70$@tarantool.org> Message-ID: Date: Fri, 16 Jul 2021 09:58:44 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <129501d779cd$9e83bfd0$db8b3f70$@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD941C43E597735A9C3D3726AD0AC8C7907896201F0AD2BBAE8182A05F538085040090D45F674E37F3A525F287B5269115C78A37CBDC5DF329184542300BD3B8B3D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72F22E6DC541F75D9EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637826327602763C04B8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8E08C204E034F3F74FBA14FD7AC5E36B9117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE4B6963042765DA4B4DB3626BA78294CCD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE37AE820D2C17D0E5603F1AB874ED89028C4224003CC8364764BC211DE43E6142DE2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637A7EFCB0EB5ACB161EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5CD301AA0BFC91771617051742EF8B2A0667E313ED619CBC0D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AB80C45F81B80D636E12F7B02E312BC0A57FA450D8AE12182461B083B4A1A66F0F3A4E69059DF1031D7E09C32AA3244CED4EE2F9C0FB8152A6477A3B1EB602B0F165894D92D62706729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojk34dk3RWCsa4SJknwpVB/Q== X-Mailru-Sender: 583F1D7ACE8F49BD1042885CEC987B6B5645115C4A61D597525F287B5269115CD38A1BEA64D5C64A7019711D9D5B048E1458020726E2BC9FD5ECBA0B92C0A936CDC7563AA7CEBD287402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [RFC PATCH 00/13] Initial datetime support 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: Oleg Babin via Tarantool-patches Reply-To: Oleg Babin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On 16.07.2021 02:03, Timur Safin wrote: > Thanks, Oleg, for your fast and valuable feedback! > [ You are the fastest hand on the wild west today! ] > > > : From: Tarantool-patches On > : Subject: Re: [Tarantool-patches] [RFC PATCH 00/13] Initial datetime support > : <...> > : > : 6. Does this module allows to manipulate with timezones and locales? > : > : Icu-date that we currently use allows it > : (https://github.com/tarantool/icu-date). > > Not, at the moment. And I do like how it's handled in icu-date. And > surprisingly that was exactly original plan - to use icu database > for translation of timezone names into their offsets, with memorization > of results (exactly as it's done in zone_id_cache in icu-date.lua). > > Locales is the strongest feature in icu-date, and should be reused > properly while handling named-timezones. Will return here > > : > : Also try to use tests from icu-date module. I think it could be useful. > : The best option is to allow drop-in but I'm not sure that it's possible. > > Drop-in is hardly possible (because icu-date api looks too much mouthful > with all their forms of a kind date:get(icu_date.fields.MILLISECOND) or > date:get_attribute(attributes.MINIMAL_DAYS_IN_FIRST_WEEK). But tests might > be of help. (And I assume you meant tap tests, not busted tests). And while > we are here - why there are 2 kinds of tests? (And not only busted used because > it's much more convenient? :) ) Because it's impossible to install "busted" via tarantoolctl, because nobody tests is it compatible with Tarantool. Tarantool has built-in tap module that is used and also there is a luatest that seems to be ok for many cases. (But luatest was introduced a bit later than module was improted so we use tap for test). > > : 7. Is it expected? > : > : ``` > : > : tarantool> datetime.now() - datetime.now() > : --- > : - +18823 days, 16 hours, 29 minutes, 44.178836107254 seconds > : ... > : > : ``` > : > : I believe it should be 0. > > Not exactly 0, but very small fraction of second. Thanks for reporting - that > Was artifact of last minute rename/refactoring. Here is the patch for this > Problem and similar in + > > diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua > index d28f931f6..57ef80403 100644 > --- a/src/lua/datetime.lua > +++ b/src/lua/datetime.lua > @@ -558,6 +558,17 @@ local function date_first(lhs, rhs) > end > end > > +local function _normalize_nsec(secs, nsec) > + if nsec < 0 then > + secs = secs - 1 > + nsec = nsec + NANOS_PER_SEC > + elseif nsec >= NANOS_PER_SEC then > + secs = secs + 1 > + nsec = nsec - NANOS_PER_SEC > + end > + return secs, nsec > +end > + > local function datetime_sub(lhs, rhs) > check_date(lhs) -- make sure left is date > local d, s = lhs, rhs > @@ -565,18 +576,15 @@ local function datetime_sub(lhs, rhs) > -- 1. left is date, right is date or delta > if (ffi.typeof(s) == datetime_t) or (ffi.typeof(s) == duration_t) then > local o > + > -- if they are both dates then result is delta > if ffi.typeof(s) == datetime_t then > o = duration_new() > else > o = datetime_new() > end > - o.secs = d.secs - s.secs > - o.nsec = d.nsec - s.nsec > - if o.nsec < 0 then > - o.secs = d.secs - 1 > - o.nsec = d.nsec + NANOS_PER_SEC > - end > + o.secs, o.nsec = _normalize_nsec(d.secs - s.secs, d.nsec - s.nsec) > + > return o > > -- 2. left is date, right is duration in months > @@ -602,12 +610,8 @@ local function datetime_add(lhs, rhs) > if (ffi.typeof(s) == datetime_t) or (ffi.typeof(s) == duration_t) then > local o = datetime_new() > > - o.secs = d.secs + s.secs > - o.nsec = d.nsec + s.nsec > - if o.nsec >= NANOS_PER_SEC then > - o.secs = d.secs + 1 > - o.nsec = d.nsec - NANOS_PER_SEC > - end > + o.secs, o.nsec = _normalize_nsec(d.secs + s.secs, d.nsec + s.nsec) > + > return o > > -- 2. left is date, right is duration in months > > : > : > : 8. Could you provide a helper that allows to convert unix time to datetime? > > Will introduce something, yes, we need one. Thanks! > > : > : I tried this way but it seems an interval instead of datetime. > : > : ``` > : > : tarantool> datetime.seconds(datetime.now().seconds) > : --- > : - +18823 days, 16 hours, 34 minutes, 10.283248901367 seconds > : ... > : > : ``` > : > : > : 9. Lua output doesn't support datetime > : > : ``` > : > : tarantool> \set output yaml > : --- > : - true > : ... > : > : tarantool> datetime.now() > : --- > : - 2021-07-15T19:46:15.797333+03:00 > : ... > : > : tarantool> \set output lua > : true; > : tarantool> datetime.now() > : nil; > : > : ``` > > Interesting, didn't know of that. Do you have any hints where it's > Implemented for, say, decimal, or uuid? https://github.com/tarantool/tarantool/blob/master/src/box/lua/serialize_lua.c > > : > : > : 10. You can export also the number of months/days in week as constansts. > : > : Probably it could be useful to make something like: > : > : ``` > : datetime_object:set('month', datetime.months.JANUARY) > : > : ``` > > Hmm, hmm. Need to discuss with others. Didn't see so much details exported > in python or perl datetime modules. How frequently you needed such > functionality from icu-date? I can't anser this question. Because we just provide some interfaces for our customers that is actually wrappers over icu-date. As I can see there are several function that uses it. There is one of them: ``` -- Returns the start of the current UTC date (zero hours, minutes, seconds, etc.) -- The result is in nanoseconds local function curr_date_nsec()     local date = get_icu_date_instance()     date:set_millis(date:now())     local year = date:get(icu_date.fields.YEAR)     local month = date:get(icu_date.fields.MONTH)     local day_of_month = date:get(icu_date.fields.DAY_OF_MONTH)     date:set_millis(0)     date:set(icu_date.fields.YEAR, year)     date:set(icu_date.fields.MONTH, month)     date:set(icu_date.fields.DAY_OF_MONTH, day_of_month)     return date:get_millis() * NSEC_IN_MILLISEC end ``` However here we need to just strip datetime to just date. Also it seems that getters are used in more intensive way: ``` local function nsec_to_day_of_week(nsec)     checks("number|cdata")     local millis = to_milliseconds(nsec)     local date = get_icu_date_instance()     date:set_millis(millis)     return date:get(icu_date.fields.DAY_OF_WEEK) end local function parse_unix_time(nsec)     checks("number|cdata")     local millis = to_milliseconds(nsec)     local date = get_icu_date_instance()     date:set_millis(millis)     return {         year = date:get(icu_date.fields.YEAR),         month = date:get(icu_date.fields.MONTH) + 1, -- starts with 0         day = date:get(icu_date.fields.DAY_OF_MONTH),     } end ``` > > : > : > : It was brief feedback as from (I hope) future customer of this module. > : > : If I've found another interface issues I'll give you feedback in next > : e-mails. > : > : I didn't see code itself if you need it I can review this patchset in > : more detailed way. > > That would be interesting to hear, thanks in advance! > > Timur > I think I'll do it on the next iteration of this patch. Thanks for your work on it.