Re: [Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime

[Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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 <COPYRIGHT HOLDER> ``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
>>> + * <COPYRIGHT HOLDER> 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 <string.h>
>>> +#include <time.h>
>>> +
>>> +#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 <COPYRIGHT HOLDER> ``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
>>> + * <COPYRIGHT HOLDER> 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 <stdint.h>
>>> +#include <stdbool.h>
>>> +#include <stdio.h>
>>> +#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<uint64_t>
> ...
> ```
> 
>>
>>> +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.