Conversation
|
Workflow [PR], commit [0b1e311] Summary: ❌
|
ee0afd1 to
248cfdd
Compare
248cfdd to
9d655f6
Compare
|
I'll do one final round of review to ensure that everything looks ok and then we can merge. |
|
sure @bharatnc lmk if there's any changes needed. |
102a2df to
3187f1c
Compare
is failing now, I didn't do any change related to this? what should I do? |
Yes, I'm not sure why it fails - I will check. |
b222ec0 to
d9838dc
Compare
tests/queries/0_stateless/03096_text_log_format_string_args_not_empty.sh
Show resolved
Hide resolved
|
currently these tests fail: https://github.com/ClickHouse/ClickHouse/actions/runs/18041406470/job/51345778282?pr=83058 i will re-trigger the pipeline to see if these fail again, if they do, i will analyze the cause, from the first inference of the logs, it appears like one test failed because it was unable to load AWS credential, other failed on cleanup so re-triggering these should work |
|
The test failures not related, no need to re-trigger. Some small fixes suggested, otherwise looks good. |
c26da29 to
c17890c
Compare
bharatnc
left a comment
There was a problem hiding this comment.
Mostly looks good, one comment needs addressing.
bharatnc
left a comment
There was a problem hiding this comment.
LGTM, can merge once builds finish.
|
@bharatnc I added in the comments, do I need to add somewhere else as well to get PR job pass? |
I think it's happening because the String category is currently not autogenerated - only allowed ones are these. Looks like you might need to add the docs for the new function here in the markdown for string-functions. |
072baab to
a267f1f
Compare
|
Btw, in future, please avoid rebase + force pushes to PR if you want to bring it up to date with master. Instead, do |
|
I thought doing a merge would increase a commit, so I did rebase, I will use merge from now on |
|
@bharatnc since last two commits one thing to note is that in all the PRs changes are related to some function code, do we need to increase some limit here? |
|
|
Test failures not related. But just in case, let's wait for a fresh set of builds before merging. |
d44aa71 to
0b1e311
Compare
|
Test failure not related:
|
22fded9
|
Thanks a lot @bharatnc !! |
|
Oh, looks like it's not yours... |
Add new 'conv' function which provides support of converting numbers between '2-36' into each other,
sample usage include:
Many of the decisions like invalid character handling, whitespace handling are inspired from
MySQLimplementationand it closes #4633
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
New
convfunction for converting numbers between bases, currently supports bases from2-36Documentation entry for user-facing changes
conv
Converts numbers between different number bases. The function takes a number in one base and converts it to another base, supporting bases from 2 to 36. This function is compatible with MySQL's CONV() function and handles partial invalid inputs by processing valid digits until the first invalid character.
Syntax
conv(number, from_base, to_base)Arguments
number— The number to convert. Can be a string or numeric type. Leading and trailing whitespace is automatically trimmed. String or Numberfrom_base— The source base (2-36). Must be an integer. Integerto_base— The target base (2-36). Must be an integer. IntegerImplementation Details
Returned value
Examples
Query:
Response:
Query:
Response:
Query:
Response:
Query:
Response: