-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ES|QL] ToAggregateMetricDouble function #124595
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
Changes from all commits
2f64c49
7a49ca5
efab117
58104fb
61c3fef
a65a1d3
227de7d
2ccfd46
5c4d965
0594990
d2531de
ce814cf
804e980
ec2862b
4749325
b5afb7f
9b47fc3
a2613f3
9e4f3b4
e469df8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 124595 | ||
| summary: '`ToAggregateMetricDouble` function' | ||
| area: "ES|QL" | ||
| type: enhancement | ||
| issues: [] |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Can in a follow up the usage of
CompositeBlockfor aggregate metric double field be replaced withAggregateMetricDoubleBlockBuilder?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.
I'm not sure I understand? There's no block creation here, just fetching the value from the block that already exists, so no building is involved
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.
Sorry, this is comment doesn't make sense here. I think I was more wondering about whether in
AggregateMetricDoubleBlockBuilderwe should use build a specialized block instead of buildingCompositeBlock.ComposteBlockis very generic and using this here now I think is ok, I'm just wondering whether in the future this makes sense? (when there are more usages ofCompositeBlock)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.
We could if we wanted, but I'm not sure it's all that important. The entire compute engine is pretty ok with runtime types and treating blocks in specific ways based on the input. We don't have a special type for utf-8 text - just bytes - so I'm not sure we'd need a special type for these blocks.
If we want it we can have it for sure, but I don't think it's all that important.