Skip to content

Set timezone to UTC for all storedprocedure timestamps#527

Merged
prb112 merged 11 commits intomasterfrom
lee-master
Dec 21, 2019
Merged

Set timezone to UTC for all storedprocedure timestamps#527
prb112 merged 11 commits intomasterfrom
lee-master

Conversation

@lmsurpre
Copy link
Copy Markdown
Member

also added shutdownDatabase placeholder in AbstractPersistenceTest

Signed-off-by: Lee Surprenant lmsurpre@us.ibm.com

@lmsurpre lmsurpre force-pushed the lee-master branch 2 times, most recently from 58a803c to c83572e Compare December 20, 2019 19:47
also added `shutdownDatabase` placeholder in AbstractPersistenceTest

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre and others added 2 commits December 20, 2019 16:29
We found that some of the logic was wrong in the DateParmBehaviorUtil.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
- Refactor DateParmBehaviorUtil and DateParmBehaviorUtilTest to reflect
the RANGE intersection
- Tone down the logging

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 changed the title WIP Set timezone to UTC for all storedprocedure timestamps Set timezone to UTC for all storedprocedure timestamps Dec 21, 2019
Copy link
Copy Markdown
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

prb112 and others added 7 commits December 20, 2019 20:45
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
1. added a bunch more date and datetime tests to AbstractSearchDateTest
(and reorganized some ones that were there)

2. found and fixed one problematic case:

We were capturing the precision wrong for Datetime values which include
0 seconds.  Our algorithm relied on TemporalAccessor.toString but this
method actually excludes the seconds when they are 0. This was causing
us to store values like `2019-12-31T20:00:00Z` as the range
`[2019-12-31T20:00:00Z, 2019-12-31T20:00:59.999999Z)` instead of
`[2019-12-31T20:00:00Z, 2019-12-31T20:00:00.999999Z)`. I addressed it by
using the DateTime or Date PARSER_FORMATTER (depending on its type).

3. updated BasicDate.json to test with a more "interesting"
dateTime...one that strattles the edge of a year, month, and day.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
The spec says "Where both search parameters and resource element date
times do not have time zones, the servers local time zone should be
assumed" and so I wanted to try seeing what that would look like.

Its not very clear what is "correct" if only one of those two excludes
time zones. It doesn't matter much on the parameter extraction side
because all datetimes with times in them are required to have a
timezone, but I think I like the idea of handling searches w/o timezone
info via local time, because usually if someone puts a date or datetime
without a timezone they are expecting local time and not UTC time.

In the process, I also found a discrepency between our EQ and NE
behavior. To rectify this, I removed the "exactly equals DATE_START"
omptimization because I think its not always right...for example if the
indexed value was actually a Period then we shold consider an exact
point to be equal to that Period (even if the start matches).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>

Co-Authored-By: Paul Bastide <pbastide@us.ibm.com>
issue #528 - use server timezone to interpret dates
issue #528 - update Conformance.md and tweak datetime precision
@prb112 prb112 added this to the Sprint 5 milestone Dec 21, 2019
@prb112 prb112 added bug Something isn't working search labels Dec 21, 2019
@prb112 prb112 merged commit 565b595 into master Dec 21, 2019
@prb112 prb112 deleted the lee-master branch December 21, 2019 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working search

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants