Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ClickHouse Compatibility] Support receive of high-precision datetime values from server #1379

Closed
MaceWindu opened this issue Oct 7, 2023 · 4 comments
Milestone

Comments

@MaceWindu
Copy link
Contributor

Hi,

I have another compatibility request similar to #1185.

With last month release of ClickHouse updated mysql protocol implementation and now sends datetime values using DateTime column type instead of string. This results in error in provider for values with precision 7+ (CH supports precision up to 9).

It is probably protocol violation, but it looks like easy fix, so it would be nice to have it.

Server sends values like 2021-02-28 17:14:55.1231234 and it fails here due to logic expecting 6 digits in fractional second part

Also filled issue on CH side

@bgrainger
Copy link
Member

I haven't tested yet, but it looks like MySqlConnector will just silently lose data here? It would be better to throw an exception than to silently lose data.

We could support up to seven decimal digits (the resolution of DateTime) and then throw an exception for higher-precision values.

@MaceWindu
Copy link
Contributor Author

"1234567" part parsed as 1234567 numer, then 567 (val % 1000) used as microseconds, which is wrong in this case and remaining part 1234 (val /1000) passed to DateTime constructor as milliseconds. Which leads to out-of-range exception from DateTime as it expects value in 0-999 range.

I agree with your proposal to throw exception when value doesn't fit datetime (I was under expression Microsoft added nanoseconds support recently, but all they did is adding DateTime.Nanosecond property with 100 step).

Also note that MySqlDateTime type also use same logic to calculate milliseconds: val / 1000

@slvrtrn
Copy link

slvrtrn commented Oct 10, 2023

Will be fixed on the ClickHouse side by ClickHouse/ClickHouse#55479

@bgrainger
Copy link
Member

There's still a bug in MySqlConnector when parsing fractional seconds like .0000001 that don't overflow; the microseconds of the parsed value are off by a factor of ten. I plan to fix that at the same time as adding support for reading up to seven decimal digits off the wire (although no servers that speak the MySQL protocol might actually send that).

@bgrainger bgrainger added this to the 2.3 milestone Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants