[ES|QL] AST creates IS (NOT) NULL node even when it is incomplete in the query string
#199401
Labels
Feature:ES|QL
ES|QL related features in Kibana
impact:low
Addressing this issue will have a low level of impact on the quality/strength of our product.
Team:ESQL
ES|QL related features in Kibana
technical debt
Improvement of the software architecture and operational architecture
Comments
drewdaemon
added
Feature:ES|QL
|
Pinging @elastic/kibana-esql (Team:ESQL) |
1 task
stratoula
added a commit
that referenced
this issue
Nov 11, 2024
## Summary Part of #195418 **NOTES** - need to make sure these don't regress - #195771 - #197139 - suggesting variables after binary operator (e.g. `field + <suggest>` - I've noticed that incomplete null statements such as `is n` are corrected in our syntax tree. This sends the autocomplete down a "completed operator expression" route as opposed to an unknown operator or "to right of column" route. So, `... | EVAL foo IS N/` is interpreted as `... | EVAL foo IS NULL /`.<br><br>It accidentally works (lol) because the logic for `<operator-expression> <suggest>` suggests operators that accept the return type of the existing operator expression as their left-hand argument. Since `foo IS NULL` is of type `boolean` and `IS NULL` accepts boolean values, it gets included in the suggestion list which Monaco then filters by the actual prefix (`IS N`). 🤣 <br><br>([issue](#199401)) ### 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]>
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this issue
Nov 11, 2024
## Summary Part of elastic#195418 **NOTES** - need to make sure these don't regress - elastic#195771 - elastic#197139 - suggesting variables after binary operator (e.g. `field + <suggest>` - I've noticed that incomplete null statements such as `is n` are corrected in our syntax tree. This sends the autocomplete down a "completed operator expression" route as opposed to an unknown operator or "to right of column" route. So, `... | EVAL foo IS N/` is interpreted as `... | EVAL foo IS NULL /`.<br><br>It accidentally works (lol) because the logic for `<operator-expression> <suggest>` suggests operators that accept the return type of the existing operator expression as their left-hand argument. Since `foo IS NULL` is of type `boolean` and `IS NULL` accepts boolean values, it gets included in the suggestion list which Monaco then filters by the actual prefix (`IS N`). 🤣 <br><br>([issue](elastic#199401)) ### 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 2466a17)
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this issue
Nov 11, 2024
## Summary Part of elastic#195418 **NOTES** - need to make sure these don't regress - elastic#195771 - elastic#197139 - suggesting variables after binary operator (e.g. `field + <suggest>` - I've noticed that incomplete null statements such as `is n` are corrected in our syntax tree. This sends the autocomplete down a "completed operator expression" route as opposed to an unknown operator or "to right of column" route. So, `... | EVAL foo IS N/` is interpreted as `... | EVAL foo IS NULL /`.<br><br>It accidentally works (lol) because the logic for `<operator-expression> <suggest>` suggests operators that accept the return type of the existing operator expression as their left-hand argument. Since `foo IS NULL` is of type `boolean` and `IS NULL` accepts boolean values, it gets included in the suggestion list which Monaco then filters by the actual prefix (`IS N`). 🤣 <br><br>([issue](elastic#199401)) ### 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]>
tkajtoch
pushed a commit
to tkajtoch/kibana
that referenced
this issue
Nov 12, 2024
## Summary Part of elastic#195418 **NOTES** - need to make sure these don't regress - elastic#195771 - elastic#197139 - suggesting variables after binary operator (e.g. `field + <suggest>` - I've noticed that incomplete null statements such as `is n` are corrected in our syntax tree. This sends the autocomplete down a "completed operator expression" route as opposed to an unknown operator or "to right of column" route. So, `... | EVAL foo IS N/` is interpreted as `... | EVAL foo IS NULL /`.<br><br>It accidentally works (lol) because the logic for `<operator-expression> <suggest>` suggests operators that accept the return type of the existing operator expression as their left-hand argument. Since `foo IS NULL` is of type `boolean` and `IS NULL` accepts boolean values, it gets included in the suggestion list which Monaco then filters by the actual prefix (`IS N`). 🤣 <br><br>([issue](elastic#199401)) ### 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
I've noticed that incomplete null statements such as
IS Nare corrected in our syntax tree. This sends the autocomplete down a "completed operator expression" route as opposed to an unknown operator or "to right of column" route. So,... | EVAL foo IS N/is interpreted as... | EVAL foo IS NULL /unless the checks are purely text based (for example).We should consider whether this is the desired AST behavior (probably not...).
The optimistic code is here: https://github.com/elastic/kibana/blob/750452e/packages/kbn-esql-ast/src/parser/walkers.ts#L497-L509
(By the way,
... | EVAL foo IS N/accidentally correctly suggestsIS NULLbecause the logic for<operator-expression> <suggest>suggests operators that accept the return type of the existing operator expression as their left-hand argument. Sincefoo IS NULLis of type boolean andIS NULLaccepts boolean values, it gets included in the suggestion list which Monaco then filters by the actual prefix (IS N).)The text was updated successfully, but these errors were encountered: