Skip to content

Commit f64d249

Browse files
committed
Fix wildcard drop after lookup join
In EsqlSession#fieldNames, keep track of wildcard patterns in DROP separately from the references and wildcard patterns in KEEP commands. If the latter are present after a LOOKUP JOIN, we'll ask fieldcaps for specific fields of the lookup index, whereas `DROP somefield*` should not trigger asking for specific fields by itself - if there's no downstream KEEP, we still need to ask for `*`.
1 parent c41922f commit f64d249

File tree

1 file changed

+26
-14
lines changed
  • x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session

1 file changed

+26
-14
lines changed

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

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -627,16 +627,20 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
627627
}
628628

629629
var referencesBuilder = AttributeSet.builder();
630-
// "keep" and "drop" attributes are special whenever a wildcard is used in their name, as the wildcard can shadow some
631-
// attributes ("lookup join" generated columns among others) and steps like removal of Aliases should ignore the fields
632-
// to remove if their name matches one of these wildcards.
630+
// "keep" and "drop" attributes are special whenever a wildcard is used in their name, as the wildcard can cover some
631+
// attributes ("lookup join" generated columns among others); steps like removal of Aliases should ignore fields matching the
632+
// wildcards.
633633
//
634-
// ie "from test | eval lang = languages + 1 | keep *l" should consider both "languages" and "*l" as valid fields to ask for
635-
// "from test | eval first_name = 1 | drop first_name | drop *name should also consider "*name" as valid field to ask for
634+
// E.g. "from test | eval lang = languages + 1 | keep *l" should consider both "languages" and "*l" as valid fields to ask for
635+
// "from test | eval first_name = 1 | drop first_name | drop *name" should also consider "*name" as valid field to ask for
636636
//
637637
// NOTE: the grammar allows wildcards to be used in other commands as well, but these are forbidden in the LogicalPlanBuilder
638-
var shadowingRefsBuilder = AttributeSet.builder();
639-
var keepJoinRefsBuilder = AttributeSet.builder();
638+
// Except in KEEP and DROP.
639+
var keepRefs = AttributeSet.builder();
640+
var dropWildcardRefs = AttributeSet.builder();
641+
// fields required to request for lookup joins to work
642+
var joinRefs = AttributeSet.builder();
643+
// lookup indices where we request "*" because we may require all their fields
640644
Set<String> wildcardJoinIndices = new java.util.HashSet<>();
641645

642646
boolean[] canRemoveAliases = new boolean[] { true };
@@ -654,14 +658,14 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
654658
referencesBuilder.addAll(enrichRefs);
655659
} else if (p instanceof LookupJoin join) {
656660
if (join.config().type() instanceof JoinTypes.UsingJoinType usingJoinType) {
657-
keepJoinRefsBuilder.addAll(usingJoinType.columns());
661+
joinRefs.addAll(usingJoinType.columns());
658662
}
659-
if (shadowingRefsBuilder.isEmpty()) {
663+
if (keepRefs.isEmpty()) {
660664
// No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution
661665
wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern());
662666
} else {
663667
// Keep commands can reference the join columns with names that shadow aliases, so we block their removal
664-
keepJoinRefsBuilder.addAll(shadowingRefsBuilder);
668+
joinRefs.addAll(keepRefs);
665669
}
666670
} else {
667671
referencesBuilder.addAll(p.references());
@@ -673,10 +677,16 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
673677
p.forEachExpression(UnresolvedNamePattern.class, up -> {
674678
var ua = new UnresolvedAttribute(up.source(), up.name());
675679
referencesBuilder.add(ua);
676-
shadowingRefsBuilder.add(ua);
680+
if (p instanceof Keep) {
681+
keepRefs.add(ua);
682+
} else if (p instanceof Drop) {
683+
dropWildcardRefs.add(ua);
684+
} else {
685+
throw new IllegalStateException("Only KEEP and DROP should allow wildcards");
686+
}
677687
});
678688
if (p instanceof Keep) {
679-
shadowingRefsBuilder.addAll(p.references());
689+
keepRefs.addAll(p.references());
680690
}
681691
}
682692

@@ -710,13 +720,15 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
710720
if (fieldNames.contains(ne.name())) {
711721
return;
712722
}
713-
referencesBuilder.removeIf(attr -> matchByName(attr, ne.name(), shadowingRefsBuilder.contains(attr)));
723+
referencesBuilder.removeIf(
724+
attr -> matchByName(attr, ne.name(), keepRefs.contains(attr) || dropWildcardRefs.contains(attr))
725+
);
714726
});
715727
}
716728
});
717729

718730
// Add JOIN ON column references afterward to avoid Alias removal
719-
referencesBuilder.addAll(keepJoinRefsBuilder);
731+
referencesBuilder.addAll(joinRefs);
720732
// If any JOIN commands need wildcard field-caps calls, persist the index names
721733
if (wildcardJoinIndices.isEmpty() == false) {
722734
result = result.withWildcardJoinIndices(wildcardJoinIndices);

0 commit comments

Comments
 (0)