Timezone naive datetimes when query_tz == pytz.UTC

[prokofyevDmitry]

Describe the bug

When a query_df is called with query_tz= "UTC" or query_tz=pytz.UTC the result Timestamp columns are timezone naive weither they have DateTime64 or DateTime columns as origin.

After analysing the code, it turns out that this seems intentional as inside active_tz the returned timezone is explicitly None when active_tz = pytz.UTC. I'm not sure why, and if there is a good reason it should be added in the documentation.

https://github.com/ClickHouse/clickhouse-connect/blob/main/clickhouse_connect/driver/query.py#L171

    def active_tz(self, datatype_tz: Optional[tzinfo]):
        if self.column_tz:
            active_tz = self.column_tz
        elif datatype_tz:
            active_tz = datatype_tz
        elif self.query_tz:
            active_tz = self.query_tz
        elif self.response_tz:
            active_tz = self.response_tz
        elif self.apply_server_tz:
            active_tz = self.server_tz
        else:
            active_tz = tzutil.local_tz
        if active_tz == pytz.UTC:
            return None
        return active_tz

But even if this method is fixed and does return UTC when query_tz set to UTC (and this solves the issue for DateTime) the DateTime64._read_column_binary method explicitly ignores setting the timezone if active_tz = pytz.UTC
https://github.com/ClickHouse/clickhouse-connect/blob/main/clickhouse_connect/datatypes/temporal.py#L246

    def _read_column_binary(self, source: ByteSource, num_rows: int, ctx: QueryContext, _read_state: Any) -> Sequence:
        if self.read_format(ctx) == 'int':
            return source.read_array('q', num_rows)
        active_tz = ctx.active_tz(self.tzinfo)
        if ctx.use_numpy:
            np_array = numpy_conv.read_numpy_array(source, self.np_type, num_rows)
            if ctx.as_pandas and active_tz and active_tz != pytz.UTC:
                return pd.DatetimeIndex(np_array, tz='UTC').tz_convert(active_tz)
            return np_array
        column = source.read_array('q', num_rows)
        if active_tz and active_tz != pytz.UTC:
            return self._read_binary_tz(column, active_tz)
        return self._read_binary_naive(column)

Steps to reproduce

1. See test in Code example

Expected behaviour

When query_tz is set to UTC, the response's date time columns should be localised to UTC.

Code example

This test can executed by being appended to tests/test_pandas.py file.

@pytest.mark.parametrize('tz_to_test', [
        None,  # No timezone - should return naive datetime => succeeds
        'Europe/Madrid', # Timezone - should return timezone aware datetime => succeeds
        'UTC', # Timezone - should return timezone aware datetime => fails
    ])
def test_pandas_timezone(test_client: Client, table_context: Callable, tz_to_test):
    if not test_client.min_version('20'):
        pytest.skip(f'DateTime64 not supported in this server version {test_client.server_version}')

    nano_timestamp = pd.Timestamp(1992, 11, 6, 12, 50, 40, 7420, 44)
    with table_context('test_pandas_timezone', ['key String',
                                            "millis DateTime",
                                            "nanos DateTime64(9)"]):
        df = pd.DataFrame([['key1', nano_timestamp, nano_timestamp]], columns=['key', 'millis', 'nanos'])
        test_client.insert_df('test_pandas_timezone', df)
        result_df = test_client.query_df('SELECT * FROM test_pandas_timezone', query_tz=tz_to_test)
        if tz_to_test is None:
            assert result_df.iloc[0]['nanos'].tzinfo is None
            assert result_df.iloc[0]['millis'].tzinfo is None
        else:
            assert result_df.iloc[0]['millis'].tzinfo.zone == tz_to_test
            assert result_df.iloc[0]['nanos'].tzinfo.zone == tz_to_test

This test fails for the UTC case with error log:

