Skip to content

Commit f275b71

Browse files
authored
ES|QL: Improve field resolution for FORK (#128501)
1 parent ba50798 commit f275b71

File tree

3 files changed

+20
-71
lines changed

3 files changed

+20
-71
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -453,12 +453,6 @@ tests:
453453
- class: org.elasticsearch.packaging.test.DockerTests
454454
method: test121CanUseStackLoggingConfig
455455
issue: https://github.com/elastic/elasticsearch/issues/128165
456-
- class: org.elasticsearch.xpack.esql.session.IndexResolverFieldNamesTests
457-
method: testForkFieldsWithKeepAfterFork
458-
issue: https://github.com/elastic/elasticsearch/issues/128271
459-
- class: org.elasticsearch.xpack.esql.session.IndexResolverFieldNamesTests
460-
method: testForkWithStatsInAllBranches
461-
issue: https://github.com/elastic/elasticsearch/issues/128272
462456
- class: org.elasticsearch.packaging.test.DockerTests
463457
method: test080ConfigurePasswordThroughEnvironmentVariableFile
464458
issue: https://github.com/elastic/elasticsearch/issues/128075

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -584,38 +584,17 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
584584
return result.withFieldNames(IndexResolver.ALL_FIELDS);
585585
}
586586

587-
Holder<Boolean> projectAll = new Holder<>(false);
588-
parsed.forEachExpressionDown(UnresolvedStar.class, us -> {// explicit "*" fields selection
589-
if (projectAll.get()) {
590-
return;
591-
}
592-
projectAll.set(true);
593-
});
594-
595-
if (projectAll.get()) {
587+
// TODO: Improve field resolution for FORK - right now we request all fields
588+
if (parsed.anyMatch(p -> p instanceof Fork)) {
596589
return result.withFieldNames(IndexResolver.ALL_FIELDS);
597590
}
598591

599-
Holder<Boolean> projectAfterFork = new Holder<>(false);
600-
Holder<Boolean> hasFork = new Holder<>(false);
601-
602-
parsed.forEachDown(plan -> {
592+
Holder<Boolean> projectAll = new Holder<>(false);
593+
parsed.forEachExpressionDown(UnresolvedStar.class, us -> {// explicit "*" fields selection
603594
if (projectAll.get()) {
604595
return;
605596
}
606-
607-
if (hasFork.get() == false && shouldCollectReferencedFields(plan, inlinestatsAggs)) {
608-
projectAfterFork.set(true);
609-
}
610-
611-
if (plan instanceof Fork fork && projectAfterFork.get() == false) {
612-
hasFork.set(true);
613-
fork.children().forEach(child -> {
614-
if (child.anyMatch(p -> shouldCollectReferencedFields(p, inlinestatsAggs)) == false) {
615-
projectAll.set(true);
616-
}
617-
});
618-
}
597+
projectAll.set(true);
619598
});
620599

621600
if (projectAll.get()) {

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/IndexResolverFieldNamesTests.java

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,21 +2009,21 @@ public void testForkFieldsWithKeepAfterFork() {
20092009
(WHERE d > 1000 AND e == "aaa" | EVAL c = a + 200)
20102010
| WHERE x > y
20112011
| KEEP a, b, c, d, x
2012-
""", Set.of("a", "a.*", "c", "c.*", "d", "d.*", "e", "e.*", "x", "x.*", "y", "y.*"));
2012+
""", ALL_FIELDS);
20132013
}
20142014

20152015
public void testForkFieldsWithKeepBeforeFork() {
20162016
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
20172017

20182018
assertFieldNames("""
20192019
FROM test
2020-
| KEEP a, b, c, d, x
2020+
| KEEP a, b, c, d, x, y
20212021
| WHERE a > 2000
20222022
| EVAL b = a + 100
20232023
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
20242024
(WHERE d > 1000 AND e == "aaa" | EVAL c = a + 200)
20252025
| WHERE x > y
2026-
""", Set.of("a", "a.*", "b", "b.*", "c", "c.*", "d", "d.*", "e", "e.*", "x", "x.*", "y", "y.*"));
2026+
""", ALL_FIELDS);
20272027
}
20282028

20292029
public void testForkFieldsWithNoProjection() {
@@ -2056,41 +2056,17 @@ public void testForkFieldsWithEnrichAndLookupJoins() {
20562056
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
20572057
assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
20582058

2059-
assertFieldNames(
2060-
"""
2061-
FROM test
2062-
| KEEP a, b, abc, def, z, xyz
2063-
| ENRICH enrich_policy ON abc
2064-
| EVAL b = a + 100
2065-
| LOOKUP JOIN my_lookup_index ON def
2066-
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
2067-
(STATS x = count(*), y=min(z))
2068-
| LOOKUP JOIN my_lookup_index ON xyz
2069-
| WHERE x > y OR _fork == "fork1"
2070-
""",
2071-
Set.of(
2072-
"x",
2073-
"y",
2074-
"_fork",
2075-
"a",
2076-
"c",
2077-
"abc",
2078-
"b",
2079-
"def",
2080-
"z",
2081-
"xyz",
2082-
"def.*",
2083-
"_fork.*",
2084-
"y.*",
2085-
"x.*",
2086-
"xyz.*",
2087-
"z.*",
2088-
"abc.*",
2089-
"a.*",
2090-
"c.*",
2091-
"b.*"
2092-
)
2093-
);
2059+
assertFieldNames("""
2060+
FROM test
2061+
| KEEP a, b, abc, def, z, xyz
2062+
| ENRICH enrich_policy ON abc
2063+
| EVAL b = a + 100
2064+
| LOOKUP JOIN my_lookup_index ON def
2065+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
2066+
(STATS x = count(*), y=min(z))
2067+
| LOOKUP JOIN my_lookup_index ON xyz
2068+
| WHERE x > y OR _fork == "fork1"
2069+
""", ALL_FIELDS);
20942070
}
20952071

20962072
public void testForkWithStatsInAllBranches() {
@@ -2104,7 +2080,7 @@ public void testForkWithStatsInAllBranches() {
21042080
(EVAL z = a * b | STATS m = max(z))
21052081
(STATS x = count(*), y=min(z))
21062082
| WHERE x > y
2107-
""", Set.of("a", "a.*", "b", "b.*", "c", "c.*", "z", "z.*"));
2083+
""", ALL_FIELDS);
21082084
}
21092085

21102086
public void testForkWithStatsAndWhere() {

0 commit comments

Comments
 (0)