Tarantool development patches archive
 help / color / mirror / Atom feed
From: Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Serge Petrenko <sergepetrenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime
Date: Wed, 18 Aug 2021 02:30:57 +0300	[thread overview]
Message-ID: <85040d30-2a0c-9ba1-5df9-46429ac4a756@tarantool.org> (raw)
In-Reply-To: <56906a41-d62c-96b9-6152-d01989b74acb@tarantool.org>

On 17.08.2021 15:15, Serge Petrenko wrote:
> 
> 
>> - check calculated attributes to date object, e.g.:
>>    - timestamp, seconds, microseconds, minute, or hours
>>    - to_utc(), and to_tz() allow to switch timezone of a
>>      datetime object. It's not changing much - only timezone
>>      but that impacts textual representation of a date.
>>
>> Part of #5941
> 
> Please, add a docbot request to the commit message.
> Here it should say that you introduce lua datetime module
> and describe shortly what the module does.

I'm planning to cheat here - to not put detailed description to docbot 
request part here, but rather to refer to the externally available 
documentation I've written in 
https://github.com/tarantool/tarantool/discussions/6244#discussioncomment-1043988

Like:

@TarantoolBot document
Title: Introduced new built-in `datetime` module


`datetime` module has been introduced, which allows to parse ISO-8601 
literals representing timestamps of various formats, and then manipulate 
with date objects.

Please refer to https://github.com/tarantool/tarantool/discussions/6244 
for more detailed description of module API.

> 
>> ---
>>   cmake/BuildCDT.cmake                          |   2 +
>>   extra/exports                                 |  26 +
>>   src/CMakeLists.txt                            |   2 +
>>   src/lib/core/CMakeLists.txt                   |   1 +
>>   src/lib/core/datetime.c                       |  96 ++++
>>   src/lib/core/datetime.h                       |  95 ++++
>>   src/lua/datetime.lua                          | 500 ++++++++++++++++++
>>   src/lua/init.c                                |   4 +-
>>   src/lua/utils.c                               |  27 +
>>   src/lua/utils.h                               |  12 +
>>   test/app-tap/datetime.test.lua                | 206 ++++++++
>>   .../gh-5632-6050-6259-gc-buf-reuse.test.lua   |  74 ++-
>>   12 files changed, 1043 insertions(+), 2 deletions(-)
>>   create mode 100644 src/lib/core/datetime.c
>>   create mode 100644 src/lib/core/datetime.h
>>   create mode 100644 src/lua/datetime.lua
>>   create mode 100755 test/app-tap/datetime.test.lua
>>
>> diff --git a/cmake/BuildCDT.cmake b/cmake/BuildCDT.cmake
>> index 343fb1b99..80b26c64a 100644
>> --- a/cmake/BuildCDT.cmake
>> +++ b/cmake/BuildCDT.cmake
>> @@ -5,4 +5,6 @@ macro(libccdt_build)
>>       file(MAKE_DIRECTORY 
>> ${CMAKE_CURRENT_BINARY_DIR}/third_party/c-dt/build/)
>>       add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/c-dt
>>                        
>> ${CMAKE_CURRENT_BINARY_DIR}/third_party/c-dt/build/)
>> +    set_target_properties(cdt PROPERTIES COMPILE_FLAGS 
>> "-DDT_NAMESPACE=tnt_")
>> +    add_definitions("-DDT_NAMESPACE=tnt_")
>>   endmacro()
> 
> 
> This change belongs to the previous commit, doesn't it?

I considered that, but decided that for better observability purposes I 
want to keep this rename of symbols via `tnt_` prefix closer to the code 
where we exports those functions (please see `tnt_dt_dow` and others 
below in the /extra/exports).

[But taking into account that we gonna squash these commits I don't care 
that much now]

> 
> 
>> 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_from_string
>>   decimal_unpack

