Tarantool development patches archive
 help / color / mirror / Atom feed
From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Timur Safin <tsafin@tarantool.org>, v.shpilevoy@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH resend v2 10/11] lua, datetime: unixtime, timestamp setters in datetime.lua
Date: Thu, 29 Jul 2021 21:58:38 +0300
Message-ID: <531544e8-0dd8-8d71-ccfc-4dae2805d498@tarantool.org> (raw)
In-Reply-To: <faee875900fc77e0b4c80876a5bc638230395bcb.1627468002.git.tsafin@tarantool.org>

Thanks for your patch.

The comment is primarly the same as in previous patch.

Why do you use such slow approach and creates redundant tables and 
function (temporary metatable in fact)

to use it only once? Please fix it.


Unixtime and timestamp is great but they loss precision. I think it 
should be possible

go get timestamp with nanoseconds precision since datetime has 
nanoseconds precision.


Also still it's hard how to convert timestamp to datetime value back.


On 28.07.2021 13:34, Timur Safin via Tarantool-patches wrote:
> * implemented proper range checks for date attributes values;
>
> * created `.unixtime` attribute, which is alias to `.secs`,
>    with corresponding setter/getter;
>
> * similarly to `unixtime`, created virtual `timestamp` attribute
>    setter. Which is a convenient way to simultaneously assign
>    unixtime (seconds since epoch) and nanoseconds
>
> Part of #5941
> ---
>   src/lua/datetime.lua | 90 ++++++++++++++++++++++++++++++--------------
>   1 file changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
> index 1466b923f..cc2ae119b 100644
> --- a/src/lua/datetime.lua
> +++ b/src/lua/datetime.lua
> @@ -342,12 +342,33 @@ local function _normalize_nsec(secs, nsec)
>       return secs, nsec
>   end
>   
> +local function seconds_fraction(v)
> +    local seconds, fraction
> +    fraction = v % 1
> +    if fraction > 0 then
> +        seconds = v - (v % 1)
> +    else
> +        seconds = v
> +    end
> +    return seconds, fraction
> +end
> +
> +local function check_range(v, range, txt)
> +    assert(#range == 2)
> +    if not (v >= range[1] and v <= range[2]) then
> +        error(('value %d of %s is out of allowed range [%d, %d]'):
> +              format(v, txt, range[1], range[2]))
> +    end
> +end
> +
>   -- addition or subtraction from date/time of a given interval
>   -- described via table direction should be +1 or -1
>   local function interval_increment(self, o, direction)
>       assert(direction == -1 or direction == 1)
> -    check_date(self, "interval_increment(date, object, -+1)")
> -    assert(type(o) == 'table')
> +    check_date(self, "interval_increment(date, object, direction)")
> +    if type(o) ~= 'table' then
> +        error('interval_increment(date, object, direction) - object expected', 2)
> +    end
>   
>       local ym_updated = false
>       local dhms_updated = false
> @@ -359,49 +380,43 @@ local function interval_increment(self, o, direction)
>       for key, value in pairs(o) do
>           local handlers = {
>               years = function(v)
> -                assert(v > 0 and v < 10000)
> +                check_range(v, {0, 9999}, key)
>                   dt = cdt.dt_add_years(dt, direction * v, cdt.DT_LIMIT)
>                   ym_updated = true
>               end,
>   
>               months = function(v)
> -                assert(v > 0 and v < 13 )
> +                check_range(v, {0, 12}, key)
>                   dt = cdt.dt_add_months(dt, direction * v, cdt.DT_LIMIT)
>                   ym_updated = true
>               end,
>   
>               weeks = function(v)
> -                assert(v > 0 and v < 32)
> +                check_range(v, {0, 52}, key)
>                   secs = secs + direction * 7 * v * SECS_PER_DAY
>                   dhms_updated = true
>               end,
>   
>               days = function(v)
> -                assert(v > 0 and v < 32)
> +                check_range(v, {0, 31}, key)
>                   secs = secs + direction * v * SECS_PER_DAY
>                   dhms_updated = true
>               end,
>   
>               hours = function(v)
> -                assert(v >= 0 and v < 24)
> +                check_range(v, {0, 23}, key)
>                   secs = secs + direction * 60 * 60 * v
>                   dhms_updated = true
>               end,
>   
>               minutes = function(v)
> -                assert(v >= 0 and v < 60)
> +                check_range(v, {0, 59}, key)
>                   secs = secs + direction * 60 * v
>               end,
>   
>               seconds = function(v)
> -                assert(v >= 0 and v < 61)
> -                local s, frac
> -                frac = v % 1
> -                if frac > 0 then
> -                    s = v - (v % 1)
> -                else
> -                    s = v
> -                end
> +                check_range(v, {0, 60}, key)
> +                local s, frac = seconds_fraction(v)
>                   secs = secs + direction * s
>                   nsec = nsec + direction * frac * 1e9 -- convert fraction to nanoseconds
>                   dhms_updated = true
> @@ -429,6 +444,9 @@ end
>   
>   local datetime_index = function(self, key)
>       local attributes = {
> +        unixtime = function(self)
> +            return self.secs
> +        end,
>           timestamp = function(self)
>               return tonumber(self.secs) + self.nsec / 1e9
>           end,
> @@ -467,6 +485,24 @@ local datetime_index = function(self, key)
>       return attributes[key] ~= nil and attributes[key](self) or nil
>   end
>   
> +local function datetime_newindex(self, key, value)
> +    local attributes = {
> +        unixtime = function(self, value)
> +            self.secs = value
> +            self.nsec, self.offset = 0, 0
> +        end,
> +        timestamp = function(self, value)
> +            local secs, frac = seconds_fraction(value)
> +            self.secs = secs
> +            self.nsec = frac * 1e9
> +            self.offset = 0
> +        end,
> +    }
> +    if attributes[key] ~= nil then
> +        attributes[key](self, value)
> +    end
> +end
> +
>   local function datetime_new_raw(secs, nsec, offset)
>       local dt_obj = ffi.new(datetime_t)
>       dt_obj.secs = secs
> @@ -518,50 +554,45 @@ local function datetime_new(o)
>               end,
>   
>               year = function(v)
> -                assert(v > 0 and v < 10000)
> +                check_range(v, {1, 9999}, key)
>                   y = v
>                   ymd = true
>               end,
>   
>               month = function(v)
> -                assert(v > 0 and v < 13 )
> +                check_range(v, {1, 12}, key)
>                   M = v
>                   ymd = true
>               end,
>   
>               day = function(v)
> -                assert(v > 0 and v < 32)
> +                check_range(v, {1, 31}, key)
>                   d = v
>                   ymd = true
>               end,
>   
>               hour = function(v)
> -                assert(v >= 0 and v < 24)
> +                check_range(v, {0, 23}, key)
>                   h = v
>                   hms = true
>               end,
>   
>               minute = function(v)
> -                assert(v >= 0 and v < 60)
> +                check_range(v, {0, 59}, key)
>                   m = v
>                   hms = true
>               end,
>   
>               second = function(v)
> -                assert(v >= 0 and v < 61)
> -                frac = v % 1
> -                if frac > 0 then
> -                    s = v - (v % 1)
> -                else
> -                    s = v
> -                end
> +                check_range(v, {0, 60}, key)
> +                s, frac = seconds_fraction(v)
>                   frac = frac * 1e9 -- convert fraction to nanoseconds
>                   hms = true
>               end,
>   
>               -- tz offset in minutes
>               tz = function(v)
> -                assert(v >= 0 and v <= 720)
> +                check_range(v, {0, 720}, key)
>                   offset = v
>               end
>           }
> @@ -918,6 +949,7 @@ local datetime_mt = {
>       __sub = datetime_sub,
>       __add = datetime_add,
>       __index = datetime_index,
> +    __newindex = datetime_newindex,
>       add = function(self, o)
>           self = interval_increment(self, o, 1)
>           return self

  reply	other threads:[~2021-07-29 18:59 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 10:34 [Tarantool-patches] [PATCH resend v2 00/11] Initial datetime support Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 01/11] build: add Christian Hansen c-dt to the build Timur Safin via Tarantool-patches
2021-07-29 23:40   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-31  9:22     ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 02/11] lua: built-in module datetime Timur Safin via Tarantool-patches
2021-07-29 18:55   ` Oleg Babin via Tarantool-patches
2021-07-30 19:00     ` Timur Safin via Tarantool-patches
2021-07-31  6:29       ` Oleg Babin via Tarantool-patches
2021-07-31 16:51         ` Timur Safin via Tarantool-patches
2021-07-29 23:36   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-30 15:39     ` Timur Safin via Tarantool-patches
2021-08-01 17:01       ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 20:23         ` Timur Safin via Tarantool-patches
2021-08-04 23:57           ` Vladislav Shpilevoy via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 03/11] lua, datetime: datetime tests Timur Safin via Tarantool-patches
2021-07-29 18:55   ` Oleg Babin via Tarantool-patches
2021-07-30 20:45     ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 04/11] lua, datetime: display datetime Timur Safin via Tarantool-patches
2021-07-29 18:55   ` Oleg Babin via Tarantool-patches
2021-07-30 21:48     ` Timur Safin via Tarantool-patches
2021-07-31  6:29       ` Oleg Babin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 05/11] box, datetime: add messagepack support for datetime Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 06/11] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches
2021-07-29 18:56   ` Oleg Babin via Tarantool-patches
2021-07-30 22:18     ` Timur Safin via Tarantool-patches
2021-07-31  6:30       ` Oleg Babin via Tarantool-patches
2021-07-31  9:31         ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 07/11] lua, datetime: proper datetime encoding Timur Safin via Tarantool-patches
2021-07-29 18:57   ` Oleg Babin via Tarantool-patches
2021-07-30 22:20     ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 08/11] lua, datetime: calculated attributes for datetimes Timur Safin via Tarantool-patches
2021-07-29 18:57   ` Oleg Babin via Tarantool-patches
2021-07-30 22:30     ` Timur Safin via Tarantool-patches
2021-07-31  6:31       ` Oleg Babin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 09/11] lua, datetime: time intervals support Timur Safin via Tarantool-patches
2021-07-29 18:58   ` Oleg Babin via Tarantool-patches
2021-07-30 22:58     ` Timur Safin via Tarantool-patches
2021-07-31  6:31       ` Oleg Babin via Tarantool-patches
2021-07-31  9:20         ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 10/11] lua, datetime: unixtime, timestamp setters in datetime.lua Timur Safin via Tarantool-patches
2021-07-29 18:58   ` Oleg Babin via Tarantool-patches [this message]
2021-07-30 23:11     ` Timur Safin via Tarantool-patches
2021-07-31  6:31       ` Oleg Babin via Tarantool-patches
2021-07-31  9:54         ` Timur Safin via Tarantool-patches
2021-07-28 10:34 ` [Tarantool-patches] [PATCH resend v2 11/11] datetime: changelog for datetime module Timur Safin via Tarantool-patches
2021-07-29 18:55 ` [Tarantool-patches] [PATCH resend v2 00/11] Initial datetime support Oleg Babin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=531544e8-0dd8-8d71-ccfc-4dae2805d498@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=tsafin@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git