[Tarantool-patches] [PATCH resend v2 09/11] lua, datetime: time intervals support
Timur Safin
tsafin at tarantool.org
Sat Jul 31 01:58:52 MSK 2021
Hi there!
: From: Oleg Babin <olegrok at tarantool.org>
: Subject: Re: [Tarantool-patches] [PATCH resend v2 09/11] lua, datetime: time
: intervals support
:
: Thanks for your patch.
:
: I wrote several comments below.
:
:
: However it makes me think that such approach will work quite slow since
:
: all functions is implemented in Lua, all arithmetic is in Lua.
I'm not that pessimistic here :) [Given the fact that I'd remove
Out of a loops all handy handlers table creation] But we need
to measure it first.
:
:
...
: > @@ -158,58 +169,146 @@ local DT_EPOCH_1970_OFFSET = 719163LL
: >
: > local datetime_t = ffi.typeof('struct datetime_t')
: > local interval_t = ffi.typeof('struct datetime_interval_t')
: > +ffi.cdef [[
: > + struct t_interval_months {
: > + int m;
: > + };
: > +
: > + struct t_interval_years {
: > + int y;
: > + };
: > +]]
: > +local interval_months_t = ffi.typeof('struct t_interval_months')
: > +local interval_years_t = ffi.typeof('struct t_interval_years')
: > +
: > +local function is_interval(o)
: > + return ffi.istype(interval_t, o) or
: > + ffi.istype(interval_months_t, o) or
: > + ffi.istype(interval_years_t, o)
: > +end
: > +
:
: It will throw for non-cdata values:
:
: tarantool> ffi.istype(interval_t, o)
: ---
: - error: 'bad argument #1 to ''?'' (C type expected, got nil)'
: ...
:
: tarantool> ffi.istype(interval_t, 123)
: ---
: - error: 'bad argument #1 to ''?'' (C type expected, got nil)'
: ...
Yes, thanks for pin-pointing it! I've modified it this way to
avoid such errors:
------------------------------------------------
@@ -182,15 +181,19 @@ local interval_months_t = ffi.typeof('struct t_interval_months')
local interval_years_t = ffi.typeof('struct t_interval_years')
local function is_interval(o)
- return ffi.istype(interval_t, o) or
- ffi.istype(interval_months_t, o) or
- ffi.istype(interval_years_t, o)
+ return type(o) == 'cdata' and
+ (ffi.istype(interval_t, o) or
+ ffi.istype(interval_months_t, o) or
+ ffi.istype(interval_years_t, o))
end
local function is_datetime(o)
- return ffi.istype(o, datetime_t)
+ return type(o) == 'cdata' and ffi.istype(o, datetime_t)
end
+local function is_date_interval(o)
+ return is_datetime(o) or is_interval(o)
+end
local function interval_new()
local interval = ffi.new(interval_t)
------------------------------------------------
: > @@ -224,19 +323,123 @@ local function interval_serialize(self)
: > return { secs = self.secs, nsec = self.nsec }
: > end
: >
: > +local function local_rd(o)
: > + return math.floor(tonumber(o.secs / SECS_PER_DAY)) +
: DT_EPOCH_1970_OFFSET
: > +end
: > +
: > +local function local_dt(o)
: > + return cdt.dt_from_rdn(local_rd(o))
: > +end
: > +
: > +local function _normalize_nsec(secs, nsec)
: > + if nsec < 0 then
: > + secs = secs - 1
: > + nsec = nsec + NANOS_PER_SEC
: > + elseif nsec >= NANOS_PER_SEC then
: > + secs = secs + 1
: > + nsec = nsec - NANOS_PER_SEC
: > + end
: > + return secs, nsec
: > +end
: > +
: > +-- addition or subtraction from date/time of a given interval
: > +-- described via table direction should be +1 or -1
: > +local function interval_increment(self, o, direction)
: > + assert(direction == -1 or direction == 1)
: > + check_date(self, "interval_increment(date, object, -+1)")
: > + assert(type(o) == 'table')
: > +
: > + local ym_updated = false
: > + local dhms_updated = false
: > +
: > + local dt = local_dt(self)
: > + local secs, nsec
: > + secs, nsec = self.secs, self.nsec
: > +
: > + for key, value in pairs(o) do
: > + local handlers = {
:
: The same as in one previous patch. It's too expensive to recreate table
: and functions for each simple
:
: action and for each iteration loop.
I've removed creation of a table, which is invariant in the loop, to
outside of loop statement. Let me measure how it much would different
than series of iffs.
---------------------------------------------------------
@@ -377,52 +382,52 @@ local function interval_increment(self, o, direction)
local secs, nsec
secs, nsec = self.secs, self.nsec
+ local handlers = {
+ years = function(k, v)
+ check_range(v, {0, 9999}, k)
+ dt = builtin.dt_add_years(dt, direction * v, builtin.DT_LIMIT)
+ ym_updated = true
+ end,
+
+ months = function(k, v)
+ check_range(v, {0, 12}, k)
+ dt = builtin.dt_add_months(dt, direction * v, builtin.DT_LIMIT)
+ ym_updated = true
+ end,
+
+ weeks = function(k, v)
+ check_range(v, {0, 52}, k)
+ secs = secs + direction * 7 * v * SECS_PER_DAY
+ dhms_updated = true
+ dhms_updated = true
+ end,
+
+ days = function(k, v)
+ check_range(v, {0, 31}, k)
+ secs = secs + direction * v * SECS_PER_DAY
+ dhms_updated = true
+ end,
+
+ hours = function(k, v)
+ check_range(v, {0, 23}, k)
+ secs = secs + direction * 60 * 60 * v
+ dhms_updated = true
+ end,
+
+ minutes = function(k, v)
+ check_range(v, {0, 59}, k)
+ secs = secs + direction * 60 * v
+ end,
+
+ seconds = function(k, v)
+ check_range(v, {0, 60}, k)
+ local s, frac = seconds_fraction(v)
+ secs = secs + direction * s
+ nsec = nsec + direction * frac * 1e9 -- convert fraction to nanoseconds
+ dhms_updated = true
+ end,
+ }
for key, value in pairs(o) do
- local handlers = {
- years = function(v)
- check_range(v, {0, 9999}, key)
- dt = cdt.dt_add_years(dt, direction * v, cdt.DT_LIMIT)
- ym_updated = true
- end,
-
- months = function(v)
- check_range(v, {0, 12}, key)
- dt = cdt.dt_add_months(dt, direction * v, cdt.DT_LIMIT)
- ym_updated = true
- end,
-
- weeks = function(v)
- check_range(v, {0, 52}, key)
- secs = secs + direction * 7 * v * SECS_PER_DAY
- dhms_updated = true
- end,
-
- days = function(v)
- check_range(v, {0, 31}, key)
- secs = secs + direction * v * SECS_PER_DAY
- dhms_updated = true
- end,
-
- hours = function(v)
- check_range(v, {0, 23}, key)
- secs = secs + direction * 60 * 60 * v
- dhms_updated = true
- end,
-
- minutes = function(v)
- check_range(v, {0, 59}, key)
- secs = secs + direction * 60 * v
- end,
-
- seconds = function(v)
- check_range(v, {0, 60}, key)
- local s, frac = seconds_fraction(v)
- secs = secs + direction * s
- nsec = nsec + direction * frac * 1e9 -- convert fraction to nanoseconds
- dhms_updated = true
- end,
- }
- handlers[key](value)
+ handlers[key](key, value)
end
secs, nsec = _normalize_nsec(secs, nsec)
---------------------------------------------------------
:
: > + years = function(v)
: > + assert(v > 0 and v < 10000)
: > + dt = cdt.dt_add_years(dt, direction * v, cdt.DT_LIMIT)
: > + ym_updated = true
: > + end,
: > +
: > + months = function(v)
: > + assert(v > 0 and v < 13 )
: > + dt = cdt.dt_add_months(dt, direction * v, cdt.DT_LIMIT)
: > + ym_updated = true
: > + end,
: > +
: > + weeks = function(v)
: > + assert(v > 0 and v < 32)
: > + secs = secs + direction * 7 * v * SECS_PER_DAY
: > + dhms_updated = true
: > + end,
: > +
: > + days = function(v)
: > + assert(v > 0 and v < 32)
: > + secs = secs + direction * v * SECS_PER_DAY
: > + dhms_updated = true
: > + end,
: > +
: > + hours = function(v)
: > + assert(v >= 0 and v < 24)
: > + secs = secs + direction * 60 * 60 * v
: > + dhms_updated = true
: > + end,
: > +
: > + minutes = function(v)
: > + assert(v >= 0 and v < 60)
: > + secs = secs + direction * 60 * v
: > + end,
: > +
: > + seconds = function(v)
: > + assert(v >= 0 and v < 61)
: > + local s, frac
: > + frac = v % 1
: > + if frac > 0 then
: > + s = v - (v % 1)
: > + else
: > + s = v
: > + end
: > + secs = secs + direction * s
: > + nsec = nsec + direction * frac * 1e9 -- convert fraction
: to nanoseconds
: > + dhms_updated = true
: > + end,
: > + }
: > + handlers[key](value)
: > + end
: > +
: > + secs, nsec = _normalize_nsec(secs, nsec)
: > +
: > + -- .days, .hours, .minutes, .seconds
: > + if dhms_updated then
: > + self.secs = secs
: > + self.nsec = nsec
: > + end
: > +
: > + -- .years, .months updated
: > + if ym_updated then
: > + self.secs = (cdt.dt_rdn(dt) - DT_EPOCH_1970_OFFSET) *
: SECS_PER_DAY +
: > + secs % SECS_PER_DAY
: > + end
: > +
: > + return self
: > +end
: > +
:
: tarantool> return require('datetime').now() + 1
: ---
: - error: '[string "return require(''datetime'').now() + 1"]:1: Usage:
: datetime:__add(interval)'
: ...
:
:
: Looks a bit confusing. User doesn't know about metamethods.
Partially agreed. Though I assume that users of this code will
be educated enough. But in any case, what would be your suggested
error message here?
Thanks,
Timur
More information about the Tarantool-patches
mailing list