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
Implement punycode encoding/decoding #57969
Conversation
This is an automated comment for commit 1d9c0db with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
Seeing @lemire as one of the main authors of the library gives me full confidence! |
We stand behind this library 100%. It is being used by millions of people as it is part of Node.js. We've had very few bug reports. We are going to keep improving it and extending it. |
Good to see Clickhouse adopting Ada Idna! Happy to help with any issues you might encounter. |
It is way too low-level to be user-friendly: https://pastila.nl/?0196792d/9db06e3e231f4096cc5ef0105a936dfa#cVjN8dNOdSrP22FyyT+pvw== |
Also, see how the error message is wrong. It shows "Internal error" and nothing else. It could not be alright. |
Or keep these functions around and introduce new ones - |
If the name is not decodable, we should return the default value (empty string), because otherwise the function is difficult to use on large real datasets. |
Sometimes such functions can be used to verify that the domain is validly encoded domain, though I guess sometimes it make sense to return default value. So maybe both variants should be added or something like |
I think it would be better to have both functions:
|
+1, agree - let's have both functions. |
@alexey-milovidov ClickHouse is fantastic. :-) |
Inspired by #34439, especially #34439 (comment), which was dormant for too long. This PR also uses the ada-idna library instead of a 15 year old homegrown implementation.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added functions for punycode encoding/decoding:
punycodeEncode()
andpunycodeDecode()
.Documentation entry for user-facing changes