[Tarantool-patches] [PATCH resend v2 00/11] Initial datetime support
Oleg Babin
olegrok at tarantool.org
Thu Jul 29 21:55:28 MSK 2021
Thanks for your patch.
I can't say that it will be complete review but it's brief feedback.
I left comments for some patches. Currently it is not complete series,
in fact it looks like as one-two commits with portion of fixups on the
top of them.
I believe this module should be implemented in pure C. I don't think
that arithmetic will
work fast enough in C.
Probably datetime and interval should be logically splitted into 2 parts.
```
tarantool> require('datetime')
---
- weeks: 'function: 0x010ca0aa48'
minutes: 'function: 0x010ca694d8'
new: 'function: 0x010ca5e098'
strftime: 'function: 0x010caa8fb0'
tostring: 'function: 0x010ca2f700'
hours: 'function: 0x010ca671c0'
parse_date: 'function: 0x010ca96f80'
seconds: 'function: 0x010ca63b78'
parse_time: 'function: 0x010ca22208'
is_interval: 'function: 0x010caa6d20'
months: 'function: 0x010ca61130'
is_datetime: 'function: 0x010ca5d3a0'
asctime: 'function: 0x010caa8e28'
ctime: 'function: 0x010ca88260'
parse_zone: 'function: 0x010ca02eb0'
now: 'function: 0x010ca86e70'
days: 'function: 0x010ca81980'
years: 'function: 0x010ca61438'
parse: 'function: 0x010ca9ad38'
interval: 'function: 0x010c9feb60'
...
```
It's not clear that part is about intervals and datetime.
```
-- Currently
datetime.now() + datetime.week(1)
-- Possible version
datetime.now() + interval.week(1)
```
Interface is better than in previous time. Thanks for working on this
series.
On 28.07.2021 13:34, Timur Safin via Tarantool-patches wrote:
> * Version #2:
>
> - fixed problem with overloaded '-' and '+' operations for datetime
> arguments;
> - fixed messagepack serialization problems;
> - heavily documented MessagePack serialization schema in the code;
> - introduced working implementation of datetime hints for storage engines;
> - made interval related names be more consistent, renamed durations and period
> to intervals, i.e. t_datetime_duration to datetime_interval_t,
> duration_* to interval_*, period to interval;
> - properly implemented all reasonable cases of datetime+interval
> arithmetic;
> - moved all initialization code to utils.c;
> - renamed core/mp_datetime.c to core/datetime.c because it makes more
> sense now;
>
> * Version #1 - initial RFC series
>
> In brief
> --------
> This patchset implements datetime lua support in box, with serialization
> to messagepack, yaml, json and lua mode. Also it contains storage
> engines' indices implementation for datetime type introduced.
>
> * Current implementation is heavily influenced by Sci-Lua lua-time module
> https://github.com/stepelu/lua-time
> e.g. you could find very similar approach for handling of operations
> with year or month long intervals (which should be handled differently than
> usual intervals of seconds, or days).
>
> * But internally we actually use Christian Hanson' c-dt module
> https://github.com/chansen/c-dt
> (though it has been modified slightly for cleaner integration
> into cmake build process)
>
>
> Datetime Module API
> -------------------
>
> We used to have here draft documentation of datetime module api, but
> for a convenience it has been extracted to the discussion topic there -
> https://github.com/tarantool/tarantool/discussions/6244#discussioncomment-1043988
>
> Messagepack serialization schema
> --------------------------------
>
> In short it looks like:
> - now we introduce new MP_EXT extension type #4;
> - we may save 1 required and 2 optional fields for datetime field;
>
> In details it's explained in MessagePack serialization schema depicted here:
> https://github.com/tarantool/tarantool/discussions/6244#discussioncomment-1043990
>
>
> https://github.com/tarantool/tarantool/issues/5941
> https://github.com/tarantool/tarantool/issues/5946
>
> https://github.com/tarantool/tarantool/tree/tsafin/gh-5941-datetime-v3
>
> Timur Safin (11):
> build: add Christian Hansen c-dt to the build
> lua: built-in module datetime
> lua, datetime: datetime tests
> lua, datetime: display datetime
> box, datetime: add messagepack support for datetime
> box, datetime: datetime comparison for indices
> lua, datetime: proper datetime encoding
> lua, datetime: calculated attributes for datetimes
> lua, datetime: time intervals support
> lua, datetime: unixtime, timestamp setters in datetime.lua
> datetime: changelog for datetime module
>
> .gitmodules | 3 +
> CMakeLists.txt | 8 +
> .../gh-5941-datetime-type-support.md | 4 +
> cmake/BuildCDT.cmake | 6 +
> src/CMakeLists.txt | 3 +
> src/box/field_def.c | 52 +-
> src/box/field_def.h | 4 +
> src/box/lua/serialize_lua.c | 7 +-
> src/box/memtx_space.c | 3 +-
> src/box/msgpack.c | 7 +-
> src/box/tuple_compare.cc | 50 +
> src/box/vinyl.c | 3 +-
> src/exports.h | 29 +
> src/lib/core/CMakeLists.txt | 4 +-
> src/lib/core/datetime.c | 251 ++++
> src/lib/core/datetime.h | 114 ++
> src/lib/core/mp_extension_types.h | 1 +
> src/lib/mpstream/mpstream.c | 11 +
> src/lib/mpstream/mpstream.h | 4 +
> src/lua/datetime.lua | 1016 +++++++++++++++++
> src/lua/init.c | 4 +-
> src/lua/msgpack.c | 12 +
> src/lua/msgpackffi.lua | 18 +
> src/lua/serializer.c | 4 +
> src/lua/serializer.h | 2 +
> src/lua/utils.c | 28 +-
> src/lua/utils.h | 12 +
> test/app-tap/datetime.test.lua | 367 ++++++
> test/engine/datetime.result | 77 ++
> test/engine/datetime.test.lua | 35 +
> test/unit/CMakeLists.txt | 2 +
> test/unit/datetime.c | 220 ++++
> test/unit/datetime.result | 358 ++++++
> third_party/c-dt | 1 +
> third_party/lua-cjson/lua_cjson.c | 8 +
> third_party/lua-yaml/lyaml.cc | 6 +-
> 36 files changed, 2709 insertions(+), 25 deletions(-)
> create mode 100644 changelogs/unreleased/gh-5941-datetime-type-support.md
> create mode 100644 cmake/BuildCDT.cmake
> create mode 100755 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
> create mode 100644 test/engine/datetime.result
> create mode 100644 test/engine/datetime.test.lua
> create mode 100644 test/unit/datetime.c
> create mode 100644 test/unit/datetime.result
> create mode 160000 third_party/c-dt
>
More information about the Tarantool-patches
mailing list