[pkg/telemetryquerylanguage] Added Int factory function (#11810) #13441
Conversation
|
|
mcdoker18
force-pushed
the
feat/int_func
branch
from
7eb1a3e to
c36c409
Compare
|
@mcdoker18 Thanks for the contribution. Please split this into 2 PRs, one that adds the function to TQL and then one after that adds the function to the transform processor. |
mcdoker18
force-pushed
the
feat/int_func
branch
from
c36c409 to
66543ba
Compare
mcdoker18
force-pushed
the
feat/int_func
branch
from
66543ba to
7a0240e
Compare
|
|
||
| `Int(value)` | ||
|
|
||
| The `Int` factory function converts a float, bool and string data to the int type. |
There was a problem hiding this comment.
Can you explicitly list out the types converted, and explicitly state that the returned type is int64
| if value { | ||
| return int64(1) | ||
| } | ||
|
|
| }, | ||
| { | ||
| name: "float64 without decimal", | ||
| value: float64(55.0), |
There was a problem hiding this comment.
| value: float64(55.0), | |
| value: float64(55), |
|
@TylerHelmuth I was planning to add the changelog in the next PR, do you mind? |
Please add a changelog entry for the function addition to pkg/telemetryquerylanguage. We'll add a second changelog entry for the transform processor when the processor is updated to register the function. We need entries for both because the TQL package can be used independently of the transform processor. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
There was a problem hiding this comment.
One question about the interpretation of strings.
Also, maybe this is out of scope for this PR, but this makes me immediately want equivalent functions for Float() and String() and Bool(). @TylerHelmuth thoughts?
I'm going to approve because I don't think either of these is a blocker.
| case int64: | ||
| return value | ||
| case string: | ||
| intValue, err := strconv.ParseInt(value, 10, 64) |
There was a problem hiding this comment.
If we were to specify 0 here instead of 10, then ParseInt will:
- handle base prefixes of 0x (hex), 0b (binary), and 0 or 0o (octal)
- support embedded _ for spacing (like 1_000_000)
I feel like that might be useful and natural. @TylerHelmuth do you agree, or do you think that's out of scope?
|
@kentquirk ya we probably should have more casting functions, or build casting into the grammar natively. |
Description:
Added the Int function to convert string and float type to int.
Use cases are described here #11810
Link to tracking Issue: #11810
Testing: unit tests
Documentation: updated the readme file