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.

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

Timezone handling #986

Open
lpandzic opened this issue Jun 14, 2021 · 12 comments
Open

Timezone handling #986

lpandzic opened this issue Jun 14, 2021 · 12 comments
Assignees
Labels
theme: date-time Issues related to handling of date, time and timezone information type: enhancement A general enhancement

Comments

@lpandzic
Copy link

lpandzic commented Jun 14, 2021

For an Instant defined as:

LocalDate.of(2021, 1, 1).atStartOfDay(ZoneOffset.UTC).toInstant()

when using Spring Data JDBC repository save method I'd expect the end result in MSSQL database column of type datetime2 to be of value: 2021-01-01 00:00:00.0000000 but it contains value 2021-01-01 01:00:00.0000000 suggesting that Spring Data JDBC uses my machine timezone when converting Instant.

Is this behavior documented anywhere and can it be configured somehow? I'd like my application to always use UTC unless a type with explicit timezone and/or offset is used.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 14, 2021
@schauder schauder self-assigned this Jun 14, 2021
@schauder
Copy link
Contributor

Is it possible, that you get mislead by the toString of the timestamp?

If I save an entity with the instant as described, and load the timestamp using a JdbcTemplate and ResultSet.getTimestamp and print the result with the following code

                WithInstant entity = new WithInstant();
		entity.testTime = LocalDate.of(2021, 1, 1).atStartOfDay(ZoneOffset.UTC).toInstant();
		entity.id = 23L;
		template.insert(entity);

		WithInstant loaded = template.findById(23L, WithInstant.class);

		assertThat(loaded.testTime).isEqualTo(entity.testTime);

		Timestamp timestamp = jdbcTemplate.queryForObject("select test_time from with_instant where id = 23", emptyMap(), (rs, i) -> rs.getTimestamp(1));
		System.out.println(timestamp);
		System.out.println(timestamp.getTime());
		System.out.println(entity.testTime);
		System.out.println(entity.testTime.getEpochSecond());

I get the following output:

2021-01-01 01:00:00.0
1609459200000
2021-01-01T00:00:00Z
1609459200

I'm in CET, which is one hour ahead of UTC right now.

@lpandzic
Copy link
Author

I've tested against 2.2.1 and 2.3.0-SNAPSHOT.

I'm looking in database viewer (IDEA):
Screenshot 2021-06-14 at 13 27 49.

I'm in CEST (+1).

In your case - what's the value stored in database?

@lpandzic
Copy link
Author

lpandzic commented Jun 14, 2021

I've tested against JPA and it seems it also results in spring data jdbc result - 2021-01-01 01:00:00.0000000.
Can you please confirm and provide your results?

@lpandzic
Copy link
Author

lpandzic commented Jun 14, 2021

Is it possible, that you get mislead by the toString of the timestamp?

The original source of my issue is located here in this test - https://github.com/infobip/infobip-spring-data-querydsl/blob/issue-31-fix/infobip-spring-data-jdbc-querydsl/src/test/java/com/infobip/spring/data/jdbc/QuerydslJdbcRepositoryTest.java#L198-L212
Both Querydsl and Spring Data say that they store the correct value in database and that the other one is wrong.

When I query the database with following tsql:

SELECT DATEDIFF(SECOND, '19700101', CreatedAt) FROM Person

I get 1609462800 as result for Spring Data which https://www.epochconverter.com/ tells me is GMT: Friday, January 1, 2021 1:00:00 or Your time zone: Friday, January 1, 2021 2:00:00 which doesn't seem right.

@schauder
Copy link
Contributor

I think we got bitten by the JDBC API which seems to have a rather creative interpretation of dates.

This Stackoverflow answer tipped me of. It talks about variants of setTimestamp.
We don't use that but setObject but I guess the JDBC driver just delegates to setTimestamp.

So here is what seems to happen.

I assume setObject(int, Object) delegates to setTimestamp(int, Timestamp)

void setTimestamp(int parameterIndex, Timestamp x)
throws SQLException
Sets the designated parameter to the given java.sql.Timestamp value. The driver converts this to an SQL TIMESTAMP value when it sends it to the database.

This intern probably delegates to

