From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Safin Timur <tsafin@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 11:56:16 +0300 [thread overview] Message-ID: <2aeef940-50f3-574b-4e15-8bf2159aa688@tarantool.org> (raw) In-Reply-To: <85040d30-2a0c-9ba1-5df9-46429ac4a756@tarantool.org> 18.08.2021 02:30, Safin Timur пишет: > 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. That's fine, I guess. > >> >>> --- >>> 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 Thanks for the changes! Looks good so far. I'll take a look at the whole series again once you push the updates to Vladimir's comments -- Serge Petrenko
next prev parent reply other threads:[~2021-08-18 8:56 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 2021-08-18 8:56 ` Serge Petrenko via Tarantool-patches [this message] 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=2aeef940-50f3-574b-4e15-8bf2159aa688@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