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
Add sub-second precision to formatReadableTimeDelta
#54250
Add sub-second precision to formatReadableTimeDelta
#54250
Conversation
tests/queries/0_stateless/02872_formatreadabletimedelta_subseconds.sql
Outdated
Show resolved
Hide resolved
This is an automated comment for commit d2c9533 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
|
||
``` text | ||
┌─bad_representation──────────────────────────────────────────────────┐ | ||
│ 123 seconds, 299 milliseconds, 999 microseconds and 999 nanoseconds │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong.
We should use the same logic as to shortest
in double-conversion
.
Example:
milovidov@milovidov-desktop:~$ clickhouse-local
ClickHouse local version 23.9.1.1.
milovidov-desktop :) SELECT 123.3
SELECT 123.3
Query id: 832735e7-d22a-4669-8276-16a91ce93aa8
┌─123.3─┐
│ 123.3 │
└───────┘
1 row in set. Elapsed: 0.001 sec.
milovidov-desktop :) SELECT 123.3::Decimal(10, 10)
SELECT CAST('123.3', 'Decimal(10, 10)')
Query id: 2ec5e8a8-d276-455e-8e45-f1020716fe09
┌─CAST('123.3', 'Decimal(10, 10)')─┐
│ 123.3 │
└──────────────────────────────────┘
1 row in set. Elapsed: 0.001 sec.
milovidov-desktop :) SELECT CAST(123.3 AS Decimal(10, 10))
SELECT CAST(123.3, 'Decimal(10, 10)')
Query id: 135f276b-a094-4a81-9a85-f1eda5f4139d
┌─CAST(123.3, 'Decimal(10, 10)')─┐
│ 123.3 │
└────────────────────────────────┘
1 row in set. Elapsed: 0.001 sec.
Too many commits - need to squash them. |
244bf86
to
6bbb3ec
Compare
6bbb3ec
to
e400279
Compare
e400279
to
9559a69
Compare
The only fail: |
{ | ||
const ColumnPtr & minimum_unit_column = arguments[2].column; | ||
const ColumnConst * minimum_unit_const_col = checkAndGetColumnConstStringOrFixedString(minimum_unit_column.get()); | ||
if (minimum_unit_const_col) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if the argument is not constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be an exception: Argument at index 2 for function formatReadableTimeDelta must be constant.
It is an always-constant one (just like the first arg was prior to this PR, see l.86):
ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {1, 2}; }
Add sub-second precision to
formatReadableTimeDelta
.Closes #53156
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add sub-second precision to
formatReadableTimeDelta
Documentation entry for user-facing changes