These guys renamed here
|
V
>> +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
>> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
>> index 97b0cb326..4473ff1da 100644
>> --- a/src/CMakeLists.txt
>> +++ b/src/CMakeLists.txt
>> @@ -51,6 +51,8 @@ lua_source(lua_sources ../third_party/luafun/fun.lua)
>>   lua_source(lua_sources lua/httpc.lua)
>>   lua_source(lua_sources lua/iconv.lua)
>>   lua_source(lua_sources lua/swim.lua)
>> +lua_source(lua_sources lua/datetime.lua)
>> +
>>   # LuaJIT jit.* library
>>   lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/bc.lua)
>>   lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/bcsave.lua)
>> diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt
>> index 2cd4d0b4f..8bc776b82 100644
>> --- a/src/lib/core/CMakeLists.txt
>> +++ b/src/lib/core/CMakeLists.txt
>> @@ -30,6 +30,7 @@ set(core_sources
>>       decimal.c
>>       mp_decimal.c
>>       cord_buf.c
>> +    datetime.c
>>   )
>>   if (TARGET_OS_NETBSD)
>> diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c
>> 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.
>> + */
>> +
> 
> As far as I know, we switched to the following license format recently:
> 
> /*
>   * SPDX-License-Identifier: BSD-2-Clause
>   *
>   * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
>   */

Much shorter - I like it. Updated.

> 
> See, for example:
> 
> ./src/box/module_cache.c: * SPDX-License-Identifier: BSD-2-Clause
> ./src/box/lua/lib.c: * SPDX-License-Identifier: BSD-2-Clause
> ./src/box/lua/lib.h: * SPDX-License-Identifier: BSD-2-Clause
> ./src/box/module_cache.h: * SPDX-License-Identifier: BSD-2-Clause
> ./src/lib/core/cord_buf.c: * SPDX-License-Identifier: BSD-2-Clause
> ./src/lib/core/crash.c: * SPDX-License-Identifier: BSD-2-Clause
> ./src/lib/core/cord_buf.h: * SPDX-License-Identifier: BSD-2-Clause
> ./src/lib/core/crash.h: * SPDX-License-Identifier: BSD-2-Clause
> 
> 
> <stripped>
> 
> 
>> diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
>> 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.
>> + */
>> +
> 
> Same about the license.

Updated

> 
> And I'd move the "#pragma once" below the license comment.
> Otherwise it's easily lost. Up to you.

On one side I agreed that it might left unrecognizable for untrained 
eye. But on other side - the sooner compiler/preprocessor will see, the 
better :)

[And there is already well established convention to put it at the first 
line.]

So I've left it on the 1st line.

> 
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +#include <stdio.h>
> 
> AFAICS you don't need stdio included here.

Indeed!

> 
>> +#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
> 
> Please, add a short comment on what this is.
> I had to spend some time googling to understand.
> 
> So, please mention that this is measured in days from 01-01-0001.

I've written some explanation about these magic numbers. Now it's 
verboser a bit:
--------------------------------------------------------
/**
  * 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
--------------------------------------------------------

also, for the MP-related patch, I've added this comment, and defines 
(might be used in asserts):

--------------------------------------------------------
/**
  * 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)
--------------------------------------------------------

> 
>> +#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;
>> +    /** nanoseconds if any */
>> +    int32_t nsec;
> 
> 
> As discussed, let's make nsec a uint32_t, since
> nsec part is always positive.

Changed.

> 
> 
>> +    /** offset in minutes from UTC */
>> +    int32_t offset;
>> +};
>> +
>> +/**
>> + * Date/time interval structure
>> + */
>> +struct datetime_interval {
>> +    /** relative seconds delta */
>> +    double secs;
>> +    /** nanoseconds delta */
>> +    int32_t nsec;
>> +};
>> +
> 
> 
> Please start comments with a capital letter and end them with a dot.

Done.

