Skip to content

Commit 87270b1

Browse files
ioanatiakderusso
authored andcommitted
ES|QL: Add number of max branches for FORK (elastic#129834)
1 parent b79479e commit 87270b1

File tree

6 files changed

+68
-17
lines changed

6 files changed

+68
-17
lines changed

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/command/pipe/ForkGenerator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public CommandDescription generate(
3434
}
3535
}
3636

37-
int n = randomIntBetween(2, 10);
37+
int n = randomIntBetween(2, 8);
3838

3939
String cmd = " | FORK " + "( WHERE true ) ".repeat(n) + " | WHERE _fork == \"fork" + randomIntBetween(1, n) + "\" | DROP _fork";
4040

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ForkIT.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ public void testOneSubQuery() {
10071007
( WHERE content:"fox" )
10081008
""";
10091009
var e = expectThrows(ParsingException.class, () -> run(query));
1010-
assertTrue(e.getMessage().contains("Fork requires at least two branches"));
1010+
assertTrue(e.getMessage().contains("Fork requires at least 2 branches"));
10111011
}
10121012

10131013
public void testForkWithinFork() {
@@ -1047,6 +1047,17 @@ public void testProfile() {
10471047
}
10481048
}
10491049

1050+
public void testWithTooManySubqueries() {
1051+
var query = """
1052+
FROM test
1053+
| FORK (WHERE true) (WHERE true) (WHERE true) (WHERE true) (WHERE true)
1054+
(WHERE true) (WHERE true) (WHERE true) (WHERE true)
1055+
""";
1056+
var e = expectThrows(ParsingException.class, () -> run(query));
1057+
assertTrue(e.getMessage().contains("Fork requires less than 8 branches"));
1058+
1059+
}
1060+
10501061
private void createAndPopulateIndices() {
10511062
var indexName = "test";
10521063
var client = client().admin().indices();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -651,9 +651,13 @@ private void checkForRemoteClusters(LogicalPlan plan, Source source, String comm
651651
@SuppressWarnings("unchecked")
652652
public PlanFactory visitForkCommand(EsqlBaseParser.ForkCommandContext ctx) {
653653
List<PlanFactory> subQueries = visitForkSubQueries(ctx.forkSubQueries());
654-
if (subQueries.size() < 2) {
655-
throw new ParsingException(source(ctx), "Fork requires at least two branches");
654+
if (subQueries.size() < Fork.MIN_BRANCHES) {
655+
throw new ParsingException(source(ctx), "Fork requires at least " + Fork.MIN_BRANCHES + " branches");
656656
}
657+
if (subQueries.size() > Fork.MAX_BRANCHES) {
658+
throw new ParsingException(source(ctx), "Fork requires less than " + Fork.MAX_BRANCHES + " branches");
659+
}
660+
657661
return input -> {
658662
checkForRemoteClusters(input, source(ctx), "FORK");
659663
List<LogicalPlan> subPlans = subQueries.stream().map(planFactory -> planFactory.apply(input)).toList();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Fork.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,19 @@
3636
public class Fork extends LogicalPlan implements PostAnalysisPlanVerificationAware, TelemetryAware {
3737

3838
public static final String FORK_FIELD = "_fork";
39+
public static final int MAX_BRANCHES = 8;
40+
public static final int MIN_BRANCHES = 2;
3941
private final List<Attribute> output;
4042

4143
public Fork(Source source, List<LogicalPlan> children, List<Attribute> output) {
4244
super(source, children);
43-
if (children.size() < 2) {
44-
throw new IllegalArgumentException("requires more than two subqueries, got:" + children.size());
45+
if (children.size() < MIN_BRANCHES) {
46+
throw new IllegalArgumentException("FORK requires more than " + MIN_BRANCHES + " branches, got: " + children.size());
4547
}
48+
if (children.size() > MAX_BRANCHES) {
49+
throw new IllegalArgumentException("FORK requires less than " + MAX_BRANCHES + " subqueries, got: " + children.size());
50+
}
51+
4652
this.output = output;
4753
}
4854

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3354,7 +3354,15 @@ public void testForkAllReleasedCommands() {
33543354
( EVAL xyz = ( (a/b) * (b/a)) )
33553355
( WHERE a < 1 )
33563356
( KEEP a )
3357-
( DROP b )
3357+
| KEEP a
3358+
""";
3359+
3360+
var plan = statement(query);
3361+
assertThat(plan, instanceOf(Keep.class));
3362+
3363+
query = """
3364+
FROM foo*
3365+
| FORK
33583366
( RENAME a as c )
33593367
( MV_EXPAND a )
33603368
( CHANGE_POINT a on b )
@@ -3365,7 +3373,7 @@ public void testForkAllReleasedCommands() {
33653373
| KEEP a
33663374
""";
33673375

3368-
var plan = statement(query);
3376+
plan = statement(query);
33693377
assertThat(plan, instanceOf(Keep.class));
33703378
}
33713379

