-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[DOCS] Improve ES|QL functions reference for functions E-Z #104623
[DOCS] Improve ES|QL functions reference for functions E-Z #104623
Conversation
Documentation preview: |
Pinging @elastic/es-docs (Team:Docs) |
Pinging @elastic/es-analytical-engine (Team:Analytics) |
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
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 gave this a first go - will continue tomorrow.
`column`:: | ||
Column from which to return the maximum value. |
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 does not have to be a column - it can be an expression.
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've got a separate PR brewing that updates the docs for the aggregate functions, now that they support inline expressions. It'll be fixed in that PR.
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 realized that also in other places (aside from aggs), column
is used instead of expression
- e.g. in mv_dedupe
; so it'd probably be best to go over the use of column
in all functions and double check, because they generally should accept expressions instead of just field/column names now.
Co-authored-by: Alexander Spies <[email protected]>
@elasticmachine merge upstream |
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.
Thanks Abdon!
Generally LGTM. I mostly have minor remarks, except maybe for the type tables that could be included systematically, I think.
I also made some comments to some text that was only moved - feel free to disregard this for this PR, as it may extend its scope over your initial intention.
|
||
Returns the character length of a string. | ||
|
||
*Example* |
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.
There's a type table for length now that we should probably include for consistency across our function docs.
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.
There is a separate issue about making sure all functions have a railroad diagram and type table: #100558 . I wanted to keep that out of scope for this PR, because it is large enough as it is. What do you think?
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.
Ah, sorry - of course, let's tackle that in the other issue!
docs/reference/esql/functions/median-absolute-deviation.asciidoc
Outdated
Show resolved
Hide resolved
`column`:: | ||
Column from which to return the maximum value. |
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 realized that also in other places (aside from aggs), column
is used instead of expression
- e.g. in mv_dedupe
; so it'd probably be best to go over the use of column
in all functions and double check, because they generally should accept expressions instead of just field/column names now.
|
||
*Supported types* | ||
|
||
The input type must be of a numeric type and result is always `double`. |
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.
to_degrees
has a type table now that can be included. This applies to multiple other functions (at least the to_...
conversion functions) - best to go over them and double check where type tables can be included.
|
||
*Description* | ||
|
||
Converts an input string to an IP value. | ||
|
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.
Missing "Supported types"
…s' into docs-esql-more-functions-examples
Thank you for your thorough review @alex-spies ! I have applied all your suggestions, except those that involve missing type tables or aggregate functions. Those will be addressed in follow-up PRs. |
…04623) * Functions E-Z * Incorporate changes from elastic#103686 * More functions * More functions * Update docs/reference/esql/functions/floor.asciidoc Co-authored-by: Liam Thompson <[email protected]> * Update docs/reference/esql/functions/left.asciidoc Co-authored-by: Liam Thompson <[email protected]> * Apply suggestions from code review Co-authored-by: Alexander Spies <[email protected]> * Review feedback * Fix geo_shape description * Change 'colum'/'field' into 'expressions' * Review feedback * One more --------- Co-authored-by: Liam Thompson <[email protected]> Co-authored-by: Alexander Spies <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Follow up of #103447
Cleans up the ES|QL function reference, by: