Skip to content

[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.

Sign up for GitHub

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

Merged
pmpailis merged 10 commits into from
Oct 21, 2025
Merged

[ES|QL] Allow single fork branch #136805

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ca1d8c6
iter
pmpailis Oct 16, 2025
c190d1b
iter
pmpailis Oct 20, 2025
d3ba27d
Update docs/changelog/136805.yaml
pmpailis Oct 20, 2025
9cd6db9
Merge branch 'main' into allow_single_fork_branch
pmpailis Oct 20, 2025
2739004
iter
pmpailis Oct 20, 2025
142a7d4
Merge branch 'main' into allow_single_fork_branch
pmpailis Oct 20, 2025
a8ce7b9
addressing PR comments - adding test cases for no branches in fork an…
pmpailis Oct 20, 2025
76570d8
Merge branch 'allow_single_fork_branch' of github.com:pmpailis/elasti…
pmpailis Oct 20, 2025
b0a0f08
Merge branch 'main' into allow_single_fork_branch
pmpailis Oct 20, 2025
5ec7f03
Merge branch 'main' into allow_single_fork_branch
pmpailis Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/136805.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 136805
summary: Allow single fork branch
area: ES|QL
type: enhancement
issues:
- 135825
10 changes: 8 additions & 2 deletions .../plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ForkIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,8 +1005,14 @@ public void testOneSubQuery() {
| FORK
( WHERE content:"fox" )
""";
var e = expectThrows(ParsingException.class, () -> run(query));
assertTrue(e.getMessage().contains("Fork requires at least 2 branches"));
try (var resp = run(query)) {
assertColumnTypes(resp.columns(), List.of("text", "integer", "keyword"));
assertColumnNames(resp.columns(), List.of("content", "id", "_fork"));
Iterable<Iterable<Object>> expectedValues = List.of(
Arrays.stream(new Object[] { "The quick brown fox jumps over the lazy dog", 6, "fork1" }).toList()
);
assertValues(resp.values(), expectedValues);
}
}

public void testForkWithinFork() {
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,9 +848,6 @@ private void checkForRemoteClusters(LogicalPlan plan, Source source, String comm
@SuppressWarnings("unchecked")
public PlanFactory visitForkCommand(EsqlBaseParser.ForkCommandContext ctx) {
List<PlanFactory> subQueries = visitForkSubQueries(ctx.forkSubQueries());
if (subQueries.size() < Fork.MIN_BRANCHES) {
throw new ParsingException(source(ctx), "Fork requires at least " + Fork.MIN_BRANCHES + " branches");
}
if (subQueries.size() > Fork.MAX_BRANCHES) {
throw new ParsingException(source(ctx), "Fork supports up to " + Fork.MAX_BRANCHES + " branches");
}
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Fork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,10 @@ public class Fork extends LogicalPlan implements PostAnalysisPlanVerificationAwa

public static final String FORK_FIELD = "_fork";
public static final int MAX_BRANCHES = 8;
public static final int MIN_BRANCHES = 2;
private final List<Attribute> output;

public Fork(Source source, List<LogicalPlan> children, List<Attribute> output) {
super(source, children);
if (children.size() < MIN_BRANCHES) {
throw new IllegalArgumentException("FORK requires more than " + MIN_BRANCHES + " branches, got: " + children.size());
}
if (children.size() > MAX_BRANCHES) {
throw new IllegalArgumentException("FORK supports up to " + MAX_BRANCHES + " branches, got: " + children.size());
}
Expand Down
6 changes: 0 additions & 6 deletions ...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3861,12 +3861,6 @@ public void testForkAllCommands() {
}

public void testInvalidFork() {
expectError("FROM foo* | FORK (WHERE a:\"baz\")", "line 1:13: Fork requires at least 2 branches");
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");

@ioanatia ioanatia Oct 20, 2025

Copy link
Copy Markdown
Member

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.

@pmpailis pmpailis Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

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 :)


expectError("""
FROM foo*
| FORK (where true) (where true) (where true) (where true)
Expand Down
Loading