search

ClickHouse / clickhouse-cpp

C++ client library for ClickHouse
Apache License 2.0
306 stars 159 forks source link

time column appending bug #318

Closed vovchick12385 closed 1 year ago

vovchick12385 commented 1 year ago

image

I found this comment in the library code. Can you please tell me what is the bug and when is it going to be fixed?

Enmk commented 1 year ago

Hi @vovchick12385 !

I assume you are talking about ColumnDate32 or ColumnDate. The problem with Append for those is that it tries to convert local timestamp (number of seconds) to day number in unix epoch, using very naive algorithm that completely ignores timezones, leap seconds, etc. https://github.com/ClickHouse/clickhouse-cpp/blob/v2.4.0/clickhouse/columns/date.cpp#L13

IMO our best bet is to deprecate those time_t-based Append methods and add ColumnDate::Append(UInt16 ) and ColumnDate32::Append(UInt32).

We could also update the existing implementation to convert time_t properly to/from day number (UInt16 or UInt32) from and to time_t, but the result would depend on the current locale's timezone and it would be much slower.

Enmk commented 1 year ago

@vovchick12385 Please take a look at PR#320, would that solve your issue? The basic idea is to provide an interface for writing/reading immediate values without conversion.