[Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime
Serge Petrenko
sergepetrenko at tarantool.org
Tue Aug 17 15:15:28 MSK 2021
16.08.2021 02:59, Timur Safin via Tarantool-patches пишет:
> * created a new Tarantool built-in module `datetime`;
> * register cdef types for this module;
> * export some `dt_*` functions from `c-dt` library;
>
> * implemented `ctime`, `asctime` and `strftime` as
> ffi wrappers in C kernel code:
>
> We used to use heavy ffi-related Lua code, but it appears
> to be quite fragile, if we run this code on different OS
> targets. To make this Lua code less platfrom dependent we
> have moved all platform specifics to the C level.
> Specifically we want to avoid dependence on `struct tm {}`
> layout and GLIBC-specific `strftime` behaviour (which
> differs to BSD/Mac LIBC behaviour).
>
> * introduced simple datetime tests
>
> Created app-tap test for new builtin module `datetime.lua`,
> where we specifically check:
>
> - cases of simple datetime string formatting using:
> - asctime (gmt time);
> - ctime (local TZ time);
> - strftime (using given format).
>
> - verify that we parse expected, and fail with unexpected:
> - for that, in the datetime.lua, we have extended api of `parse_date()`,
> `parse_time()`, and `parse_time_zone()` so they return
> not only parsed object, but also a length of parsed substring;
> - which allows us to parse even _partially_ valid strings like
> "20121224 Foo bar".
>
> - 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.
> ---
> 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?
> 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
> +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.
*/
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.
And I'd move the "#pragma once" below the license comment.
Otherwise it's easily lost. Up to you.
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <stdio.h>
AFAICS you don't need stdio included here.
> +#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.
> +#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.
> + /** 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.
> +/**
> + * 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 "*/"
> + 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.
> + 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>
>
>
--
Serge Petrenko
More information about the Tarantool-patches
mailing list