[Tarantool-patches] [PATCH v5 3/8] lua, datetime: display datetime
Safin Timur
tsafin at tarantool.org
Wed Aug 18 17:10:50 MSK 2021
On 17.08.2021 20:06, Vladimir Davydov via Tarantool-patches wrote:
> On Mon, Aug 16, 2021 at 02:59:37AM +0300, Timur Safin via
> Tarantool-patches wrote:
>> * 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
>
> /**
> * ...
>
> (according to our coding style)
AFAIK external comments (in headers) should be doxygen, but internal
comments (in the implementation) may be not in doxygen format. That's
internal comment.
>
>> + * 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;
>
> Please use SNPRINT helper.
Yes, already has have it reimplemented after Serge Petrenko comment.
It's a little bit shorter now. Nice!
>
>> +
>> + 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;
>
> Please define on a separate line:
>
> int hour = ...;
> int minute = ...;
Done.
>
>> + sec = secs % 60;
>
> sec, secs, oh
>
> Please use better names.
Renamed.
>
>> + ns = date->nsec;
>> +
>> + int ret = 0;
>> + uint32_t sz = snprintf(buf, len, "%04d-%02d-%02dT%02d:%02d",
>> + year, month, day, hour, minute);
>> + ADVANCE(sz);
>> + 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 {
>
> Bad formatting. Should be '} else {'.
Fixed.
>
>> + if (offset < 0)
>> + sign = '-', offset = -offset;
>
> Please don't abuse ','.
>
>> + else
>> + sign = '+';
>> +
>> + sz = snprintf(buf, len, "%c%02d:%02d", sign, offset / 60,
> offset % 60);
>> + ADVANCE(sz);
>> + }
>> + return ret;
>> +}
>> +#undef ADVANCE
>> +
>> diff --git a/src/lib/core/datetime.h b/src/lib/core/datetime.h
>> index 1a8d7e34f..964e76fcc 100644
>> --- a/src/lib/core/datetime.h
>> +++ b/src/lib/core/datetime.h
>> @@ -70,6 +70,15 @@ struct datetime_interval {
>> int32_t nsec;
>> };
>>
>> +/**
>> + * Convert datetime to string using default format
>> + * @param date source datetime value
>> + * @param buf output character buffer
>> + * @param len size ofoutput buffer
>> + */
>> +int
>> +datetime_to_string(const struct datetime *date, char *buf, uint32_t
> len);
>> +
>
> This should be an snprint-like function: buf and len should go first.
> This would allow us to use it in conjunction with SNPRINT.
Will reorder.
[BTW, if we keep argument at the same order then for x64 ABI, with
fastcall and upto 6 arguments passed via registers RDI, RSI, RDX, RCX,
R8, R9, then compiler may keep arguments in the same registers without
any spilling to the stack]
>
>> /**
>> * Convert datetime to string using default asctime format
>> * "Sun Sep 16 01:03:52 1973\n\0"
>> diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
>> index ce579828f..4d946f194 100644
>> --- a/src/lua/datetime.lua
>> +++ b/src/lua/datetime.lua
>> @@ -249,6 +249,37 @@ local function datetime_new(obj)
>> return datetime_new_dt(dt, secs, frac, offset)
>> end
>>
>> +local function datetime_tostring(o)
>
> Please add a comment to this function with example output.
Documented.
>
>> + if ffi.typeof(o) == datetime_t then
>> + local sz = 48
>> + local buff = ffi.new('char[?]', sz)
>> + local len = builtin.datetime_to_string(o, buff, sz)
>> + assert(len < sz)
>> + return ffi.string(buff)
>> + elseif ffi.typeof(o) == interval_t then
>
> Please define separate functions for interval and datetime.
Yes, that will simplify. Part of a corresponding diff below...
----------------------------------------
@@ -373,9 +366,16 @@ local function datetime_new(obj)
secs = h * 3600 + m * 60 + s
end
- return datetime_new_dt(dt, secs, frac, offset)
+ return datetime_new_dt(dt, secs, nsec, offset)
end
+--[[
+ Convert to text datetime values
+
+ - datetime will use ISO-8601 forat:
+ 1970-01-01T00:00Z
+ 2021-08-18T16:57:08.981725+03:00
+]]
local function datetime_tostring(o)
if ffi.typeof(o) == datetime_t then
local sz = 48
@@ -383,7 +383,25 @@ local function datetime_tostring(o)
local len = builtin.datetime_to_string(o, buff, sz)
assert(len < sz)
return ffi.string(buff)
- elseif ffi.typeof(o) == interval_years_t then
+ end
+end
+
+--[[
+ Convert to text interval values of different types
+
+ - depending on a values stored there generic interval
+ values may display in following format:
+ +12 secs
+ -23 minutes, 0 seconds
+ +12 hours, 23 minutes, 1 seconds
+ -7 days, 23 hours, 23 minutes, 1 seconds
+ - years will be displayed as
+ +10 years
+ - months will be displayed as:
+ +2 months
+]]
+local function interval_tostring(o)
+ if ffi.typeof(o) == interval_years_t then
return ('%+d years'):format(o.y)
elseif ffi.typeof(o) == interval_months_t then
return ('%+d months'):format(o.m)
@@ -828,7 +901,7 @@ local datetime_mt = {
}
local interval_mt = {
- __tostring = datetime_tostring,
+ __tostring = interval_tostring,
__serialize = interval_serialize,
__eq = datetime_eq,
__lt = datetime_lt,
@@ -839,7 +912,7 @@ local interval_mt = {
}
local interval_tiny_mt = {
- __tostring = datetime_tostring,
+ __tostring = interval_tostring,
__serialize = interval_serialize,
__sub = datetime_sub,
__add = datetime_add,
----------------------------------------
>
>> + local ts = o.timestamp
>> + local sign = '+'
>> +
>> + if ts < 0 then
>> + ts = -ts
>> + sign = '-'
>> + end
>> +
>> + if ts < 60 then
>> + return ('%s%s secs'):format(sign, ts)
>> + elseif ts < 60 * 60 then
>> + return ('%+d minutes, %s seconds'):format(o.minutes, ts %
> 60)
>> + elseif ts < 24 * 60 * 60 then
>> + return ('%+d hours, %d minutes, %s seconds'):format(
>> + o.hours, o.minutes % 60, ts % 60)
>> + else
>> + return ('%+d days, %d hours, %d minutes, %s
> seconds'):format(
>> + o.days, o.hours % 24, o.minutes % 60, ts % 60)
>> + end
>> + end
>> +end
>> +
>> +
>> --[[
>> Basic Extended
>> 20121224 2012-12-24 Calendar date (ISO 8601)
>> @@ -457,6 +488,7 @@ local function strftime(fmt, o)
>> end
>>
>> local datetime_mt = {
>> + __tostring = datetime_tostring,
>> __serialize = datetime_serialize,
>> __eq = datetime_eq,
>> __lt = datetime_lt,
>> @@ -466,6 +498,7 @@ local datetime_mt = {
>> }
>>
>> local interval_mt = {
>> + __tostring = datetime_tostring,
>> __serialize = interval_serialize,
>> __eq = datetime_eq,
>> __lt = datetime_lt,
>> @@ -487,6 +520,8 @@ return setmetatable(
>> parse_time = parse_time,
>> parse_zone = parse_zone,
>>
>> + tostring = datetime_tostring,
>> +
>
> I don't think we need this function in the module. Global tostring() is
> enough.
>
-----------------------------------------
@@ -862,15 +935,12 @@ return setmetatable(
hours = interval_hours_new,
minutes = interval_minutes_new,
seconds = interval_seconds_new,
- interval = interval_new,
parse = parse,
parse_date = parse_date,
parse_time = parse_time,
parse_zone = parse_zone,
- tostring = datetime_tostring,
-
now = local_now,
strftime = strftime,
asctime = asctime,
-----------------------------------------
Deleted.
Thanks,
Timur
More information about the Tarantool-patches
mailing list