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

Supported type conversions in v2 #574

Open
4 tasks done
gingerwizard opened this issue May 11, 2022 · 13 comments
Open
4 tasks done

Supported type conversions in v2 #574

gingerwizard opened this issue May 11, 2022 · 13 comments

Comments

@gingerwizard
Copy link
Collaborator

gingerwizard commented May 11, 2022

v1 of this driver allowed different types to be passed for a column at insertion type, with this necessary type conversion supported implicitly. Whilst this was convenient, it also allowed the potential loss of precision. Our aspiration for v2 is to support all possible implicit conversions that can be done safely with no precision loss.

Currently, v2 has a number of functional gaps where no precision would be lost and conversion could be safely performed. This issue will be used to track current gaps and the eventual compatibility matrix.

Current support recorded here.

Known gaps:

@gwvandesteeg
Copy link

Can you add String to UUID and vice versa, especially for those of us that don't use the google UUID library (github.com/google/uuid), but alternatives such as github.com/gofrs/uuid, which for us has a much cleaner API and more expansive functionality.

@quan-nguyen2
Copy link

I have the same issue with Date type, in v1, we are able to use string to represent date; but in v2, it returns the error converting string to Date is unsupported

@fiftin
Copy link
Contributor

fiftin commented Jul 12, 2022

Hi @gingerwizard ,

What should be done to close this issue?

@gingerwizard
Copy link
Collaborator Author

@fiftin we need to go through and compare v1 and v2 to see where type conversions are missing that also don't loose precision. Then we need to address gaps.
Right now I'm addressing as users report.

Noting @quan-nguyen2 issue I'll fix in next release.

@fiftin
Copy link
Contributor

fiftin commented Jul 12, 2022

@gingerwizard

So: time & date to/from string conversion? That's all?

@gingerwizard
Copy link
Collaborator Author

No im sure there are more cases we have gaps, I just haven't done a complete audit.

@gingerwizard
Copy link
Collaborator Author

Currently supported conversions now documented here. This will be merged to main. We welcome PRs to add X's.

@DGuang21
Copy link
Contributor

hi! @gingerwizard
I want to complete the type conversions of string to datetime, and I have completed part of it now, but I am worried that it will not meet the code specifications. Can you tell me whether the implementation of this PR is feasible? https://github.com/DGuang21/clickhouse-go/pull/1

The principle is to match the text through regular rules, and if it is a text time type that conforms to the rules, it will be converted.

@gingerwizard
Copy link
Collaborator Author

@DGuang21 if we support text im hesitant to support anything other than the default format supported by ClickHouse. Maybe we could consider https://clickhouse.com/docs/en/operations/settings/settings/#date_time_input_format but I'm not sure I want to get into supporting lots of date formats with different regexes.

Users can always use https://clickhouse.com/docs/en/sql-reference/functions/type-conversion-functions/#parsedatetimebesteffort if needed.

@DGuang21
Copy link
Contributor

@DGuang21 if we support text im hesitant to support anything other than the default format supported by ClickHouse. Maybe we could consider https://clickhouse.com/docs/en/operations/settings/settings/#date_time_input_format but I'm not sure I want to get into supporting lots of date formats with different regexes.

Users can always use https://clickhouse.com/docs/en/sql-reference/functions/type-conversion-functions/#parsedatetimebesteffort if needed.

Thanks for your reply . So should/can I go about doing string-to-time that supports a large number of types? Or like datetime64 only supports default type of time.

I'm also not sure if users really need it, but time types in business can be relatively many and complex, I just wrote a demo code based on the foundation of that can be done safely with no precision loss.

@gingerwizard
Copy link
Collaborator Author

Thanks for your reply . So should/can I go about doing string-to-time that supports a large number of types? Or like datetime64 only supports default type of time.

I would stay to the default. If there is a library that is well supported, written and tested for datetime parsing I would be potentially interested in this. I'd rather this code not live in the client, though.

@gingerwizard
Copy link
Collaborator Author

As an example, https://github.com/araddon/dateparse/issues has over 1.7k stars and still have issues and no commits in 2yrs. We open the can to support formats we have to take issues on it. I'd rather a simple line of "default" or do it outside of the client.

@gwvandesteeg
Copy link

gwvandesteeg commented Sep 4, 2023

Thanks for your reply . So should/can I go about doing string-to-time that supports a large number of types? Or like datetime64 only supports default type of time.

I would stay to the default. If there is a library that is well supported, written and tested for datetime parsing I would be potentially interested in this. I'd rather this code not live in the client, though.

There's a reason there's an ISO8601 standard for representing date and time in an unambiguous and lexographically sorted manner that are supported by go with default format strings using time.RFC3339 and time.RFC3339Nano (with caveats on this one). Supporting these out of the box are relatively straight forward to add and are commonly used in many systems and as such should really be the default supported format, and include a rather important suggestion in their documentation RFC3339 should be preferred for new protocols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants