Skip to content

Commit af9c70d

Browse files
committed
Slightly adjust the solution and add a couple of tests
1 parent 108b133 commit af9c70d

File tree

2 files changed

+41
-9
lines changed

2 files changed

+41
-9
lines changed

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -590,9 +590,15 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
590590
}
591591

592592
var referencesBuilder = AttributeSet.builder();
593-
// "keep" attributes are special whenever a wildcard is used in their name
593+
// "keep" and "drop" attributes are special whenever a wildcard is used in their name, as the wildcard can shadow some
594+
// attributes ("lookup join" generated columns among others) and steps like removal of Aliases should ignore the fields
595+
// to remove if their name matches one of these wildcards.
596+
//
594597
// ie "from test | eval lang = languages + 1 | keep *l" should consider both "languages" and "*l" as valid fields to ask for
595-
var keepCommandRefsBuilder = AttributeSet.builder();
598+
// "from test | eval first_name = 1 | drop first_name | drop *name should also consider "*name" as valid field to ask for
599+
//
600+
// NOTE: the grammar allows wildcards to be used in other commands as well, but these are forbidden in the LogicalPlanBuilder
601+
var shadowingRefsBuilder = AttributeSet.builder();
596602
var keepJoinRefsBuilder = AttributeSet.builder();
597603
Set<String> wildcardJoinIndices = new java.util.HashSet<>();
598604

@@ -617,12 +623,12 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
617623
if (join.config().type() instanceof JoinTypes.UsingJoinType usingJoinType) {
618624
keepJoinRefsBuilder.addAll(usingJoinType.columns());
619625
}
620-
if (keepCommandRefsBuilder.isEmpty()) {
626+
if (shadowingRefsBuilder.isEmpty()) {
621627
// No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution
622628
wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern());
623629
} else {
624630
// Keep commands can reference the join columns with names that shadow aliases, so we block their removal
625-
keepJoinRefsBuilder.addAll(keepCommandRefsBuilder);
631+
keepJoinRefsBuilder.addAll(shadowingRefsBuilder);
626632
}
627633
} else {
628634
referencesBuilder.addAll(p.references());
@@ -634,12 +640,10 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
634640
p.forEachExpression(UnresolvedNamePattern.class, up -> {
635641
var ua = new UnresolvedAttribute(up.source(), up.name());
636642
referencesBuilder.add(ua);
637-
if (p instanceof Keep || p instanceof Drop) {
638-
keepCommandRefsBuilder.add(ua);
639-
}
643+
shadowingRefsBuilder.add(ua);
640644
});
641645
if (p instanceof Keep) {
642-
keepCommandRefsBuilder.addAll(p.references());
646+
shadowingRefsBuilder.addAll(p.references());
643647
}
644648
}
645649

@@ -669,7 +673,7 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
669673
if (fieldNames.contains(alias.name())) {
670674
return;
671675
}
672-
referencesBuilder.removeIf(attr -> matchByName(attr, alias.name(), keepCommandRefsBuilder.contains(attr)));
676+
referencesBuilder.removeIf(attr -> matchByName(attr, alias.name(), shadowingRefsBuilder.contains(attr)));
673677
});
674678
}
675679
});

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1720,6 +1720,34 @@ public void testDropAgainWithWildcardAfterEval() {
17201720
""", Set.of("emp_no", "emp_no.*", "*name", "*name.*"));
17211721
}
17221722

1723+
public void testDropWildcardedFields_AfterRename() {
1724+
assertFieldNames(
1725+
"""
1726+
from employees
1727+
| rename first_name AS first_names, last_name AS last_names
1728+
| eval first_names = 1
1729+
| drop first_names
1730+
| drop *_names
1731+
| keep gender""",
1732+
Set.of("first_name", "first_name.*", "last_name", "last_name.*", "*_names", "*_names.*", "gender", "gender.*")
1733+
);
1734+
}
1735+
1736+
public void testDropWildcardFields_WithLookupJoin() {
1737+
assertFieldNames(
1738+
"""
1739+
FROM sample_data
1740+
| EVAL client_ip = client_ip::keyword
1741+
| LOOKUP JOIN clientips_lookup ON client_ip
1742+
| LOOKUP JOIN message_types_lookup ON message
1743+
| KEEP @timestamp, message, *e*
1744+
| SORT @timestamp
1745+
| DROP *e""",
1746+
Set.of("client_ip", "client_ip.*", "message", "message.*", "@timestamp", "@timestamp.*", "*e*", "*e", "*e.*"),
1747+
Set.of()
1748+
);
1749+
}
1750+
17231751
private Set<String> fieldNames(String query, Set<String> enrichPolicyMatchFields) {
17241752
var preAnalysisResult = new EsqlSession.PreAnalysisResult(null);
17251753
return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames();

0 commit comments

Comments
 (0)