-
Notifications
You must be signed in to change notification settings - Fork 25.7k
[ES|QL] Allow single fork branch #136805
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
[ES|QL] Allow single fork branch #136805
Conversation
|
Hi @pmpailis, I've created a changelog YAML for you. |
|
@ioanatia went through the documentation but could not find any specific references to the >= 2 branches limitation that needed to be updated. |
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
| expectError("FROM foo* | FORK (LIMIT 10)", "line 1:13: Fork requires at least 2 branches"); | ||
| expectError("FROM foo* | FORK (SORT a)", "line 1:13: Fork requires at least 2 branches"); | ||
| expectError("FROM foo* | FORK (WHERE x>1 | LIMIT 5)", "line 1:13: Fork requires at least 2 branches"); | ||
| expectError("FROM foo* | WHERE x>1 | FORK (WHERE a:\"baz\")", "Fork requires at least 2 branches"); |
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.
can we get a test for FORK with no branches? I think the grammar does not support it, but I don't think I added a test for that.
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.
Updated in a8ce7b9. I've also added a test in FuseIT to consume the result of a single-branched FORK. Let me know if you see any issues :)
|
also thought a bit more about the discussion we had around doing an optimization, like in the LogicalPlanBuilder when a single FORK branch is being used, to not even use the FORK logical plan, but have a "linear" plan. |
…d chaining fuse after a one-branch fork
…csearch into allow_single_fork_branch
|
Thanks for reviewing this (again) @ioanatia ! |
Removing the requirement for > 2 fork branches. So a query like
will now be allowed, and only
fork1will be added to the result set.Closes #135825