CASSANDRA-19546: Add to_human_size and to_human_duration function #3741
Conversation
patch by Stefan Miklosovic; reviewed by TBD for CASSANDRA-19546
|
|
||
| ==== format_bytes | ||
|
|
||
| This function looks at values in a column as if it was in bytes, and it will convert it to whatever a user pleases. There are three ways how to call this function |
| ) | ||
| ---- | ||
|
|
||
| Imagine that we wanted to look at `val` values as if they were in mebibytes. We would like to have more human-friendly output in order to not visually divide the values by 1024 in order to get them in respective bigger units. The following function call may take just a column itself as an argument, and it will |
There was a problem hiding this comment.
TIL what a mebibyte is :-)
https://simple.wikipedia.org/wiki/Mebibyte
| */ | ||
| public static class FormatTimeFct extends NativeScalarFunction | ||
| { | ||
| private static final String FUNCTION_NAME = "format_time"; |
There was a problem hiding this comment.
Should we be more specific with the naming and have it like human_format_time?
|
|
||
| long value = getValue(arguments); | ||
|
|
||
| if (value < 0) |
There was a problem hiding this comment.
Is there a reason we don't want to support negative times? (as in before now?)
| sourceUnit = MILLISECONDS; | ||
| targetUnitAsString = arguments.get(1); | ||
| } | ||
| else |
There was a problem hiding this comment.
Should we also check for arguments.size() > 3? I think we should also return an `InvalidRequestException on that case instead of silently ignoring the extra args.
| else if (targetUnit == DAYS) | ||
| return x(duration::toDays); | ||
| else | ||
| throw new InvalidRequestException("unsupported target unit " + targetUnit); |
There was a problem hiding this comment.
This makes me wonder, should we move the validate unit calls inside this method to avoid having to call them from the main execute method?
| */ | ||
| public static class FormatBytesFct extends NativeScalarFunction | ||
| { | ||
| private static final String FUNCTION_NAME = "format_bytes"; |
There was a problem hiding this comment.
Should we be more specific with the naming and have it like human_format_bytes?
| sourceUnit = BYTES; | ||
| targetUnit = validateUnit(arguments.get(1)); | ||
| } | ||
| else |
There was a problem hiding this comment.
Same as before: Should we also check for arguments.size() > 3? I think we should also return an InvalidRequestException on that case instead of silently ignoring the extra args.
| else if (targetUnit == GIBIBYTES) | ||
| return sourceUnit.toGibibytes(valueToConvert); | ||
| else | ||
| throw new InvalidRequestException("unsupported target unit " + targetUnit); |
There was a problem hiding this comment.
Same as before: This makes me wonder, should we move the validate unit calls inside this method to avoid having to call them from the main execute method?
| import static org.apache.cassandra.cql3.CQL3Type.Native.VARINT; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| public class FormatBytesFctTest extends CQLTester |
There was a problem hiding this comment.
These tests look like could benefit from the usage of QuickTheories. There are other tests in the code base using it. Maybe it is worth taking a look to increase coverage against unexpected inputs.
Thanks for sending a pull request! Here are some tips if you're new here:
Commit messages should follow the following format:
The Cassandra Jira