clickhouse-driver icon indicating copy to clipboard operation
clickhouse-driver copied to clipboard

Select datetime accurate to millisecond

Open veotani opened this issue 2 years ago • 4 comments

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

  • Version of package with the problem: 0.2.3
  • ClickHouse server version. Version can be obtained by running SELECT version() query: 21.12.3.32
  • Python version: 3.9.7

veotani avatar May 03 '22 14:05 veotani

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.

xzkostyan avatar May 04 '22 21:05 xzkostyan

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.

veotani avatar May 05 '22 14:05 veotani

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.

xzkostyan avatar Jul 10 '22 12:07 xzkostyan

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

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

FedericoCeratto avatar Jul 19 '23 17:07 FedericoCeratto