This is in reference to the TimestampDataType
class.
I'm using a postgres database with a column of type typestamp without time zone
When I try to populate a UTC timestamp using DBUnit dataset, it always stores the timestamp in the DB after converting it to my local timezone.
Eg Dataset - 2024-03-31 00:00:00 +0000
Actual Value in DB - 2024-03-31 08:00:00
Expected Value in DB - 2024-03-31 00:00:00
This is on my local machine and I'm in the Singapore timezone (UTC +8)
Upon debugging, I found that the TimestampDataType.typeCast
logic correctly accounts for the timezone and calculates the millis since UTC correctly but in the end it converts it to a java.sql.Timestamp
. Due to that java.sql.Timestamp
assumes my local machine's timezone (+8). And finally this Timestamp
is passed to the PreparedStatement like this along with the local timezone -
statement.setTimestamp(column, (java.sql.Timestamp) typeCast(value));
My assumption is that, DBUnit should respect the timezone provided in the dataset and not let local timezone override it as my tests can run on any server in any timezone. One way I can think of to fix this is to pass the correct timezone to the setTimestamp
method . We already extract this timezone from the dataset in the typeCast
method-
val cal: Calendar = Calendar.getInstance(TimeZone.getTimeZone("GMT"))
statement.setTimestamp(column, (java.sql.Timestamp) typeCast(value), cal);
I don't want to modify my jvm timezone as I want it to replicate actual server timezone.
Please advice if this is a bug or am I misunderstanding this? Thanks!
Hi Jyotman, thank you for reporting the issue and digging into it.
From your initial investigation work and description, it seems like a bug. Next step is to assess it further and if is a bug, reproduce it in tests and then fix.
Have you reviewed the TimestampDataTypeTest class to see if it covers your scenario(s)? If it doesn't, can you add your scenarios/tests to it, adjust TimestampDataType to make the tests pass, and create a MR? I can review it, merge it, and publish a point release if you can get to it!
Hi Jeff, thanks for your prompt reply.
This is difficult to test
typeCast
as the problem seems to be in the return type of the function itself. The current return typejava.sql.Timestamp
uses the machine's default timezone which leads to neglecting the timezone provided in the DBUnit dataset. To fix this would involve returning different type fromtypeCast
that can respect the timezone provided in the dataset.But still to prove my point, this test would always fail. Same UTC value but depends on local system timezone.
java.sql.Timezone
relies internally on java.util.Date and always uses the default local system timezone. Even though the it stores the correct long value for milliseconds in UTC, it's value keeps changing as the system local timezone changes. Thus, it sends this local timezone to the Database as well when we do -The solution I can think of is to let
typeCast
returnZonedDateTime
so that it preserves the value of the dataset timezone and then we construct a calendar instance from that timezone to pass to the statement. Here's the rough code -Also I believe, fixing this would be considered a breaking change as people might be dependent on the existing behaviour?
Last edit: Jyotman Singh 2024-08-04
To make it more clear, I've added another test that would fail on any system not running on UTC -
I've also attached a debug screenshot of my system running on UTC+8 (Singapore) timezone.
I see what you mean. Great analysis and writeup, thank you!
Hmmm that's unfortunate!
Possibly. It requires the differing zones, as in your case, so possibly less common(?). Also possibly avoided with a timestamp column defined with zone, but we won't know... The worst case is this change causes a test(s) to fail, not production code. Let's see how it turns out, a potential breaking change for some but it fixes a problem.
Should work... At quick glance in your message, wondering if in
typeCast()
can do the additionalsetSqlValue()
steps, andtypeCast()
still returnsTimestamp
? Or is there something that requires the split logic?Are you ok to proceed with creating an MR with the additional tests for the various permutations/scenarios and the corrections for it?
If so, this page [0] has some relevant notes that may have use for you. Complete your commits as much as you want/can and I can tweak as needed before merge.
[0] https://dbunit.sourceforge.net/dbunit/devguide.html
Hi Jeff,
Yes, I agree! I'm also not happy with the additional steps required in
setSqlValue()
but I can't returnTimestamp
fromtypeCast
because it doesn't retain the expected dataset zone. I need to return the correct zone along with the Timestamp so that thestatement.setTimestamp
can receive the correct zone along with the Timestamp in the form of theCalendar
instance.So I deemed
ZonedDateTime
as the correct choice which retains the datatime as well as the desired zone.I thought about returning a Calendar instance directly from
typeCast
instead ofZonedDateTime
to minimise writing code insetSqlValue
but it doesn't allow to store nanoseconds precision.Open to any other suggestions.
Last edit: Jyotman Singh 2024-08-05
None to suggest at the moment. Good approach is to "fix it with tests" first then we can refactor to improve, if possible and needed.
Thank you for reporting and solving this problem!
Hi Jyotman, just checking in to see how it's going on this one? Any questions or issues I can help with?
Hi Jeff, apologies for the late reply. This will take significant effort as the tests will require a complete rewrite due to the return type of the method changing and will also need to find a way to test the
setSqlValue()
method since it'll contain some logic too because of the above suggested approach.I'll try to find some time to do this but can't promise a timeline right now. Will keep you updated though if/when I start working on this. Thanks for the help so far!
Also fine if someone else wants to pick this up since I won't be able to pick it up immediately.