Skip to content

Commit

Permalink
Fix datetime parameter binding with timezones
Browse files Browse the repository at this point in the history
  • Loading branch information
genzgd committed Nov 9, 2023
1 parent 0b11909 commit 2123d9c
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ In any case, this should not affect the basic usage of Superset with ClickHouse.
your Superset installation, the ClickHouse datasource will be available with either the enhanced connection dialog
or a standard SqlAlchemy DSN in the form of `clickhousedb://{username}:{password}@{host}:{port}`.

## 0.6.20, 2023-11-09
### Bug Fix
- Fixed an issue where client side binding of datetimes with timezones would produce the incorrect time string if
timezones differed between the client and ClickHouse server. Closes https://github.com/ClickHouse/clickhouse-connect/issues/268

## 0.6.19, 2023-11-07
### Bug Fixes
- In some circumstances it was possible to insert a `None` value into a non-Nullable String column. As this could mask
Expand Down
2 changes: 1 addition & 1 deletion clickhouse_connect/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
version = '0.6.19'
version = '0.6.20'
4 changes: 2 additions & 2 deletions clickhouse_connect/driver/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ def format_query_value(value: Any, server_tz: tzinfo = pytz.UTC):
if isinstance(value, str):
return format_str(value)
if isinstance(value, datetime):
if value.tzinfo is None:
value = value.replace(tzinfo=server_tz)
if value.tzinfo is not None or server_tz != pytz.UTC:
value = value.astimezone(server_tz)
return f"'{value.strftime('%Y-%m-%d %H:%M:%S')}'"
if isinstance(value, date):
return f"'{value.isoformat()}'"
Expand Down
3 changes: 2 additions & 1 deletion tests/integration_tests/test_params.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from datetime import datetime, date
from typing import Callable

from clickhouse_connect.driver import Client


def test_params(test_client: Client, table_context: callable):
def test_params(test_client: Client, table_context: Callable):
result = test_client.query('SELECT name, database FROM system.tables WHERE database = {db:String}',
parameters={'db': 'system'})
assert result.first_item['database'] == 'system'
Expand Down
55 changes: 54 additions & 1 deletion tests/integration_tests/test_timezones.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ def test_server_timezone(test_client: Client):
# This test is really for manual testing since changing the timezone on the test ClickHouse server
# still requires a restart. Other tests will depend on https://github.com/ClickHouse/ClickHouse/pull/44149
test_client.apply_server_timezone = True
test_datetime = datetime(2023, 3, 18, 16, 4, 25)
try:
date = test_client.query("SELECT toDateTime('2023-03-18 16:04:25') as st").first_row[0]
date = test_client.query('SELECT toDateTime(%s) as st', parameters=[test_datetime]).first_row[0]
if test_client.server_tz == pytz.UTC:
assert date.tzinfo is None
assert date == datetime(2023, 3, 18, 16, 4, 25, tzinfo=None)
Expand Down Expand Up @@ -105,3 +106,55 @@ def test_naive_timezones(test_client: Client):
else:
assert row[0].tzinfo is None
assert row[1].tzinfo is None


def test_timezone_binding_client(test_client: Client):
os.environ['TZ'] = 'America/Denver'
time.tzset()
denver_tz = datetime.now().astimezone().tzinfo
BaseQueryContext.local_tz = denver_tz
denver_time = datetime(2023, 3, 18, 16, 4, 25, tzinfo=denver_tz)
try:
server_time = test_client.query(
'SELECT toDateTime(%(dt)s) as dt', parameters={'dt': denver_time}).first_row[0]
assert server_time == denver_time
finally:
os.environ['TZ'] = 'UTC'
time.tzset()
BaseQueryContext.local_tz = pytz.UTC

naive_time = datetime(2023, 3, 18, 16, 4, 25)
server_time = test_client.query(
'SELECT toDateTime(%(dt)s) as dt', parameters={'dt': naive_time}).first_row[0]
assert server_time.astimezone(pytz.UTC) == naive_time.astimezone(pytz.UTC)

utc_time = datetime(2023, 3, 18, 16, 4, 25, tzinfo=pytz.UTC)
server_time = test_client.query(
'SELECT toDateTime(%(dt)s) as dt', parameters={'dt': utc_time}).first_row[0]
assert server_time.astimezone(pytz.UTC) == utc_time


def test_timezone_binding_server(test_client: Client):
denver_tz = datetime.now().astimezone().tzinfo
os.environ['TZ'] = 'America/Denver'
time.tzset()
BaseQueryContext.local_tz = denver_tz
denver_time = datetime(2022, 3, 18, 16, 4, 25, tzinfo=denver_tz)
try:
server_time = test_client.query(
'SELECT toDateTime({dt:DateTime}) as dt', parameters={'dt': denver_time}).first_row[0]
assert server_time == denver_time
finally:
os.environ['TZ'] = 'UTC'
time.tzset()
BaseQueryContext.local_tz = pytz.UTC

naive_time = datetime(2022, 3, 18, 16, 4, 25)
server_time = test_client.query(
'SELECT toDateTime({dt:DateTime}) as dt', parameters={'dt': naive_time}).first_row[0]
assert naive_time.astimezone(pytz.UTC) == server_time.astimezone(pytz.UTC)

utc_time = datetime(2020, 3, 18, 16, 4, 25, tzinfo=pytz.UTC)
server_time = test_client.query(
'SELECT toDateTime({dt:DateTime}) as dt', parameters={'dt': utc_time}).first_row[0]
assert server_time.astimezone(pytz.UTC) == utc_time
File renamed without changes.

0 comments on commit 2123d9c

Please sign in to comment.