Skip to content

Commit

Permalink
datetime: make timezones only affect formatting
Browse files Browse the repository at this point in the history
This patch makes `datetime` handle specified `tz` or `tzoffset`
properly. It means timezones are now only affect the way datetime
objects are formatted instead of timestamp value they represent.

The patch involves this changes in the behavior:
1. Creating/setting a `timestamp` value with `tz`/`tzoffset` results
   with a day time corresponding to the timestamp value plus the
   timezone offset. E.g if we create a datetime object with a timestamp
   corresponding to 9:12 o'clock in `+0000` timezone and set
   `tzoffset = -60` the resulting timestamp will represent 8:12 o'clock.
2. Setting a new `tz`/`tzoffset` affects the time of day represented by
   the datetime. E.g. if you update a datetime object of 11:12 o'clock
   having `tzoffset = 180` with `tzoffset = 120` the resulting datetime
   object will represent 10:12.

Closes tarantool#10363

NO_DOC=will be in tarantool/doc#4720
  • Loading branch information
georgiy-belyanin committed Dec 25, 2024
1 parent c50d2bb commit 0b4b431
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 13 deletions.
5 changes: 5 additions & 0 deletions changelogs/unreleased/gh-10363-datetime-handle-timezone.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
## bugfix/datetime

* `datetime` methods now handle timezones properly. Now timezone changes
only affect the way the object is formatted and don't alter the
represented timestamp (gh-10363).
30 changes: 28 additions & 2 deletions src/lua/datetime.lua
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,9 @@ local function datetime_new(obj)
s = s - 1
fraction = fraction + 1
end

s = s + (offset or 0) * 60

-- if there are separate nsec, usec, or msec provided then
-- timestamp should be integer
if count_usec == 0 then
Expand Down Expand Up @@ -939,7 +942,20 @@ local function datetime_parse_from(str, obj)
-- Override timezone, if it was not specified in a parsed
-- string.
if date.tz == '' and date.tzoffset == 0 then
datetime_set(date, { tzoffset = tzoffset, tz = tzname })
local offset = nil

if tzoffset ~= nil then
offset = get_timezone(tzoffset, 'tzoffset')
check_range(offset, -720, 840, 'tzoffset')
end

if tzname ~= nil then
offset, date.tzindex = parse_tzname(date.epoch, tzname)
end

if offset ~= nil then
time_localize(date, offset)
end
end

return date, len
Expand Down Expand Up @@ -1152,13 +1168,23 @@ function datetime_set(self, obj)
if tzname ~= nil then
offset, self.tzindex = parse_tzname(sec_int, tzname)
end
self.epoch = utc_secs(sec_int, offset)
self.epoch = sec_int
self.nsec = nsec
self.tzoffset = offset

return self
end

-- Only timezone is changed.
if not hms and not ymd then
if tzname ~= nil then
offset, self.tzindex = parse_tzname(self.epoch, tzname)
end

self.tzoffset = offset
return self
end

-- normalize time to UTC from current timezone
time_delocalize(self)

Expand Down
53 changes: 53 additions & 0 deletions test/app-luatest/datetime_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2098,3 +2098,56 @@ for supported_by, standard_cases in pairs(UNSUPPORTED_DATETIME_FORMATS) do
end
end
end

-- Test providing a timezone doesn't affect the timestamp
-- value but instead it affects hours/minutes represented by
-- datetime objects.
--
-- The scenario has been broken before gh-10363.
pg.test_timestamp_with_tz_in_new_preserves_timestamp = function()
local now = dt.now()
local tzoffset = now.tzoffset

local now_from_timestamp_with_tzoffset = dt.new({
timestamp = now.timestamp,
tzoffset = tzoffset,
})
local now_from_timestamp_different_tzoffset = dt.new({
timestamp = now.timestamp,
tzoffset = tzoffset + 60,
})

-- Provided timestamp values aren't affected by tzoffsets.
t.assert_equals(now.timestamp, now_from_timestamp_with_tzoffset.timestamp)
t.assert_equals(now.timestamp,
now_from_timestamp_different_tzoffset.timestamp)

-- Hours are updated correspondingly to the provided
-- tzoffsets.
t.assert_equals(now.hour, now_from_timestamp_with_tzoffset.hour)
t.assert_equals(now.hour + 1, now_from_timestamp_different_tzoffset.hour)
end

