-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ES|QL: Fork - support GROK, CHANGE_POINT and COMPLETION in branches #129249
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
Conversation
| | changePointCommand | ||
| | completionCommand | ||
| | grokCommand | ||
| | {this.isDevVersion()}? inlinestatsCommand |
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.
we just extend the grammar to support these 4 commands.
what we miss are:
- some commands that are only available in snapshot that I am not sure when/if they will be available (like
RERANK) - DROP/KEEP/MV_EXPAND/RENAME which produce a parsing error when used inside fork branches, so we exclude them for now
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.
We discussed this and for now we don't need to support commands that are in snapshot.
So I am removing inlinestats
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Pinging @elastic/kibana-esql (ES|QL-ui) |
ChrisHegarty
left a comment
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.
LGTM
carlosdelest
left a comment
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.
Looks good!
| Spanish | null | fork2 | ||
| ; | ||
|
|
||
| forkBranchWithDrop-Ignore |
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.
Should we remove the -Ignore tests? They don't seem to be able to pass even when commands are supported (the results don't look right).
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'd like to keep them, because we would add them anyway when we fix the grammar in the next days.
|
the serverless checks failed, but it looked like it was a Buildkite fluke. |
related: #126553
Changes the FORK command so that we support more commands in FORK branches.
We want to change the grammar so that we support all commands, but we get a weird error for some of them.
The only ones that are not commands available only in snapshot, that we don't support yet are DROP/MV_EXPAND/RENAME/KEEP.
What these have in common is that they use
qualifiedNamePatternsorqualifiedName.In the meantime for all the rest, we added CSV tests to make sure we actually support them.