[Tarantool-patches] [PATCH v5 3/8] lua, datetime: display datetime

Safin Timur tsafin at tarantool.org
Wed Aug 18 02:32:57 MSK 2021


My responses below, thanks...

On 17.08.2021 15:15, Serge Petrenko wrote:
> 
> 
> 16.08.2021 02:59, Timur Safin via Tarantool-patches пишет:
>> * introduced output routine for converting datetime
>>    to their default output format.
>>
>> * use this routine for tostring() in datetime.lua
>>
>> Part of #5941
>> ---
>>   extra/exports                  |   1 +
>>   src/lib/core/datetime.c        |  71 ++++++++++++++++++
>>   src/lib/core/datetime.h        |   9 +++
>>   src/lua/datetime.lua           |  35 +++++++++
>>   test/app-tap/datetime.test.lua | 131 ++++++++++++++++++---------------
>>   test/unit/CMakeLists.txt       |   2 +-
>>   test/unit/datetime.c           |  61 +++++++++++----
>>   7 files changed, 236 insertions(+), 74 deletions(-)
>>
>> diff --git a/extra/exports b/extra/exports
>> index 80eb92abd..2437e175c 100644
>> --- a/extra/exports
>> +++ b/extra/exports
>> @@ -152,6 +152,7 @@ datetime_asctime
>>   datetime_ctime
>>   datetime_now
>>   datetime_strftime
>> +datetime_to_string
>>   decimal_unpack
>>   decimal_from_string
>>   decimal_unpack
>> diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c
>> index c48295a6f..c24a0df82 100644
>> --- a/src/lib/core/datetime.c
>> +++ b/src/lib/core/datetime.c
>> @@ -29,6 +29,8 @@
>>    * SUCH DAMAGE.
>>    */
>> +#include <assert.h>
>> +#include <limits.h>
>>   #include <string.h>
>>   #include <time.h>
>> @@ -94,3 +96,72 @@ datetime_strftime(const struct datetime *date, 
>> const char *fmt, char *buf,
>>       struct tm *p_tm = datetime_to_tm(date);
>>       return strftime(buf, len, fmt, p_tm);
>>   }
>> +
>> +#define SECS_EPOCH_1970_OFFSET ((int64_t)DT_EPOCH_1970_OFFSET * 
>> SECS_PER_DAY)
>> +
>> +/* NB! buf may be NULL, and we should handle it gracefully, returning
>> + * calculated length of output string
>> + */
>> +int
>> +datetime_to_string(const struct datetime *date, char *buf, uint32_t len)
>> +{
>> +#define ADVANCE(sz)        \
>> +    if (buf != NULL) {     \
>> +        buf += sz;     \
>> +        len -= sz;     \
>> +    }            \
>> +    ret += sz;
>> +
>> +    int offset = date->offset;
>> +    /* for negative offsets around Epoch date we could get
>> +     * negative secs value, which should be attributed to
>> +     * 1969-12-31, not 1970-01-01, thus we first shift
>> +     * epoch to Rata Die then divide by seconds per day,
>> +     * not in reverse
>> +     */
>> +    int64_t secs = (int64_t)date->secs + offset * 60 + 
>> SECS_EPOCH_1970_OFFSET;
>> +    assert((secs / SECS_PER_DAY) <= INT_MAX);
>> +    dt_t dt = dt_from_rdn(secs / SECS_PER_DAY);
>> +
>> +    int year, month, day, sec, ns, sign;
>> +    dt_to_ymd(dt, &year, &month, &day);
>> +
>> +    int hour = (secs / 3600) % 24,
>> +        minute = (secs / 60) % 60;
>> +    sec = secs % 60;
>> +    ns = date->nsec;
>> +
>> +    int ret = 0;
>> +    uint32_t sz = snprintf(buf, len, "%04d-%02d-%02dT%02d:%02d",
>> +                   year, month, day, hour, minute);
>> +    ADVANCE(sz);
> 
> 
> Please, replace snprintf + ADVANCE() with SNPRINT macro
> from src/trivia/util.h
> 
> It does exactly what you need.

OMG! What a coincidence! Thanks!

Updated accordingly...
-----------------------------------------
diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c
index c05197efd..bdaaff555 100644
--- a/src/lib/core/datetime.c
+++ b/src/lib/core/datetime.c
@@ -78,15 +78,8 @@ datetime_strftime(const struct datetime *date, const 
char *fmt, char *buf,
   * calculated length of output string
   */
  int
-datetime_to_string(const struct datetime *date, char *buf, uint32_t len)
+datetime_to_string(const struct datetime *date, char *buf, int len)
  {
-#define ADVANCE(sz)            \
-       if (buf != NULL) {      \
-               buf += sz;      \
-               len -= sz;      \
-       }                       \
-       ret += sz;
-
         int offset = date->offset;
         /* for negative offsets around Epoch date we could get
          * negative secs value, which should be attributed to
@@ -106,26 +99,24 @@ datetime_to_string(const struct datetime *date, 
char *buf, uint32_t len)
         sec = secs % 60;
         ns = date->nsec;

-       int ret = 0;
-       uint32_t sz = snprintf(buf, len, "%04d-%02d-%02dT%02d:%02d",
-                              year, month, day, hour, minute);
-       ADVANCE(sz);
+       int sz = 0;
+       SNPRINT(sz, snprintf, buf, len, "%04d-%02d-%02dT%02d:%02d",
+               year, month, day, hour, minute);
         if (sec || ns) {
-               sz = snprintf(buf, len, ":%02d", sec);
-               ADVANCE(sz);
+               SNPRINT(sz, snprintf, buf, len, ":%02d", sec);
                 if (ns) {
                         if ((ns % 1000000) == 0)
-                               sz = snprintf(buf, len, ".%03d", ns / 
1000000);
+                               SNPRINT(sz, snprintf, buf, len, ".%03d",
+                                       ns / 1000000);
                         else if ((ns % 1000) == 0)
-                               sz = snprintf(buf, len, ".%06d", ns / 1000);
+                               SNPRINT(sz, snprintf, buf, len, ".%06d",
+                                       ns / 1000);
                         else
-                               sz = snprintf(buf, len, ".%09d", ns);
-                       ADVANCE(sz);
+                               SNPRINT(sz, snprintf, buf, len, ".%09d", 
ns);
                 }
         }
         if (offset == 0) {
-               sz = snprintf(buf, len, "Z");
-               ADVANCE(sz);
+               SNPRINT(sz, snprintf, buf, len, "Z");
         }
         else {
                 if (offset < 0)
@@ -133,10 +124,9 @@ datetime_to_string(const struct datetime *date, 
char *buf, uint32_t len)
                 else
                         sign = '+';

-               sz = snprintf(buf, len, "%c%02d:%02d", sign, offset / 
60, offset % 60);
-               ADVANCE(sz);
+               SNPRINT(sz, snprintf, buf, len, "%c%02d:%02d", sign,
+                       offset / 60, offset % 60);
         }
-       return ret;
+       return sz;
  }
-#undef ADVANCE

diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
index 3c7a7d99d..688ab59ec 100644
--- a/src/lib/core/datetime.h
+++ b/src/lib/core/datetime.h
@@ -62,7 +62,7 @@ struct datetime_interval {
   * @param len size ofoutput buffer
   */
  int
-datetime_to_string(const struct datetime *date, char *buf, uint32_t len);
+datetime_to_string(const struct datetime *date, char *buf, int len);

  /**
   * Convert datetime to string using default asctime format
diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
index 4d946f194..96ecd1fee 100644
--- a/src/lua/datetime.lua
+++ b/src/lua/datetime.lua
@@ -31,7 +31,7 @@ ffi.cdef [[

      // datetime.c
      int
-    datetime_to_string(const struct datetime * date, char *buf, 
uint32_t len);
+    datetime_to_string(const struct datetime * date, char *buf, int len);

      char *
      datetime_asctime(const struct datetime *date, char *buf);
-----------------------------------------

> 
> 
>> +    if (sec || ns) {
>> +        sz = snprintf(buf, len, ":%02d", sec);
>> +        ADVANCE(sz);
>> +        if (ns) {
>> +            if ((ns % 1000000) == 0)
>> +                sz = snprintf(buf, len, ".%03d", ns / 1000000);
>> +            else if ((ns % 1000) == 0)
>> +                sz = snprintf(buf, len, ".%06d", ns / 1000);
>> +            else
>> +                sz = snprintf(buf, len, ".%09d", ns);
>> +            ADVANCE(sz);
>> +        }
>> +    }
>> +    if (offset == 0) {
>> +        sz = snprintf(buf, len, "Z");
>> +        ADVANCE(sz);
>> +    }
>> +    else {
>> +        if (offset < 0)
>> +            sign = '-', offset = -offset;
>> +        else
>> +            sign = '+';
>> +
>> +        sz = snprintf(buf, len, "%c%02d:%02d", sign, offset / 60, 
>> offset % 60);
>> +        ADVANCE(sz);
>> +    }
>> +    return ret;
>> +}
>> +#undef ADVANCE
>> +
> 
> 
> <stripped>
> 
> 
>> diff --git a/test/app-tap/datetime.test.lua 
>> b/test/app-tap/datetime.test.lua
>> index 464d4bd49..244ec2575 100755
>> --- a/test/app-tap/datetime.test.lua
>> +++ b/test/app-tap/datetime.test.lua
>> @@ -6,7 +6,7 @@ local date = require('datetime')
>>   local ffi = require('ffi')
>> -test:plan(6)
>> +test:plan(7)
>>   test:test("Simple tests for parser", function(test)
>>       test:plan(2)
>> @@ -17,74 +17,78 @@ test:test("Simple tests for parser", function(test)
>>   end)
>>   test:test("Multiple tests for parser (with nanoseconds)", 
>> function(test)
>> -    test:plan(168)
>> +    test:plan(193)
>>       -- borrowed from p5-time-moments/t/180_from_string.t
>>       local tests =
>>       {
>> -        { '1970-01-01T00:00:00Z',                       0,           
>> 0,    0 },
>> -        { '1970-01-01T02:00:00+02:00',                  0,           
>> 0,  120 },
>> -        { '1970-01-01T01:30:00+01:30',                  0,           
>> 0,   90 },
>> -        { '1970-01-01T01:00:00+01:00',                  0,           
>> 0,   60 },
>> -        { '1970-01-01T00:01:00+00:01',                  0,           
>> 0,    1 },
>> -        { '1970-01-01T00:00:00+00:00',                  0,           
>> 0,    0 },
>> -        { '1969-12-31T23:59:00-00:01',                  0,           
>> 0,   -1 },
>> -        { '1969-12-31T23:00:00-01:00',                  0,           
>> 0,  -60 },
>> -        { '1969-12-31T22:30:00-01:30',                  0,           
>> 0,  -90 },
>> -        { '1969-12-31T22:00:00-02:00',                  0,           
>> 0, -120 },
>> -        { '1970-01-01T00:00:00.123456789Z',             0,   
>> 123456789,    0 },
>> -        { '1970-01-01T00:00:00.12345678Z',              0,   
>> 123456780,    0 },
>> -        { '1970-01-01T00:00:00.1234567Z',               0,   
>> 123456700,    0 },
>> -        { '1970-01-01T00:00:00.123456Z',                0,   
>> 123456000,    0 },
>> -        { '1970-01-01T00:00:00.12345Z',                 0,   
>> 123450000,    0 },
>> -        { '1970-01-01T00:00:00.1234Z',                  0,   
>> 123400000,    0 },
>> -        { '1970-01-01T00:00:00.123Z',                   0,   
>> 123000000,    0 },
>> -        { '1970-01-01T00:00:00.12Z',                    0,   
>> 120000000,    0 },
>> -        { '1970-01-01T00:00:00.1Z',                     0,   
>> 100000000,    0 },
>> -        { '1970-01-01T00:00:00.01Z',                    0,    
>> 10000000,    0 },
>> -        { '1970-01-01T00:00:00.001Z',                   0,     
>> 1000000,    0 },
>> -        { '1970-01-01T00:00:00.0001Z',                  0,      
>> 100000,    0 },
>> -        { '1970-01-01T00:00:00.00001Z',                 0,       
>> 10000,    0 },
>> -        { '1970-01-01T00:00:00.000001Z',                0,        
>> 1000,    0 },
>> -        { '1970-01-01T00:00:00.0000001Z',               0,         
>> 100,    0 },
>> -        { '1970-01-01T00:00:00.00000001Z',              0,          
>> 10,    0 },
>> -        { '1970-01-01T00:00:00.000000001Z',             0,           
>> 1,    0 },
>> -        { '1970-01-01T00:00:00.000000009Z',             0,           
>> 9,    0 },
>> -        { '1970-01-01T00:00:00.00000009Z',              0,          
>> 90,    0 },
>> -        { '1970-01-01T00:00:00.0000009Z',               0,         
>> 900,    0 },
>> -        { '1970-01-01T00:00:00.000009Z',                0,        
>> 9000,    0 },
>> -        { '1970-01-01T00:00:00.00009Z',                 0,       
>> 90000,    0 },
>> -        { '1970-01-01T00:00:00.0009Z',                  0,      
>> 900000,    0 },
>> -        { '1970-01-01T00:00:00.009Z',                   0,     
>> 9000000,    0 },
>> -        { '1970-01-01T00:00:00.09Z',                    0,    
>> 90000000,    0 },
>> -        { '1970-01-01T00:00:00.9Z',                     0,   
>> 900000000,    0 },
>> -        { '1970-01-01T00:00:00.99Z',                    0,   
>> 990000000,    0 },
>> -        { '1970-01-01T00:00:00.999Z',                   0,   
>> 999000000,    0 },
>> -        { '1970-01-01T00:00:00.9999Z',                  0,   
>> 999900000,    0 },
>> -        { '1970-01-01T00:00:00.99999Z',                 0,   
>> 999990000,    0 },
>> -        { '1970-01-01T00:00:00.999999Z',                0,   
>> 999999000,    0 },
>> -        { '1970-01-01T00:00:00.9999999Z',               0,   
>> 999999900,    0 },
>> -        { '1970-01-01T00:00:00.99999999Z',              0,   
>> 999999990,    0 },
>> -        { '1970-01-01T00:00:00.999999999Z',             0,   
>> 999999999,    0 },
>> -        { '1970-01-01T00:00:00.0Z',                     0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.00Z',                    0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.000Z',                   0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.0000Z',                  0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.00000Z',                 0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.000000Z',                0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.0000000Z',               0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.00000000Z',              0,           
>> 0,    0 },
>> -        { '1970-01-01T00:00:00.000000000Z',             0,           
>> 0,    0 },
>> -        { '1973-11-29T21:33:09Z',               123456789,           
>> 0,    0 },
>> -        { '2013-10-28T17:51:56Z',              1382982716,           
>> 0,    0 },
>> -        { '9999-12-31T23:59:59Z',            253402300799,           
>> 0,    0 },
>> +        {'1970-01-01T00:00Z',                  0,         0,    0, 1},
>> +        {'1970-01-01T02:00+02:00',             0,         0,  120, 1},
>> +        {'1970-01-01T01:30+01:30',             0,         0,   90, 1},
>> +        {'1970-01-01T01:00+01:00',             0,         0,   60, 1},
>> +        {'1970-01-01T00:01+00:01',             0,         0,    1, 1},
>> +        {'1970-01-01T00:00Z',                  0,         0,    0, 1},
>> +        {'1969-12-31T23:59-00:01',             0,         0,   -1, 1},
>> +        {'1969-12-31T23:00-01:00',             0,         0,  -60, 1},
>> +        {'1969-12-31T22:30-01:30',             0,         0,  -90, 1},
>> +        {'1969-12-31T22:00-02:00',             0,         0, -120, 1},
>> +        {'1970-01-01T00:00:00.123456789Z',     0, 123456789,    0, 1},
>> +        {'1970-01-01T00:00:00.12345678Z',      0, 123456780,    0, 0},
>> +        {'1970-01-01T00:00:00.1234567Z',       0, 123456700,    0, 0},
>> +        {'1970-01-01T00:00:00.123456Z',        0, 123456000,    0, 1},
>> +        {'1970-01-01T00:00:00.12345Z',         0, 123450000,    0, 0},
>> +        {'1970-01-01T00:00:00.1234Z',          0, 123400000,    0, 0},
>> +        {'1970-01-01T00:00:00.123Z',           0, 123000000,    0, 1},
>> +        {'1970-01-01T00:00:00.12Z',            0, 120000000,    0, 0},
>> +        {'1970-01-01T00:00:00.1Z',             0, 100000000,    0, 0},
>> +        {'1970-01-01T00:00:00.01Z',            0,  10000000,    0, 0},
>> +        {'1970-01-01T00:00:00.001Z',           0,   1000000,    0, 1},
>> +        {'1970-01-01T00:00:00.0001Z',          0,    100000,    0, 0},
>> +        {'1970-01-01T00:00:00.00001Z',         0,     10000,    0, 0},
>> +        {'1970-01-01T00:00:00.000001Z',        0,      1000,    0, 1},
>> +        {'1970-01-01T00:00:00.0000001Z',       0,       100,    0, 0},
>> +        {'1970-01-01T00:00:00.00000001Z',      0,        10,    0, 0},
>> +        {'1970-01-01T00:00:00.000000001Z',     0,         1,    0, 1},
>> +        {'1970-01-01T00:00:00.000000009Z',     0,         9,    0, 1},
>> +        {'1970-01-01T00:00:00.00000009Z',      0,        90,    0, 0},
>> +        {'1970-01-01T00:00:00.0000009Z',       0,       900,    0, 0},
>> +        {'1970-01-01T00:00:00.000009Z',        0,      9000,    0, 1},
>> +        {'1970-01-01T00:00:00.00009Z',         0,     90000,    0, 0},
>> +        {'1970-01-01T00:00:00.0009Z',          0,    900000,    0, 0},
>> +        {'1970-01-01T00:00:00.009Z',           0,   9000000,    0, 1},
>> +        {'1970-01-01T00:00:00.09Z',            0,  90000000,    0, 0},
>> +        {'1970-01-01T00:00:00.9Z',             0, 900000000,    0, 0},
>> +        {'1970-01-01T00:00:00.99Z',            0, 990000000,    0, 0},
>> +        {'1970-01-01T00:00:00.999Z',           0, 999000000,    0, 1},
>> +        {'1970-01-01T00:00:00.9999Z',          0, 999900000,    0, 0},
>> +        {'1970-01-01T00:00:00.99999Z',         0, 999990000,    0, 0},
>> +        {'1970-01-01T00:00:00.999999Z',        0, 999999000,    0, 1},
>> +        {'1970-01-01T00:00:00.9999999Z',       0, 999999900,    0, 0},
>> +        {'1970-01-01T00:00:00.99999999Z',      0, 999999990,    0, 0},
>> +        {'1970-01-01T00:00:00.999999999Z',     0, 999999999,    0, 1},
>> +        {'1970-01-01T00:00:00.0Z',             0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.00Z',            0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.000Z',           0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.0000Z',          0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.00000Z',         0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.000000Z',        0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.0000000Z',       0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.00000000Z',      0,         0,    0, 0},
>> +        {'1970-01-01T00:00:00.000000000Z',     0,         0,    0, 0},
>> +        {'1973-11-29T21:33:09Z',       123456789,         0,    0, 1},
>> +        {'2013-10-28T17:51:56Z',      1382982716,         0,    0, 1},
>> +        {'9999-12-31T23:59:59Z',    253402300799,         0,    0, 1},
> 
> 
> Please, squash this change into the previous commit.

Surprisingly, that was original configuration of patches, then I've 
split it to smaller chunks.

But yes, this test dependency shows that code should be in a single 
patch. Will do. [Will not push updated branch yet, to accumulate changes 
for Vova feedback]

> 
> 
>>       }
>>       for _, value in ipairs(tests) do
>> -        local str, epoch, nsec, offset
>> -        str, epoch, nsec, offset = unpack(value)
>> +        local str, epoch, nsec, offset, check
>> +        str, epoch, nsec, offset, check = unpack(value)
>>           local dt = date(str)
>>           test:ok(dt.secs == epoch, ('%s: dt.secs == %d'):format(str, 
>> epoch))
>>           test:ok(dt.nsec == nsec, ('%s: dt.nsec == %d'):format(str, 
>> nsec))
>>           test:ok(dt.offset == offset, ('%s: dt.offset == 
>> %d'):format(str, offset))
>> +        if check > 0 then
>> +            test:ok(str == tostring(dt), ('%s == tostring(%s)'):
>> +                    format(str, tostring(dt)))
>> +        end
>>       end
>>   end)
>> @@ -203,4 +207,11 @@ test:test("Parse tiny date into seconds and other 
>> parts", function(test)
>>       test:ok(tiny.hours == 0.00848, "hours")
>>   end)
>>
>>
> 
> 
> <stripped>
> 

Thanks,
Timur


More information about the Tarantool-patches mailing list