@@ -3383,7 +3391,15 @@ public void testForkAllCommands() {
33833391
( EVAL xyz = ( (a/b) * (b/a)) )
33843392
( WHERE a < 1 )
33853393
( KEEP a )
3386-
( DROP b )
3394+
3395+
| KEEP a
3396+
""";
3397+
var plan = statement(query);
3398+
assertThat(plan, instanceOf(Keep.class));
3399+
3400+
query = """
3401+
FROM foo*
3402+
| FORK
33873403
( RENAME a as c )
33883404
( MV_EXPAND a )
33893405
( CHANGE_POINT a on b )
@@ -3392,22 +3408,36 @@ public void testForkAllCommands() {
33923408
( FORK ( WHERE a:"baz" ) ( EVAL x = [ 1, 2, 3 ] ) )
33933409
( COMPLETION a = b WITH c )
33943410
( SAMPLE 0.99 )
3411+
| KEEP a
3412+
""";
3413+
plan = statement(query);
3414+
assertThat(plan, instanceOf(Keep.class));
3415+
3416+
query = """
3417+
FROM foo*
3418+
| FORK
33953419
( INLINESTATS x = MIN(a), y = MAX(b) WHERE d > 1000 )
33963420
( INSIST_🐔 a )
33973421
( LOOKUP_🐔 a on b )
33983422
| KEEP a
33993423
""";
3400-
3401-
var plan = statement(query);
3424+
plan = statement(query);
34023425
assertThat(plan, instanceOf(Keep.class));
34033426
}
34043427

34053428
public void testInvalidFork() {
3406-
expectError("FROM foo* | FORK (WHERE a:\"baz\")", "line 1:13: Fork requires at least two branches");
3407-
expectError("FROM foo* | FORK (LIMIT 10)", "line 1:13: Fork requires at least two branches");
3408-
expectError("FROM foo* | FORK (SORT a)", "line 1:13: Fork requires at least two branches");
3409-
expectError("FROM foo* | FORK (WHERE x>1 | LIMIT 5)", "line 1:13: Fork requires at least two branches");
3410-
expectError("FROM foo* | WHERE x>1 | FORK (WHERE a:\"baz\")", "Fork requires at least two branches");
3429+
expectError("FROM foo* | FORK (WHERE a:\"baz\")", "line 1:13: Fork requires at least 2 branches");
3430+
expectError("FROM foo* | FORK (LIMIT 10)", "line 1:13: Fork requires at least 2 branches");
3431+
expectError("FROM foo* | FORK (SORT a)", "line 1:13: Fork requires at least 2 branches");
3432+
expectError("FROM foo* | FORK (WHERE x>1 | LIMIT 5)", "line 1:13: Fork requires at least 2 branches");
3433+
expectError("FROM foo* | WHERE x>1 | FORK (WHERE a:\"baz\")", "Fork requires at least 2 branches");
3434+
3435+
expectError("""
3436+
FROM foo*
3437+
| FORK (where true) (where true) (where true) (where true)
3438+
(where true) (where true) (where true) (where true)
3439+
(where true)
3440+
""", "Fork requires less than 8 branches");
34113441

34123442
expectError("FROM foo* | FORK ( x+1 ) ( WHERE y>2 )", "line 1:20: mismatched input 'x+1'");
34133443
expectError("FROM foo* | FORK ( LIMIT 10 ) ( y+2 )", "line 1:33: mismatched input 'y+2'");

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/tree/EsqlNodeSubclassTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ private static Object makeMap(Class<? extends Node<?>> toBuildClass, Parameteriz
562562

563563
private static int randomSizeForCollection(Class<? extends Node<?>> toBuildClass) {
564564
int minCollectionLength = 0;
565-
int maxCollectionLength = 10;
565+
int maxCollectionLength = 8;
566566

567567
if (hasAtLeastTwoChildren(toBuildClass)) {
568568
minCollectionLength = 2;

0 commit comments

Comments
 (0)