void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) throws SQLException
Sets the designated parameter to the given java.sql.Timestamp value, using the given Calendar object. The driver uses the Calendar object to construct an SQL TIMESTAMP value, which the driver then sends to the database. With a Calendar object, the driver can calculate the timestamp taking into account a custom timezone. If no Calendar object is specified, the driver uses the default timezone, which is that of the virtual machine running the application.

Now the interesting part is of course the statement:

The driver uses the Calendar object to construct an SQL TIMESTAMP value

java.sql.Timestamp are java.util.Date instances and as such are supposed to be in UT = GMT ~= UTC and this is the way Spring Data uses them.

But JDBC (or at least the driver of MS SQL Server) seems to do the following:

  • take the millis since epoch from the timestamp.
  • adds the offset of the timezone from the calendar converted into millis to it.
  • and sends that to the database.

And it odes the reverse when loading the timestamp from the database again.

At least that is my current interpretation of what I'm seeing and reading.
I'm not asking if this makes sens

@lpandzic
Copy link
Author

I've debugged the test and Spring Data JDBC invokes setTimestamp method on PreparedStatement with java.sql.Timestamp and my understanding is that it's game over by that point.
Querydsl SQL module on the other hand doesn't seem to be using PreparedStatement but generates the Insert Statement with com.querydsl.sql.SQLSerializer.
Not sure I agree with either approach but I can understand/appreciate how it got there.

@schauder
Copy link
Contributor

While I think the JDBC API or SQL Servers driver implementation is using Timestamp wrong there isn't much of a point in bitching about this.

But we can't really switch to an approach where we provide a Calendar or change the way we convert to Timestamp without breaking peoples existing data. So we'll probably introduce some kind of configuration property that allows to specify a timezone to be used with arguments that are passed as Timestamp to statements.

@schauder schauder added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 15, 2021
@lpandzic
Copy link
Author

I agree and to clarify - I wanted to identify root of the issue before proceeding.

I'm trying to solve this issue (infobip/infobip-spring-data-querydsl#31) because I've run out of easy options to prevent users from stumbling on this issue - that is time handling difference between Spring Data JDBC and Querydsl.
Previously I've opted in to Spring Data JDBC of doing things but current infrastructure is entity focused and is not, at least on API surface, friendly to projections.
I'm aware of #980 and wanted to ask what's the recommended approach on how to bridge this problem. Critical code path is located here - https://github.com/infobip/infobip-spring-data-querydsl/blob/master/infobip-spring-data-jdbc-querydsl/src/main/java/com/infobip/spring/data/jdbc/QuerydslJdbcPredicateExecutor.java#L160-L170.

I believe the following "hack" would work but I have yet to test it:

<O> List<O> queryMany(SQLQuery<O> query) {
    @SuppressWarnings("unchecked")
    RowMapper<O> rowMapper = (RowMapper<O>) new EntityRowMapper<>(entity, converter);
    RowMapperResultSetExtractor<O> rowMapperResultSetExtractor = new RowMapperResultSetExtractor<>(rowMapper);
    List<O> result = query(query, rowMapperResultSetExtractor);

    if (Objects.isNull(result)) {
        return Collections.emptyList();
    }

    return result;
}

@lpandzic
Copy link
Author

After further investigation the "hack" I mentioned above doesn't work and mapping SQLQuery projection expression to a custom row mapper or something similar doesn't seem like something that would scale or make sense really.
The only workaround I could find is to set JVM level timezone TimeZone.setDefault(TimeZone.getTimeZone(ZoneOffset.UTC))which is bad, really bad.

It'd be great if Spring Data JDBC provided some sort of override mechanism.

@schauder
Copy link
Contributor

We'll probably implement some way to configure this. In the meantime you should be able to register custom converters for the conversion from and to Timestamp.

@schauder
Copy link
Contributor

@Demo-80 please do not hijack only vaguely related tickets. Also, your question seems to be a usage question. Please ask those over on Stackoverflow.

If you think you found a bug or want to share an idea for a feature or improvement feel free to create a ticket.

@Demo-80
Copy link

Demo-80 commented Nov 16, 2021

@Demo-80请不要只劫持相关的票证。另外,您的问题似乎是一个用法问题。请在 Stackoverflow 上询问那些人。

如果您认为自己发现了错误或想要分享功能或改进的想法,请随时创建票证。
I'm sorry

@schauder schauder added the theme: date-time Issues related to handling of date, time and timezone information label Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: date-time Issues related to handling of date, time and timezone information type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants