mymarilyn / clickhouse-driver

ClickHouse Python Driver with native interface support
https://clickhouse-driver.readthedocs.io
Other
1.21k stars 213 forks source link

Select datetime accurate to millisecond #306

Open veotani opened 2 years ago

veotani commented 2 years ago

Describe the bug Selecting using datetime parameter doesn't consider milliseconds. Probably, can use strftime like this %Y-%m-%d %H:%M:%S.%f here.

To Reproduce

client = make_client()
client.execute('CREATE TABLE IF NOT EXISTS test (ts DateTime64) ENGINE MergeTree ORDER BY ts')
milisec = timedelta(milliseconds=1)
start = datetime.now()
client.execute(f'INSERT INTO test VALUES', [(start,), (start + milisec,)])
selected_items = client.execute('SELECT ts FROM test WHERE ts > %(start)s', {'start': start})
assert selected_items[0][0] > start

Assert is failing but shouldn't.

Expected behavior Just like ClickHouse:

:) SELECT ts FROM test WHERE ts > '2022-05-03 16:35:10.731'

SELECT ts
FROM test
WHERE ts > '2022-05-03 16:35:10.731'

Query id: bf4e043b-b1b4-4084-9922-887d7d5c6d27

┌──────────────────────ts─┐
│ 2022-05-03 16:35:10.732 │
└─────────────────────────┘

Versions

xzkostyan commented 2 years ago

It seems that parameter substitution should be context dependent.

For DateTime fractional part throws exception

SELECT '2017-10-16 00:18:50.0' > now()

Query id: 8621f007-d80d-4264-8216-69b6bce9bf2f

0 rows in set. Elapsed: 0.002 sec. 

Received exception from server (version 22.4.3):
Code: 53. DB::Exception: Received from localhost:9000. DB::Exception: Cannot convert string 2017-10-16 00:18:50.0 to type DateTime: While processing '2017-10-16 00:18:50.0' > now(). (TYPE_MISMATCH)

But for DateTime64 fractional part doesn't throw exception

SELECT '2017-10-16 00:18:50.0' > now64()

Query id: 8dcf7526-c1cd-4a0f-9778-4a00afb9c27c

┌─greater('2017-10-16 00:18:50.0', now64())─┐
│                                         0 │
└───────────────────────────────────────────┘

I don't see any obvious condition for it. Data type information is unavailable during parameters substitution.

veotani commented 2 years ago

I think it's better to check whether datetime object has microseconds set and substitute them in this case. Currently it can be done with warning that can be removed in a time in case if you are afraid of users who are relying on this "rounding".

The error you get in 2nd example is enough for library user to spot his bug and drop microseconds by himself, but the current behaviour hides it. I would even say currently it has something in common with casting int64 to int32. 😄 We managed to find this in tests but others could get less lucky.

xzkostyan commented 2 years ago

I guess we should wait for https://github.com/ClickHouse/ClickHouse/issues/38995 and then such comparison will be available: SELECT '2017-10-16 00:18:50.0' > now(). Then we can just add .%f to format.

FedericoCeratto commented 1 year ago

As a workaround I had to convert the date from Python using

t.strftime("%Y-%m-%d %H:%M:%S.%f")[:-3]