tests/integration_tests/test_pandas.py:202 (test_pandas_timezone[UTC])
test_client = <clickhouse_connect.driver.httpclient.HttpClient object at 0x7ebedc8f3e00>
table_context = <function table_context_fixture.<locals>.context at 0x7ebedc806f20>
tz_to_test = 'UTC'

    @pytest.mark.parametrize('tz_to_test', [
        None,  # No timezone - should return naive datetime => succeeds
        'Europe/Madrid', # Timezone - should return timezone aware datetime => succeeds
        'UTC', # Timezone - should return timezone aware datetime => fails
    ])
    def test_pandas_timezone(test_client: Client, table_context: Callable, tz_to_test):
        if not test_client.min_version('20'):
            pytest.skip(f'DateTime64 not supported in this server version {test_client.server_version}')
    
        nano_timestamp = pd.Timestamp(1992, 11, 6, 12, 50, 40, 7420, 44)
        with table_context('test_pandas_timezone', ['key String',
                                                "millis DateTime",
                                                "nanos DateTime64(9)"]):
            df = pd.DataFrame([['key1', nano_timestamp, nano_timestamp]], columns=['key', 'millis', 'nanos'])
            test_client.insert_df('test_pandas_timezone', df)
            result_df = test_client.query_df('SELECT * FROM test_pandas_timezone', query_tz=tz_to_test)
            if tz_to_test is None:
                assert result_df.iloc[0]['nanos'].tzinfo is None
                assert result_df.iloc[0]['millis'].tzinfo is None
            else:
                assert result_df.iloc[0]['millis'].tzinfo.zone == tz_to_test
>               assert result_df.iloc[0]['nanos'].tzinfo.zone == tz_to_test
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E               AttributeError: 'NoneType' object has no attribute 'zone'

tests/integration_tests/test_pandas.py:224: AttributeError

Proposed solution

Modify active_tz function to return the query_tz if explicitly set by the caller as it should logically prime over other tzs.
Remove the and active_tz != pytz.UTC condition from the DateTime64._read_column_binary method.
After doing the tests on my side this solution solves this problem and does not break any other tests, if this is a genuine issue and not just an undocumented edge case for some resons that escapes me I can do a PR.

• clickhouse-connect version: 0.9.2
• Python version: 3.13
• Operating system: Ubuntu 22.04.5 LTS

[genzgd]

This behavior is in fact explicitly documented

Note that if the applied timezone based on these rules is UTC, clickhouse-connect will always return a time
zone naive Pythondatetime.datetime object. Additional timezone information can then be added to this timezone 
naive object by the application code if desired.

The original reason was performance based. ClickHouse itself always stores data times in UTC and timezone information in a ClickHouse column is just metadata. With the changes in #545, I suspect the performance optimization is no longer relevant, so it probably makes sense to remove this special behavior.

[prokofyevDmitry]

Oh, my bad for missing those lines in the doc !

I understand the optimisation.
But from the api consumer perspective it feels like when requested explicitly by the caller via column_tzs or query_tz the localisation should be done regarding of performance hit ( and having the potentiality of the performance hit documented in column_tzs or query_tz ).

