[ES|QL] SET header instruction parsing
#236460
Conversation
vadimkibana
force-pushed
the
esql-set-instruction-2
branch
from
4c56f6b to
21002d4
Compare
vadimkibana
changed the title
SET header instructionsSET header instruction parsing
vadimkibana
added
review
release_note:skip
|
Pinging @elastic/kibana-esql (Team:ESQL) |
| const result = parseEsqlQuery(esqlQuery); | ||
| expect(result.errors.length).toEqual(1); | ||
| expect(result.errors[0].message.startsWith('SyntaxError:')).toBeTruthy(); | ||
| expect(result.errors.length > 0).toEqual(true); |
There was a problem hiding this comment.
Should we expect a precise errors number? What if there are unexpected error pop up due to a bug?
In case it's still preferable to track any number of errors it'd be nice to add a shot comment explaining why more than one error is expected. And additionally a small improvement to the assertion could be done
| expect(result.errors.length > 0).toEqual(true); | |
| expect(result.errors.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
I have moved it back to one error to get it approved: 37d53a8
The reason for not relying on one error is because it is the internals of the parser how many errors it returns, and it sometimes changes over time as the ES|QL language evolves.
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
History
|
## Summary Closes elastic#233942 Implements parsing for `SET` pseudo-commands, which come before the main query. ``` SET key1 = 123; SET key2 = "asdf"; FROM index | LIMIT 123 ``` In `query` AST node introduces `header` section, which contain AST nodes before the main command. Although each `SET` instruction can now have a single key-value pair, it is implemented such that it will be able to store a list of those pairs. The reason is because in Elasticsearch team first implemented it to hold multiple key-value pairs, then changed to store only one, further they commented ([see thread](elastic/elasticsearch#134029)) that they may add support for a list of key-value pairs to `SET` back. ### 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
5 participants
Summary
Partially addresses #233942
Implements parsing for
SETpseudo-commands, which come before the main query.In
queryAST node introducesheadersection, which contain AST nodes before the main command.Although each
SETinstruction can now have a single key-value pair, it is implemented such that it will be able to store a list of those pairs. The reason is because in Elasticsearch team first implemented it to hold multiple key-value pairs, then changed to store only one, further they commented (see thread) that they may add support for a list of key-value pairs toSETback.Checklist