ClickHouse.Client icon indicating copy to clipboard operation
ClickHouse.Client copied to clipboard

Wrong (may be) time zone conversion

Open dimasrespect opened this issue 3 years ago • 5 comments

Hi, I think that there are wrong behaviour when writing DateTime values in to CH database with column of DateTime type (without timezone specified). When I doing so i don't care about time zones, because they are (time zones) not set in code (using simple DateTime instead of DateTimeOffset type) and not set in table schema (using DateTime, not DateTime('time/zone')). So when i'm writing some value to database I want to see the same value in SELECT results. But, at this time when it write to CH it converts value to UTC time zone. My time zone is UTC+3, so when i selecting data, it returns value with 3 hours difference.

I think that in this case it should write DateTime value as is, without conversion.

May be this method should looks like this:

public ZonedDateTime ToZonedDateTime(DateTime dateTime)
{
    if (TimeZone == null)
        return ZonedDateTime.FromDateTimeOffset(dateTime);

    return TimeZone.ResolveLocal(LocalDateTime.FromDateTime(dateTime), Resolvers.LenientResolver);
}

What do you think?

By the way, variable timeZone is set, but not used in this method now. May be it should be used in next line of code instead of TimeZone property.

dimasrespect avatar Feb 10 '22 17:02 dimasrespect

Hi. So your expectation is that values written into a DateTime column (without timezone specified) should be read in same way, and, I assume, with DateTime.Kind == DateTime.Unspecified. That is a fair assumption which I think intertwines with #119 a lot.

Does clickhouse-client display the behavior you want? I.e. if you create a column without timezone specification, insert a value and then recover a value - is that behavior matches your expected one

DarkWanderer avatar Feb 10 '22 18:02 DarkWanderer

Does clickhouse-client display the behavior you want? I.e. if you create a column without timezone specification, insert a value and then recover a value - is that behavior matches your expected one

That's right, both clickhouse-client and web ui playground of CH works as expected. When i inserting some DateTime value, the SELECT returns same value, without timezone conversion.

#119 is a little bit different issue. In my case client and server located somewhere in the same timezone (in fact, they are running on same hardware), but in case of #119 the server and client are placed in different time zones, as i understand. To meet all requirements there are must be ability to set time zone at ClickHouseConnection scope, may be a property like this:

public class ClickHouseConnection : DbConnection, IClickHouseConnection, ICloneable
{
...
    public DateTimeZone TimeZone { get; set; }
...
}

which has null value by default, that means no TZ conversion required. And, if i need to use server side time zone, i can set this property to hard coded value or use a query SELECT timezone() to obtain it. There are also could be something like public DateTimeZone GetServerTimeZone() method, that runs this query internally and parses result. So, when we need to use server timezone we can simply write:

// somewhere in our code
var connection = new ClickHouseConnection(...);
connection.TimeZone = connection.GetServerTimeZone();
// now we are using server side TZ

The only problem is that value of this propertry should be passed to AbstractDateTimeType.ToZonedDateTime() as additinal parameter from connection.

public ZonedDateTime ToZonedDateTime(DateTime dateTime, DateTimeZone usingTimeZone)
{
    var timeZone = TimeZone ?? usingTimeZone;
    if (timeZone == null)
        return ZonedDateTime.FromDateTimeOffset(dateTime);

    return timeZone.ResolveLocal(LocalDateTime.FromDateTime(dateTime), Resolvers.LenientResolver);
}

usingTimeZone parameter is passed from connection class. That's could be a little bit complicated as there are a lot of routines between connection class and this method, but I think that it may solve both issues. Or, may be, you know more elegant solution

dimasrespect avatar Feb 11 '22 06:02 dimasrespect

There one more method to rework i found AbstractDateTimeType.ToDateTime(NodaTime.Instant). We need to pass TZ from connection to it too. There are may be others, but i don't know your code very well, sorry, that's all from a first look.

dimasrespect avatar Feb 11 '22 06:02 dimasrespect

I've found #95. It looks like you've already thought about it ;)

dimasrespect avatar Feb 11 '22 07:02 dimasrespect

5.1.0 now has UseServerTimezone flag - it will make the behavior similar to clickhouse-client default. Please check & let me know if it works correctly for you - if there is positive feedback, I will make that the default from next major version

DarkWanderer avatar Sep 15 '22 19:09 DarkWanderer

Will consider this completed - please create a ticket if you still experience an issue

DarkWanderer avatar Dec 21 '22 13:12 DarkWanderer