I'll come back with actual benchmark diff between using this optimization and without it.
Do you guys already have benchmark facilities for this lib (could not find any but again might be looking in the wrong places) (if not no big deal I'll setup my own).

[genzgd]

I think at this point we always end up applying a timezone, and unless there's some optimization in the core Python code for UTC, you're unlikely to see any difference, but more benchmarks are always welcome. There is some very old benchmarking code in /examples/benchmark.py, but I don't know how useful it is.

[prokofyevDmitry]

As promised I ran the benchmarks for the orginal code and the proposed solutions (using pytest-benchmark ) .

@pytest.mark.parametrize("col_name", ['datetime', 'dt64','dt64d'])
@pytest.mark.parametrize("query_tz", [None, pytz.UTC], ids=['NoTZ','UTC'])
def test_benchmark_dt64(benchmark,client, col_name, query_tz):
    rows = 100000
    client = clickhouse_connect.get_client(compress=False)
    set_default_formats('IP*', 'native', '*Int64', 'native')
    create_table(client, [col_name], rows)
    result = benchmark(
            client.query,
            f'SELECT * FROM benchmark_test LIMIT {rows}', query_tz=query_tz, column_oriented=True)
    assert result.row_count == rows

The code is available in a branch in my fork here but is the bottom line : systematic application of the UTC timezone does not badly affect the performances. If anything they enhance them for reasons that escapes me for now.
ignore_utc is the run before modification
apply_utc is the run after modification
Here are the benchmark results :

----------------------------------------------------------------------------------------- benchmark 'NoTZ-datetime': 2 tests -----------------------------------------------------------------------------------------
Name (time in ms)                                        Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[NoTZ-datetime] (apply_utc)      129.3301 (1.0)      137.5435 (1.0)      133.5520 (1.0)      2.5329 (1.0)      133.5023 (1.0)      3.1209 (1.0)           2;0  7.4877 (1.0)           8           1
test_benchmark_dt64[NoTZ-datetime] (ignore_utc)     196.3196 (1.52)     207.3876 (1.51)     199.7437 (1.50)     4.0380 (1.59)     198.9171 (1.49)     3.4340 (1.10)          1;1  5.0064 (0.67)          6           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------ benchmark 'NoTZ-dt64': 2 tests ------------------------------------------------------------------------------------------
Name (time in ms)                                    Min                 Max                Mean             StdDev              Median                IQR            Outliers     OPS            Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[NoTZ-dt64] (apply_utc)      166.6959 (1.0)      204.2885 (1.0)      190.5366 (1.0)      13.0345 (1.0)      191.0991 (1.0)      10.0237 (1.0)           2;1  5.2483 (1.0)           6           1
test_benchmark_dt64[NoTZ-dt64] (ignore_utc)     256.5034 (1.54)     329.3604 (1.61)     277.3083 (1.46)     30.0739 (2.31)     264.0694 (1.38)     30.6779 (3.06)          1;0  3.6061 (0.69)          5           1
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------ benchmark 'NoTZ-dt64d': 2 tests ------------------------------------------------------------------------------------------
Name (time in ms)                                     Min                 Max                Mean             StdDev              Median                IQR            Outliers     OPS            Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[NoTZ-dt64d] (apply_utc)      172.5403 (1.0)      183.4568 (1.0)      175.5737 (1.0)       4.1306 (1.0)      174.0477 (1.0)       3.5463 (1.0)           1;1  5.6956 (1.0)           6           1
test_benchmark_dt64[NoTZ-dt64d] (ignore_utc)     179.4482 (1.04)     242.9979 (1.32)     210.7030 (1.20)     22.7242 (5.50)     207.4540 (1.19)     32.3103 (9.11)          2;0  4.7460 (0.83)          6           1
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

----------------------------------------------------------------------------------------- benchmark 'UTC-datetime': 2 tests -----------------------------------------------------------------------------------------
Name (time in ms)                                       Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[UTC-datetime] (apply_utc)      138.6144 (1.0)      166.3436 (1.0)      145.0240 (1.0)      9.0811 (3.69)     142.6929 (1.0)      6.1547 (2.16)          1;1  6.8954 (1.0)           8           1
test_benchmark_dt64[UTC-datetime] (ignore_utc)     176.7200 (1.27)     183.6046 (1.10)     181.0207 (1.25)     2.4598 (1.0)      181.4483 (1.27)     2.8513 (1.0)           2;0  5.5242 (0.80)          6           1
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

----------------------------------------------------------------------------------------- benchmark 'UTC-dt64': 2 tests -----------------------------------------------------------------------------------------
Name (time in ms)                                   Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[UTC-dt64] (apply_utc)      191.8082 (1.0)      211.7470 (1.0)      197.1233 (1.0)      7.4449 (1.59)     194.8447 (1.0)      4.9782 (1.0)           1;1  5.0730 (1.0)           6           1
test_benchmark_dt64[UTC-dt64] (ignore_utc)     249.6473 (1.30)     261.8670 (1.24)     255.7573 (1.30)     4.6796 (1.0)      254.4804 (1.31)     6.4902 (1.30)          2;0  3.9100 (0.77)          5           1
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------------------ benchmark 'UTC-dt64d': 2 tests ------------------------------------------------------------------------------------------
Name (time in ms)                                    Min                 Max                Mean             StdDev              Median                IQR            Outliers     OPS            Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_dt64[UTC-dt64d] (ignore_utc)     157.1102 (1.0)      181.5630 (1.01)     172.8874 (1.0)      10.8261 (5.75)     178.6482 (1.02)     19.1576 (9.78)          2;0  5.7841 (1.0)           6           1
test_benchmark_dt64[UTC-dt64d] (apply_utc)      174.3444 (1.11)     179.5495 (1.0)      176.1755 (1.02)      1.8836 (1.0)      175.4669 (1.0)       1.9595 (1.0)           1;0  5.6762 (0.98)          6           1
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

So after seen those numbers I agree with you that the timezone should always be applied if available.

[prokofyevDmitry]

Narrowed down the pretty perf diff between datetime.fromtimestamp and tzutil.utcfromtimestamp (used respectively in DateTime64's _read_binary_tz and _read_binary_naive methods) which is just the application of timezone to none inside the tzutil function.
So the conclusion is that it is actually faster to decode into tz aware datetime objects.

------------------------------------------------------------------------------------------------------- benchmark: 2 tests ------------------------------------------------------------------------------------------------------
Name (time in ns)                                 Min                    Max                  Mean              StdDev                Median                IQR              Outliers  OPS (Kops/s)            Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_benchmark_datetime_fromtimestamp        272.8822 (1.0)      31,117.2352 (1.0)        343.9738 (1.0)      188.4416 (1.0)        311.0001 (1.0)      10.2938 (1.0)      9686;20607    2,907.1986 (1.0)      197864          17
test_benchmark_tzutil_utcfromtimestamp     1,532.3007 (5.62)     64,513.0996 (2.07)     2,064.9458 (6.00)     810.3300 (4.30)     1,921.1999 (6.18)     62.6998 (6.09)     3950;10114      484.2742 (0.17)      62500          10
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

[genzgd]

Thanks for the detailed investigation! Feel free to submit a PR or our team will put it on the priority list. @joe-clickhouse

[joe-clickhouse]

Nice work @prokofyevDmitry! I've been following this, so thanks for the nice work. We have our Python roadmap planning meeting this week so I'll be sure to bring it up.

[prokofyevDmitry]

I created the PR #569

I have few questions about the best way to fix the tests that are broken by this PR, I added those questions in the PR's text.

[misuzu]

Please leave the ability to return timezone-naive dates, upgrading is going to be a nightmare for legacy projects...

[mitchhollander]

This is EXTREMELY problematic. It literally works exactly backwards of the correct behavior from a user perspective.

Let's say I have my server configured to utc default and I create a table:

CREATE OR REPLACE TABLE test.test_table (
    id UInt32, 
    user String, 
    created_at_utc DateTime('UTC') DEFAULT now('UTC'),
    created_at_naive DateTime DEFAULT now()
) 
ENGINE = MergeTree()
ORDER BY (user, id)

Insert a simple a row:

INSERT INTO test.test_table (id, user) VALUES (1, 'me')

When I query via clickhouse-connect (version 0.10.0):

client = clickhouse_connect.get_client(..., utc_tz_aware=None)
df = client.query_df("SELECT * FROM test.test_table")

I get that:

• created_at_utc is naive
• created_at_naive is Etc/UCT

Again, literally the exact opposite of the intended behavior. This is totally unacceptable IMO.

@misuzu I totally agree that upgrading the default behavior is a nightmare from a migration perspective, but there needs to be some level of update to wake me from the nightmare :)

[mitchhollander]

@joe-clickhouse I do not believe this is resolved per my above comment.