Menu

#449 UTC Timestamp in dataset converted to local timezone in DB

v2.7.*
pending
None
(not fixed)
3
2024-08-14
2024-08-03
No

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!

Discussion

  • Jeff Jensen

    Jeff Jensen - 2024-08-04
    • status: open --> pending
    • assigned_to: Jeff Jensen
    • Priority: 5 --> 3
     
  • Jeff Jensen

    Jeff Jensen - 2024-08-04

    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!

     
  • Jyotman Singh

    Jyotman Singh - 2024-08-04

    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 type java.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 from typeCast 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.

    public void test() throws Exception {
        long millis = 1359246161900L;
        TimeZone.setDefault(TimeZone.getTimeZone("GMT+1"));
        Timestamp ts1 = new Timestamp(millis);
        String ts1Str = ts1.toString();
        TimeZone.setDefault(TimeZone.getTimeZone("GMT+2"));
        Timestamp ts2 = new Timestamp(millis);
        String ts2Str = ts2.toString();
        assertEquals(ts1Str, ts2Str);
    }
    

    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 -

    statement.setTimestamp(column, timestamp);
    

    The solution I can think of is to let typeCast return ZonedDateTime 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 -

    public Object typeCast(Object value) {
    ....
    // ts = new Timestamp(components[0].longValue());
    // ts.setNanos(components[1].intValue());
    // return ts;
        ZoneOffset zoneOffset = ZoneOffset.of(zoneValue);
        Instant instant = Instant.ofEpochMilli(components[0].longValue());
        ZonedDateTime zonedDateTime = ZonedDateTime.ofInstant(instant, zoneOffset);
        return zonedDateTime;
    }
    
    public void setSqlValue(Object value, int column, PreparedStatement statement) {
        ...
        ZonedDateTime zonedDateTime = (ZonedDateTime) typeCast(value);
        Timestamp timestamp = Timestamp.from(zonedDateTime.toInstant());
        GregorianCalendar cal = new GregorianCalendar(TimeZone.getTimeZone(zonedDateTime.getZone()));
        cal.setTimeInMillis(timestamp.getTime());
        statement.setTimestamp(column, timestamp, cal);
    }
    

    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
  • Jyotman Singh

    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 -

    public void testWithTimezone_RetainTimezone() throws Exception
        {
            String ts = "2013-01-27 01:22:41.900 +0000";
            String expected = "2013-01-27 01:22:41.9";
            String received = THIS_TYPE.typeCast(ts).toString();
            assertEquals(expected, received);
        }
    

    I've also attached a debug screenshot of my system running on UTC+8 (Singapore) timezone.

     
    • Jeff Jensen

      Jeff Jensen - 2024-08-05

      I see what you mean. Great analysis and writeup, thank you!

      The current return type java.sql.Timestamp uses the machine's default timezone which leads to neglecting the timezone provided in the DBUnit dataset.

      Hmmm that's unfortunate!

      Also I believe, fixing this would be considered a breaking change as people might be dependent on the existing behaviour?

      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.

      Here's the rough code

      Should work... At quick glance in your message, wondering if in typeCast() can do the additional setSqlValue() steps, and typeCast() still returns Timestamp? 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

       
  • Jyotman Singh

    Jyotman Singh - 2024-08-05

    Hi Jeff,

    Yes, I agree! I'm also not happy with the additional steps required in setSqlValue() but I can't return Timestamp from typeCast because it doesn't retain the expected dataset zone. I need to return the correct zone along with the Timestamp so that the statement.setTimestamp can receive the correct zone along with the Timestamp in the form of the Calendar 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 of ZonedDateTime to minimise writing code in setSqlValue but it doesn't allow to store nanoseconds precision.
    Open to any other suggestions.

     

    Last edit: Jyotman Singh 2024-08-05
    • Jeff Jensen

      Jeff Jensen - 2024-08-05

      Open to any other suggestions.

      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!

       
      👍
      1
      • Jeff Jensen

        Jeff Jensen - 2024-08-14

        Hi Jyotman, just checking in to see how it's going on this one? Any questions or issues I can help with?

         
        • Jyotman Singh

          Jyotman Singh - 2024-08-14

          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.

           

Log in to post a comment.