Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

All timestamps should be stored in DB as UTC #528

Closed
lmsurpre opened this issue Dec 20, 2019 · 0 comments
Closed

All timestamps should be stored in DB as UTC #528

lmsurpre opened this issue Dec 20, 2019 · 0 comments
Assignees
@lmsurpre
Labels
bug Something isn't working
Milestone
Sprint 5

Comments

@lmsurpre
Copy link
Member

Describe the bug
We thought this was this case, but I found it is not.

To Reproduce
Steps to reproduce the behavior:

  1. Run JDBCSearchDateTest.java
  2. Look at the BASIC_RESOURCES table's LAST_UPDATED column

Expected behavior
The database value should be stored as UTC.
@lmsurpre lmsurpre added the bug Something isn't working label Dec 20, 2019
@lmsurpre lmsurpre added this to the Sprint 5 milestone Dec 20, 2019
lmsurpre added a commit that referenced this issue Dec 20, 2019
@lmsurpre
issue #528 - set UTC timezone for all storedprocedure timestamps
7f226da 
also added `shutdownDatabase` placeholder in AbstractPersistenceTest

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Dec 20, 2019
@lmsurpre
issue #528 - revert some db changes (store datetimes as ranges again)
d3539f9 
We found that some of the logic was wrong in the DateParmBehaviorUtil.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
issue #528
c05880a 
- Refactor DateParmBehaviorUtil and DateParmBehaviorUtilTest to reflect
the RANGE intersection
- Tone down the logging

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
issue #528
1e88239 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
issue #528
08d0ada 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
lmsurpre added a commit that referenced this issue Dec 21, 2019
@lmsurpre
issue #528 - update Conformance.md and tweak datetime precision
b27fe08 
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>
lmsurpre added a commit that referenced this issue Dec 21, 2019
@lmsurpre
issue #528 - use server timezone to interpret dates
bee79cb 
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>
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
Merge pull request #530 from IBM/lee-master-patch2
65eff00 
issue #528 - use server timezone to interpret dates
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
Merge pull request #529 from IBM/lee-master-patch1
05fd624 
issue #528 - update Conformance.md and tweak datetime precision
@prb112 prb112 closed this as completed Dec 21, 2019
lmsurpre added a commit that referenced this issue Dec 21, 2019
@lmsurpre
issue #528 - use UTC in DateTimeHandlerTest and ParameterExtractionTest
69d39c6 
Now the DateTimeHandler are dependent on the current timezone of the
java runtime.  On our CI environment, it must be UTC.  Now we'll set UTC
as the default timezone at the beginning of these tests so they can pass
like they used to.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
prb112 added a commit that referenced this issue Dec 21, 2019
@prb112
Merge pull request #531 from IBM/lee-master
03ecee6 
issue #528 - use UTC in DateTimeHandlerTest and ParameterExtractionTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmsurpre lmsurpre

Labels
bug Something isn't working
Projects
None yet
Milestone
Sprint 5
Development

No branches or pull requests
2 participants
@prb112 @lmsurpre