[ES|QL] Support replacement range#190465
Conversation
|
/ci |
|
/ci |
|
@elasticmachine merge upstream |
|
/ci |
|
/ci |
|
Pinging @elastic/kibana-esql (Team:ESQL) |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
| ); | ||
|
|
||
| /** | ||
| * @TODO — this string manipulation is crude and can't support all cases |
There was a problem hiding this comment.
Should we make an issue for this todo to keep track or is that already existing?
There was a problem hiding this comment.
Good question. There is no issue. But, using the AST more is a key tenant of my vision for a re-architecture of this code. In other words, I am tracking this but I think an RFC/architecture document is the best first step here. From that, we can create more specific issues.
| testSuggestions('FROM a | WHERE doubleField IS N/', [ | ||
| { text: 'IS NOT NULL', rangeToReplace: { start: 28, end: 31 } }, | ||
| { text: 'IS NULL', rangeToReplace: { start: 28, end: 31 } }, | ||
| { text: '!= $0', rangeToReplace: { start: 31, end: 31 } }, |
There was a problem hiding this comment.
I'm not seeing this behavior in the editor with from kibana_sample_data_flights | where AvgTicketPrice is n. Makes sense that it suggests IS NOT NULL and IS NULL where the cursor is but why the rest?
There was a problem hiding this comment.
I had a choice: make it so that the range only gets added for IS NOT NULL and IS NULL or just add it in a generic way for all.
I opted for the latter because
- including an empty range is valid and does no harm
- it saves us adding a special case
- if new operators with spaces are ever added, it will work automatically
There was a problem hiding this comment.
That makes sense. Thanks for the context!
Summary
Fix #187184
Field names
Before
Screen.Recording.2024-08-15.at.12.18.38.PM.mov
After
Screen.Recording.2024-08-15.at.12.18.47.PM.mov
Functions with spaces
Before
Screen.Recording.2024-08-15.at.12.20.51.PM.mov
After
Screen.Recording.2024-08-15.at.12.20.14.PM.mov
Checklist