[ES|QL] separate STATS autocomplete routine
#198224
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-autocomplete-routines-2
15 tasks
drewdaemon
added
Feature:ES|QL
drewdaemon
changed the title
STATS and WHERE autocomplete routinesSTATS autocomplete routine
|
Pinging @elastic/kibana-esql (Team:ESQL) |
stratoula
reviewed
Nov 1, 2024
There was a problem hiding this comment.
variables from previous EVAL commands are no longer suggested (e.g. ...| EVAL foo = 1 | STATS /).
I really think we want this. A user will create an eval variable to use it in stats. Is a very common scenario. Think about date buckets or variables with case.
- If I write the variable on my own now, the suggestions seem wrong
All the other decisions seem ok to me 👍
There was a problem hiding this comment.
Thanks @stratoula
variables from previous EVAL commands are no longer suggested (e.g. ...| EVAL foo = 1 | STATS /).
I really think we want this. A user will create an eval variable to use it in stats. Is a very common scenario. Think about date buckets or variables with case.
Just to make sure I understand—
- we want only variables (no fields) in
STATS / - but we want variables and fields in
STATS … BY /
Is that what you are saying?
If I write the variable on my own now, the suggestions seem wrong
From what I can see, this is no change in behavior.
There was a problem hiding this comment.
Synced offline, I misunderstood the behavior described. We are good
💚 Build Succeeded
Metrics [docs]Module Count
Page load bundle
History
|
stratoula
approved these changes
Nov 1, 2024
|
Starting backport for target branches: 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Nov 1, 2024
## Summary Part of elastic#195418 This PR moves the `STATS` completion logic to its own home. There are also a few changes in behavior. I am open for feedback on any of these. - the cursor is automatically advanced after accepting a comma suggestion - variables from previous `EVAL` commands are no longer suggested (e.g. `...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it seemed potentially unintended to suggest variables but no other columns such as field names. - a new variable is suggested for new expressions in the `BY` clause. Formerly, new variables were only suggested in the `STATS` clause. - `+` and `-` are no longer suggested after a completed function call within an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 = BUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but it feels unintended to me, especially since it only applied when the result of the function was assigned to a new variable in the `BY` clause. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <[email protected]> (cherry picked from commit 7f2b56f)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Nov 1, 2024
… (#198693) # Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] separate `STATS` autocomplete routine (#198224)](#198224) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Drew Tate","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-01T15:11:05Z","message":"[ES|QL] separate `STATS` autocomplete routine (#198224)\n\n## Summary\r\n\r\nPart of https://github.com/elastic/kibana/issues/195418\r\n\r\nThis PR moves the `STATS` completion logic to its own home.\r\n\r\nThere are also a few changes in behavior. I am open for feedback on any\r\nof these.\r\n- the cursor is automatically advanced after accepting a comma\r\nsuggestion\r\n- variables from previous `EVAL` commands are no longer suggested (e.g.\r\n`...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it\r\nseemed potentially unintended to suggest variables but no other columns\r\nsuch as field names.\r\n- a new variable is suggested for new expressions in the `BY` clause.\r\nFormerly, new variables were only suggested in the `STATS` clause.\r\n- `+` and `-` are no longer suggested after a completed function call\r\nwithin an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 =\r\nBUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but\r\nit feels unintended to me, especially since it only applied when the\r\nresult of the function was assigned to a new variable in the `BY`\r\nclause.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"7f2b56f0e687d466c20127d358d25a0456e51a03","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Feature:ES|QL","Team:ESQL","backport:version","v8.17.0"],"title":"[ES|QL] separate `STATS` autocomplete routine","number":198224,"url":"https://github.com/elastic/kibana/pull/198224","mergeCommit":{"message":"[ES|QL] separate `STATS` autocomplete routine (#198224)\n\n## Summary\r\n\r\nPart of https://github.com/elastic/kibana/issues/195418\r\n\r\nThis PR moves the `STATS` completion logic to its own home.\r\n\r\nThere are also a few changes in behavior. I am open for feedback on any\r\nof these.\r\n- the cursor is automatically advanced after accepting a comma\r\nsuggestion\r\n- variables from previous `EVAL` commands are no longer suggested (e.g.\r\n`...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it\r\nseemed potentially unintended to suggest variables but no other columns\r\nsuch as field names.\r\n- a new variable is suggested for new expressions in the `BY` clause.\r\nFormerly, new variables were only suggested in the `STATS` clause.\r\n- `+` and `-` are no longer suggested after a completed function call\r\nwithin an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 =\r\nBUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but\r\nit feels unintended to me, especially since it only applied when the\r\nresult of the function was assigned to a new variable in the `BY`\r\nclause.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"7f2b56f0e687d466c20127d358d25a0456e51a03"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198224","number":198224,"mergeCommit":{"message":"[ES|QL] separate `STATS` autocomplete routine (#198224)\n\n## Summary\r\n\r\nPart of https://github.com/elastic/kibana/issues/195418\r\n\r\nThis PR moves the `STATS` completion logic to its own home.\r\n\r\nThere are also a few changes in behavior. I am open for feedback on any\r\nof these.\r\n- the cursor is automatically advanced after accepting a comma\r\nsuggestion\r\n- variables from previous `EVAL` commands are no longer suggested (e.g.\r\n`...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it\r\nseemed potentially unintended to suggest variables but no other columns\r\nsuch as field names.\r\n- a new variable is suggested for new expressions in the `BY` clause.\r\nFormerly, new variables were only suggested in the `STATS` clause.\r\n- `+` and `-` are no longer suggested after a completed function call\r\nwithin an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 =\r\nBUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but\r\nit feels unintended to me, especially since it only applied when the\r\nresult of the function was assigned to a new variable in the `BY`\r\nclause.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"7f2b56f0e687d466c20127d358d25a0456e51a03"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Drew Tate <[email protected]>
nreese
pushed a commit
to nreese/kibana
that referenced
this pull request
Nov 1, 2024
## Summary Part of elastic#195418 This PR moves the `STATS` completion logic to its own home. There are also a few changes in behavior. I am open for feedback on any of these. - the cursor is automatically advanced after accepting a comma suggestion - variables from previous `EVAL` commands are no longer suggested (e.g. `...| EVAL foo = 1 | STATS /`). I'm not sure about this change, but it seemed potentially unintended to suggest variables but no other columns such as field names. - a new variable is suggested for new expressions in the `BY` clause. Formerly, new variables were only suggested in the `STATS` clause. - `+` and `-` are no longer suggested after a completed function call within an assignment in the `BY` clause (e.g. `... | STATS ... BY var1 = BUCKET(dateField, 1 day) /`. This behavior was encoded in our tests, but it feels unintended to me, especially since it only applied when the result of the function was assigned to a new variable in the `BY` clause. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Part of #195418
This PR moves the
STATScompletion logic to its own home.There are also a few changes in behavior. I am open for feedback on any of these.
EVALcommands are no longer suggested (e.g....| EVAL foo = 1 | STATS /). I'm not sure about this change, but it seemed potentially unintended to suggest variables but no other columns such as field names.BYclause. Formerly, new variables were only suggested in theSTATSclause.+and-are no longer suggested after a completed function call within an assignment in theBYclause (e.g.... | STATS ... BY var1 = BUCKET(dateField, 1 day) /. This behavior was encoded in our tests, but it feels unintended to me, especially since it only applied when the result of the function was assigned to a new variable in theBYclause.Checklist