> 
> 
>> +/**
>> + * 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);
>> +
>> +#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.
>> +
>> +    */
> 
> 
> I'd move the comments outside the ffi.cdef block. This way they'd get
> proper highlighting, and it would be harder to mess something up
> by accidentally deleting the "*/"

Extracted.

> 
> 
>> +    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);
>> +
>> +    // datetime.c
> 
> 
> Also you may split the definitions into multiple ffi.cdef[[]] blocks
> if you want to add some per-definition comments.
> 

Have split it into several. Like that
------------------------------------
diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
index ce579828f..7601421b1 100644
--- a/src/lua/datetime.lua
+++ b/src/lua/datetime.lua
@@ -1,8 +1,6 @@
  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).

@@ -14,22 +12,27 @@ ffi.cdef [[
          dt = (secs / 86400) + 719163
      Where 719163 is an offset of Unix Epoch (1970-01-01) since Rata Die
      (0001-01-01) in dates.
+]]

-    */
+-- dt_core.h definitions
+ffi.cdef [[
      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
+-- 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);
+]]

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

@@ -45,7 +48,6 @@ ffi.cdef [[

      void
      datetime_now(struct datetime * now);
-
  ]]

  local builtin = ffi.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);
>> +
>> +]]
> 
> 
> <stripped>
> 
> 
>> diff --git a/test/app-tap/datetime.test.lua 
>> b/test/app-tap/datetime.test.lua
>> new file mode 100755
>> index 000000000..464d4bd49
>> --- /dev/null
>> +++ b/test/app-tap/datetime.test.lua
>> @@ -0,0 +1,206 @@
>> +#!/usr/bin/env tarantool
>> +
>> +local tap = require('tap')
>> +local test = tap.test("errno")
>> +local date = require('datetime')
>> +local ffi = require('ffi')
>> +
>> +
>> +test:plan(6)
>> +
>> +test:test("Simple tests for parser", function(test)
>> +    test:plan(2)
>> +    test:ok(date("1970-01-01T01:00:00Z") ==
>> +            date {year=1970, month=1, day=1, hour=1, minute=0, 
>> second=0})
>> +    test:ok(date("1970-01-01T02:00:00+02:00") ==
>> +            date {year=1970, month=1, day=1, hour=2, minute=0, 
>> second=0, tz=120})
>> +end)
>> +
>> +test:test("Multiple tests for parser (with nanoseconds)", function(test)
>> +    test:plan(168)
>> +    -- 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 },
>> +    }
>> +    for _, value in ipairs(tests) do
>> +        local str, epoch, nsec, offset
>> +        str, epoch, nsec, offset = 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))
>> +    end
>> +end)
>> +
>> +ffi.cdef [[
>> +    void tzset(void);
>> +]]
>> +
>>
> 
> 
> <stripped>
> 
> 
> 

Here is (was) incremental patch. [Now it's slightly changed, with 
MP-related defines, but you got the point]:
------------------------------------------------------------------
diff --git a/src/lib/core/datetime.c b/src/lib/core/datetime.c
index c48295a6f..719a4cd47 100644
--- a/src/lib/core/datetime.c
+++ b/src/lib/core/datetime.c
@@ -1,32 +1,7 @@
  /*
- * 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.
+ * SPDX-License-Identifier: BSD-2-Clause
   *
- * 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.
+ * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file.
   */

  #include <string.h>
diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
index 1a8d7e34f..88774110c 100644
--- a/src/lib/core/datetime.h
+++ b/src/lib/core/datetime.h
@@ -1,38 +1,12 @@
  #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.
+ * SPDX-License-Identifier: BSD-2-Clause
   *
- * 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.
+ * Copyright 2021, Tarantool AUTHORS, please see AUTHORS file.
   */

  #include <stdint.h>
  #include <stdbool.h>
-#include <stdio.h>
  #include "c-dt/dt.h"

  #if defined(__cplusplus)
@@ -40,23 +14,34 @@ extern "C"
  {
  #endif /* defined(__cplusplus) */

+/**
+ * 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

  /**
- * Full datetime structure representing moments
- * since Unix Epoch (1970-01-01).
- * Time is kept normalized to UTC, time-zone offset
+ * datetime structure keeps number of seconds since
+ * Unix Epoch.
+ * Time is normalized by UTC, so time-zone offset
   * is informative only.
   */
  struct datetime {
-       /** seconds since epoch */
+       /** Seconds since Epoch. */
         double secs;
-       /** nanoseconds if any */
-       int32_t nsec;
-       /** offset in minutes from UTC */
+       /** Nanoseconds, if any. */
+       uint32_t nsec;
+       /** Offset in minutes from UTC. */
         int32_t offset;
  };

@@ -64,10 +49,10 @@ struct datetime {
   * Date/time interval structure
   */
  struct datetime_interval {
-       /** relative seconds delta */
+       /** Relative seconds delta. */
         double secs;
-       /** nanoseconds delta */
-       int32_t nsec;
+       /** Nanoseconds delta, if any. */
+       uint32_t nsec;
  };

  /**
------------------------------------------------------------------

Thanks,
Timur

  reply	other threads:[~2021-08-17 23:31 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-15 23:59 [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation Timur Safin via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 1/8] build: add Christian Hansen c-dt to the build Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:24     ` Safin Timur via Tarantool-patches
2021-08-18  8:56       ` Serge Petrenko via Tarantool-patches
2021-08-17 15:50   ` Vladimir Davydov via Tarantool-patches
2021-08-18 10:04     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:30     ` Safin Timur via Tarantool-patches [this message]
2021-08-18  8:56       ` Serge Petrenko via Tarantool-patches
2021-08-17 16:52   ` Vladimir Davydov via Tarantool-patches
2021-08-17 19:16     ` Vladimir Davydov via Tarantool-patches
2021-08-18 13:38       ` Safin Timur via Tarantool-patches
2021-08-18 10:03     ` Safin Timur via Tarantool-patches
2021-08-18 10:06       ` Safin Timur via Tarantool-patches
2021-08-18 11:45       ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 3/8] lua, datetime: display datetime Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:32     ` Safin Timur via Tarantool-patches
2021-08-17 17:06   ` Vladimir Davydov via Tarantool-patches
2021-08-18 14:10     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 4/8] box, datetime: messagepack support for datetime Timur Safin via Tarantool-patches
2021-08-16  0:20   ` Safin Timur via Tarantool-patches
2021-08-17 12:15     ` Serge Petrenko via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:42     ` Safin Timur via Tarantool-patches
2021-08-18  9:01       ` Serge Petrenko via Tarantool-patches
2021-08-17 18:36   ` Vladimir Davydov via Tarantool-patches
2021-08-18 14:27     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 5/8] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:43     ` Safin Timur via Tarantool-patches
2021-08-18  9:03       ` Serge Petrenko via Tarantool-patches
2021-08-17 19:05   ` Vladimir Davydov via Tarantool-patches
2021-08-18 17:18     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 6/8] lua, datetime: time intervals support Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:44     ` Safin Timur via Tarantool-patches
2021-08-17 18:52   ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 7/8] datetime: perf test for datetime parser Timur Safin via Tarantool-patches
2021-08-17 19:13   ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 8/8] datetime: changelog for datetime module Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:44     ` Safin Timur via Tarantool-patches
2021-08-18  9:04       ` Serge Petrenko via Tarantool-patches
2021-08-17 12:15 ` [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation Serge Petrenko via Tarantool-patches
     [not found] ` <20210818082222.mofgheciutpipelz@esperanza>
2021-08-18  8:25   ` Vladimir Davydov via Tarantool-patches
2021-08-18 13:24     ` Safin Timur via Tarantool-patches
2021-08-18 14:22       ` Vladimir Davydov 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=85040d30-2a0c-9ba1-5df9-46429ac4a756@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergepetrenko@tarantool.org \
    --cc=tsafin@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox