Skip to content

Commit c8581b0

Browse files
authored
Fix field resolution for FORK (#128193)
1 parent b335c1a commit c8581b0

File tree

2 files changed

+150
-5
lines changed

2 files changed

+150
-5
lines changed

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

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -571,22 +571,45 @@ private void resolveInferences(
571571
}
572572

573573
static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicyMatchFields, PreAnalysisResult result) {
574-
if (false == parsed.anyMatch(plan -> plan instanceof Aggregate || plan instanceof Project)) {
574+
if (false == parsed.anyMatch(EsqlSession::shouldCollectReferencedFields)) {
575575
// no explicit columns selection, for example "from employees"
576576
return result.withFieldNames(IndexResolver.ALL_FIELDS);
577577
}
578578

579-
if (parsed.anyMatch(plan -> plan instanceof Fork)) {
580-
return result.withFieldNames(IndexResolver.ALL_FIELDS);
581-
}
582-
583579
Holder<Boolean> projectAll = new Holder<>(false);
584580
parsed.forEachExpressionDown(UnresolvedStar.class, us -> {// explicit "*" fields selection
585581
if (projectAll.get()) {
586582
return;
587583
}
588584
projectAll.set(true);
589585
});
586+
587+
if (projectAll.get()) {
588+
return result.withFieldNames(IndexResolver.ALL_FIELDS);
589+
}
590+
591+
Holder<Boolean> projectAfterFork = new Holder<>(false);
592+
Holder<Boolean> hasFork = new Holder<>(false);
593+
594+
parsed.forEachDown(plan -> {
595+
if (projectAll.get()) {
596+
return;
597+
}
598+
599+
if (hasFork.get() == false && shouldCollectReferencedFields(plan)) {
600+
projectAfterFork.set(true);
601+
}
602+
603+
if (plan instanceof Fork fork && projectAfterFork.get() == false) {
604+
hasFork.set(true);
605+
fork.children().forEach(child -> {
606+
if (child.anyMatch(EsqlSession::shouldCollectReferencedFields) == false) {
607+
projectAll.set(true);
608+
}
609+
});
610+
}
611+
});
612+
590613
if (projectAll.get()) {
591614
return result.withFieldNames(IndexResolver.ALL_FIELDS);
592615
}
@@ -703,6 +726,13 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
703726
}
704727
}
705728

729+
/**
730+
* Indicates whether the given plan gives an exact list of fields that we need to collect from field_caps.
731+
*/
732+
private static boolean shouldCollectReferencedFields(LogicalPlan plan) {
733+
return plan instanceof Aggregate || plan instanceof Project;
734+
}
735+
706736
/**
707737
* Could a plan "accidentally" override aliases?
708738
* Examples are JOIN and ENRICH, that _could_ produce fields with the same

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,121 @@ public void testDropWildcardFields_WithLookupJoin() {
18631863
);
18641864
}
18651865

1866+
public void testForkFieldsWithKeepAfterFork() {
1867+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1868+
1869+
assertFieldNames("""
1870+
FROM test
1871+
| WHERE a > 2000
1872+
| EVAL b = a + 100
1873+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
1874+
(WHERE d > 1000 AND e == "aaa" | EVAL c = a + 200)
1875+
| WHERE x > y
1876+
| KEEP a, b, c, d, x
1877+
""", Set.of("a", "a.*", "c", "c.*", "d", "d.*", "e", "e.*", "x", "x.*", "y", "y.*"));
1878+
}
1879+
1880+
public void testForkFieldsWithKeepBeforeFork() {
1881+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1882+
1883+
assertFieldNames("""
1884+
FROM test
1885+
| KEEP a, b, c, d, x
1886+
| WHERE a > 2000
1887+
| EVAL b = a + 100
1888+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
1889+
(WHERE d > 1000 AND e == "aaa" | EVAL c = a + 200)
1890+
| WHERE x > y
1891+
""", Set.of("a", "a.*", "b", "b.*", "c", "c.*", "d", "d.*", "e", "e.*", "x", "x.*", "y", "y.*"));
1892+
}
1893+
1894+
public void testForkFieldsWithNoProjection() {
1895+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1896+
1897+
assertFieldNames("""
1898+
FROM test
1899+
| WHERE a > 2000
1900+
| EVAL b = a + 100
1901+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
1902+
(WHERE d > 1000 AND e == "aaa" | EVAL c = a + 200)
1903+
| WHERE x > y
1904+
""", ALL_FIELDS);
1905+
}
1906+
1907+
public void testForkFieldsWithStatsInOneBranch() {
1908+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1909+
1910+
assertFieldNames("""
1911+
FROM test
1912+
| WHERE a > 2000
1913+
| EVAL b = a + 100
1914+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
1915+
(STATS x = count(*), y=min(z))
1916+
| WHERE x > y
1917+
""", ALL_FIELDS);
1918+
}
1919+
1920+
public void testForkFieldsWithEnrichAndLookupJoins() {
1921+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1922+
assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
1923+
1924+
assertFieldNames(
1925+
"""
1926+
FROM test
1927+
| KEEP a, b, abc, def, z, xyz
1928+
| ENRICH enrich_policy ON abc
1929+
| EVAL b = a + 100
1930+
| LOOKUP JOIN my_lookup_index ON def
1931+
| FORK (WHERE c > 1 AND a < 10000 | EVAL d = a + 500)
1932+
(STATS x = count(*), y=min(z))
1933+
| LOOKUP JOIN my_lookup_index ON xyz
1934+
| WHERE x > y OR _fork == "fork1"
1935+
""",
1936+
Set.of(
1937+
"x",
1938+
"y",
1939+
"_fork",
1940+
"a",
1941+
"c",
1942+
"abc",
1943+
"b",
1944+
"def",
1945+
"z",
1946+
"xyz",
1947+
"def.*",
1948+
"_fork.*",
1949+
"y.*",
1950+
"x.*",
1951+
"xyz.*",
1952+
"z.*",
1953+
"abc.*",
1954+
"a.*",
1955+
"c.*",
1956+
"b.*"
1957+
)
1958+
);
1959+
}
1960+
1961+
public void testForkWithStatsInAllBranches() {
1962+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1963+
1964+
assertFieldNames("""
1965+
FROM test
1966+
| WHERE a > 2000
1967+
| EVAL b = a + 100
1968+
| FORK (WHERE c > 1 AND a < 10000 | STATS m = count(*))
1969+
(EVAL z = a * b | STATS m = max(z))
1970+
(STATS x = count(*), y=min(z))
1971+
| WHERE x > y
1972+
""", Set.of("a", "a.*", "b", "b.*", "c", "c.*", "z", "z.*"));
1973+
}
1974+
1975+
public void testForkWithStatsAndWhere() {
1976+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1977+
1978+
assertFieldNames(" FROM employees | FORK ( WHERE true | stats min(salary) by gender) ( WHERE true | LIMIT 3 )", ALL_FIELDS);
1979+
}
1980+
18661981
private Set<String> fieldNames(String query, Set<String> enrichPolicyMatchFields) {
18671982
var preAnalysisResult = new EsqlSession.PreAnalysisResult(null);
18681983
return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames();

0 commit comments

Comments
 (0)