Skip to content

Commit 34c883e

Browse files
committed
Add test and apply refactor suggestion
1 parent 4d23bc0 commit 34c883e

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ 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
}
@@ -592,14 +592,18 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
592592
Holder<Boolean> hasFork = new Holder<>(false);
593593

594594
parsed.forEachDown(plan -> {
595-
if (hasFork.get() == false && (plan instanceof Project || plan instanceof Aggregate)) {
595+
if (projectAll.get()) {
596+
return;
597+
}
598+
599+
if (hasFork.get() == false && shouldCollectReferencedFields(plan)) {
596600
projectAfterFork.set(true);
597601
}
598602

599603
if (plan instanceof Fork fork && projectAfterFork.get() == false) {
600604
hasFork.set(true);
601605
fork.children().forEach(child -> {
602-
if (child.anyMatch(p -> p instanceof Project || p instanceof Aggregate) == false) {
606+
if (child.anyMatch(EsqlSession::shouldCollectReferencedFields) == false) {
603607
projectAll.set(true);
604608
}
605609
});
@@ -722,6 +726,13 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
722726
}
723727
}
724728

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+
725736
/**
726737
* Could a plan "accidentally" override aliases?
727738
* 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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,6 +1905,12 @@ public void testForkWithStatsInAllBranches() {
19051905
""", Set.of("a", "a.*", "b", "b.*", "c", "c.*", "z", "z.*"));
19061906
}
19071907

1908+
public void testForkWithStatsAndWhere() {
1909+
assumeTrue("FORK available as snapshot only", EsqlCapabilities.Cap.FORK.isEnabled());
1910+
1911+
assertFieldNames(" FROM employees | FORK ( WHERE true | stats min(salary) by gender) ( WHERE true | LIMIT 3 )", ALL_FIELDS);
1912+
}
1913+
19081914
private Set<String> fieldNames(String query, Set<String> enrichPolicyMatchFields) {
19091915
var preAnalysisResult = new EsqlSession.PreAnalysisResult(null);
19101916
return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames();

0 commit comments

Comments
 (0)