-- Test setting a timezone also changes the hour represented
-- by a datetime object and a corresponding timestamp remains
-- the same.
--
-- The scenario has been broken before gh-10363.
pg.test_tz_updates_not_change_timestamps = function()
local now = dt.now()
local tzoffset = now.tzoffset

local now_with_tzoffset_plus_60 = dt.new({timestamp = now.timestamp})
:set({tzoffset = tzoffset + 60})
local now_with_tzoffset_minus_60 = dt.new({timestamp = now.timestamp})
:set({tzoffset = tzoffset - 60})

-- Timestamp values aren't affected by tzoffset changes.
t.assert_equals(now.timestamp, now_with_tzoffset_plus_60.timestamp)
t.assert_equals(now.timestamp, now_with_tzoffset_minus_60.timestamp)

-- Hours are updated correspondingly to the tzoffset
-- changes.
t.assert_equals(now.hour + 1, now_with_tzoffset_plus_60.hour)
t.assert_equals(now.hour - 1, now_with_tzoffset_minus_60.hour)
end
23 changes: 12 additions & 11 deletions test/app-tap/datetime.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2819,27 +2819,28 @@ test:test("Time :set{} operations", function(test)
'hour 6')
test:is(tostring(ts:set{ min = 12, sec = 23 }), '2020-11-09T06:12:23+0300',
'min 12, sec 23')
test:is(tostring(ts:set{ tzoffset = -8*60 }), '2020-11-09T06:12:23-0800',
test:is(tostring(ts:set{ tzoffset = -8*60 }), '2020-11-08T19:12:23-0800',
'offset -0800' )
test:is(tostring(ts:set{ tzoffset = '+0800' }), '2020-11-09T06:12:23+0800',
test:is(tostring(ts:set{ tzoffset = '+0800' }), '2020-11-09T11:12:23+0800',
'offset +0800' )
-- timestamp 1630359071.125 is 2021-08-30T21:31:11.125Z
-- Timestamp 1630359071.125 is 2021-08-30T21:31:11.125+0000
-- or 2021-08-31T05:31:11.125+0800.
test:is(tostring(ts:set{ timestamp = 1630359071.125 }),
'2021-08-30T21:31:11.125+0800', 'timestamp 1630359071.125' )
test:is(tostring(ts:set{ msec = 123}), '2021-08-30T21:31:11.123+0800',
'2021-08-31T05:31:11.125+0800', 'timestamp 1630359071.125' )
test:is(tostring(ts:set{ msec = 123}), '2021-08-31T05:31:11.123+0800',
'msec = 123')
test:is(tostring(ts:set{ usec = 123}), '2021-08-30T21:31:11.000123+0800',
test:is(tostring(ts:set{ usec = 123}), '2021-08-31T05:31:11.000123+0800',
'usec = 123')
test:is(tostring(ts:set{ nsec = 123}), '2021-08-30T21:31:11.000000123+0800',
test:is(tostring(ts:set{ nsec = 123}), '2021-08-31T05:31:11.000000123+0800',
'nsec = 123')
test:is(tostring(ts:set{timestamp = 1630359071, msec = 123}),
'2021-08-30T21:31:11.123+0800', 'timestamp + msec')
'2021-08-31T05:31:11.123+0800', 'timestamp + msec')
test:is(tostring(ts:set{timestamp = 1630359071, usec = 123}),
'2021-08-30T21:31:11.000123+0800', 'timestamp + usec')
'2021-08-31T05:31:11.000123+0800', 'timestamp + usec')
test:is(tostring(ts:set{timestamp = 1630359071, nsec = 123}),
'2021-08-30T21:31:11.000000123+0800', 'timestamp + nsec')
'2021-08-31T05:31:11.000000123+0800', 'timestamp + nsec')
test:is(tostring(ts:set{timestamp = -0.1}),
'1969-12-31T23:59:59.900+0800', 'negative timestamp')
'1970-01-01T07:59:59.900+0800', 'negative timestamp')
end)

test:test("Check :set{} and .new{} equal for all attributes", function(test)
Expand Down

0 comments on commit 0b4b431

Please sign in to comment.