Skip to content

Commit 93ff611

Browse files
astefankanoshiouelasticsearchmachine
authored
ESQL: Keep DROP attributes when resolving field names (elastic#127009) (elastic#130456)
* ESQL: Keep `DROP` attributes when resolving field names (elastic#127009) (elastic#128147) (cherry picked from commit 54f2668) Co-authored-by: kanoshiou <73424326+kanoshiou@users.noreply.github.com> * [CI] Auto commit changes from spotless --------- Co-authored-by: kanoshiou <73424326+kanoshiou@users.noreply.github.com> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
1 parent 2f196bc commit 93ff611

File tree

7 files changed

+107
-14
lines changed

7 files changed

+107
-14
lines changed

docs/changelog/127009.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 127009
2+
summary: "ESQL: Keep `DROP` attributes when resolving field names"
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 126418

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
5050
"token recognition error at: '``", // https://github.com/elastic/elasticsearch/issues/125870
5151
"Unknown column \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126026
5252
"optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/116781
53-
"No matches found for pattern", // https://github.com/elastic/elasticsearch/issues/126418
5453
"The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework
5554
);
5655

x-pack/plugin/esql/qa/testFixtures/src/main/resources/drop.csv-spec

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,3 +172,19 @@ Milky Way
172172
Milky Way
173173
Milky Way
174174
;
175+
176+
dropAgainWithWildcardAfterEval
177+
required_capability: drop_again_with_wildcard_after_eval
178+
from languages
179+
| eval language_code = 12, x = 13
180+
| drop language_code
181+
| drop language*
182+
| keep x
183+
| limit 3
184+
;
185+
186+
x:integer
187+
13
188+
13
189+
13
190+
;

x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,29 @@ Spanish
15671567
German
15681568
;
15691569

1570+
dropAgainWithWildcardAfterEval2
1571+
required_capability: join_lookup_v12
1572+
required_capability: drop_again_with_wildcard_after_eval
1573+
from addresses,cartesian_multipolygons,hosts
1574+
| rename city.name as message
1575+
| lookup join message_types_lookup on message
1576+
| eval card = -6013949614291505456, hOntTwnVC = null, PQAF = null, DXkxCFXyw = null, number = -7336429038807752405
1577+
| eval dewAwHC = -1186293612, message = null
1578+
| sort number ASC, street ASC, ip0 DESC, name ASC NULLS FIRST, host ASC
1579+
| drop number, host_group, *umber, `city.country.continent.name`, dewAwHC, `zip_code`, `message`, city.country.continent.planet.name, `name`, `ip1`, message, zip_code
1580+
| drop description, *e, id
1581+
| keep `hOntTwnVC`, city.country.continent.planet.galaxy, street
1582+
| limit 5
1583+
;
1584+
1585+
hOntTwnVC:null | city.country.continent.planet.galaxy:keyword | street:keyword
1586+
null | Milky Way | Kearny St
1587+
null | Milky Way | Keizersgracht
1588+
null | Milky Way | Marunouchi
1589+
null | null | null
1590+
null | null | null
1591+
;
1592+
15701593
###############################################
15711594
# LOOKUP JOIN on date_nanos field
15721595
###############################################

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,13 @@ public enum Cap {
734734
* During resolution (pre-analysis) we have to consider that joins or enriches can override EVALuated values
735735
* https://github.com/elastic/elasticsearch/issues/126419
736736
*/
737-
FIX_JOIN_MASKING_EVAL;
737+
FIX_JOIN_MASKING_EVAL,
738+
739+
/**
740+
* Support for keeping `DROP` attributes when resolving field names.
741+
* see <a href="https://github.com/elastic/elasticsearch/issues/126418"> ES|QL: no matches for pattern #126418 </a>
742+
*/
743+
DROP_AGAIN_WITH_WILDCARD_AFTER_EVAL;
738744

739745
private final boolean enabled;
740746

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,16 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
576576
}
577577

578578
AttributeSet references = new AttributeSet();
579-
// "keep" attributes are special whenever a wildcard is used in their name
579+
// "keep" and "drop" attributes are special whenever a wildcard is used in their name, as the wildcard can shadow some
580+
// attributes ("lookup join" generated columns among others) and steps like removal of Aliases should ignore the fields
581+
// to remove if their name matches one of these wildcards.
582+
//
580583
// ie "from test | eval lang = languages + 1 | keep *l" should consider both "languages" and "*l" as valid fields to ask for
581-
AttributeSet keepCommandReferences = new AttributeSet();
582-
AttributeSet keepJoinReferences = new AttributeSet();
584+
// "from test | eval first_name = 1 | drop first_name | drop *name should also consider "*name" as valid field to ask for
585+
//
586+
// NOTE: the grammar allows wildcards to be used in other commands as well, but these are forbidden in the LogicalPlanBuilder
587+
var shadowingRefs = new AttributeSet();
588+
var keepJoinRefs = new AttributeSet();
583589
Set<String> wildcardJoinIndices = new java.util.HashSet<>();
584590

585591
boolean[] canRemoveAliases = new boolean[] { true };
@@ -601,14 +607,14 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
601607
references.addAll(enrichRefs);
602608
} else if (p instanceof LookupJoin join) {
603609
if (join.config().type() instanceof JoinTypes.UsingJoinType usingJoinType) {
604-
keepJoinReferences.addAll(usingJoinType.columns());
610+
keepJoinRefs.addAll(usingJoinType.columns());
605611
}
606-
if (keepCommandReferences.isEmpty()) {
612+
if (shadowingRefs.isEmpty()) {
607613
// No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution
608614
wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern());
609615
} else {
610616
// Keep commands can reference the join columns with names that shadow aliases, so we block their removal
611-
keepJoinReferences.addAll(keepCommandReferences);
617+
keepJoinRefs.addAll(shadowingRefs);
612618
}
613619
} else {
614620
references.addAll(p.references());
@@ -620,12 +626,10 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
620626
p.forEachExpression(UnresolvedNamePattern.class, up -> {
621627
var ua = new UnresolvedAttribute(up.source(), up.name());
622628
references.add(ua);
623-
if (p instanceof Keep) {
624-
keepCommandReferences.add(ua);
625-
}
629+
shadowingRefs.add(ua);
626630
});
627631
if (p instanceof Keep) {
628-
keepCommandReferences.addAll(p.references());
632+
shadowingRefs.addAll(p.references());
629633
}
630634
}
631635

@@ -655,13 +659,13 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
655659
if (fieldNames.contains(alias.name())) {
656660
return;
657661
}
658-
references.removeIf(attr -> matchByName(attr, alias.name(), keepCommandReferences.contains(attr)));
662+
references.removeIf(attr -> matchByName(attr, alias.name(), shadowingRefs.contains(attr)));
659663
});
660664
}
661665
});
662666

663667
// Add JOIN ON column references afterward to avoid Alias removal
664-
references.addAll(keepJoinReferences);
668+
references.addAll(keepJoinRefs);
665669
// If any JOIN commands need wildcard field-caps calls, persist the index names
666670
if (wildcardJoinIndices.isEmpty() == false) {
667671
result = result.withWildcardJoinIndices(wildcardJoinIndices);

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,45 @@ public void testEnrichAndJoinMaskingEvalWh() {
16321632
| keep emp_no, language_name""", Set.of("emp_no", "language_name", "languages", "language_name.*", "languages.*", "emp_no.*"));
16331633
}
16341634

1635+
public void testDropAgainWithWildcardAfterEval() {
1636+
assertFieldNames("""
1637+
from employees
1638+
| eval full_name = 12
1639+
| drop full_name
1640+
| drop *name
1641+
| keep emp_no
1642+
""", Set.of("emp_no", "emp_no.*", "*name", "*name.*"));
1643+
}
1644+
1645+
public void testDropWildcardedFields_AfterRename() {
1646+
assertFieldNames(
1647+
"""
1648+
from employees
1649+
| rename first_name AS first_names, last_name AS last_names
1650+
| eval first_names = 1
1651+
| drop first_names
1652+
| drop *_names
1653+
| keep gender""",
1654+
Set.of("first_name", "first_name.*", "last_name", "last_name.*", "*_names", "*_names.*", "gender", "gender.*")
1655+
);
1656+
}
1657+
1658+
public void testDropWildcardFields_WithLookupJoin() {
1659+
assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
1660+
assertFieldNames(
1661+
"""
1662+
FROM sample_data
1663+
| EVAL client_ip = client_ip::keyword
1664+
| LOOKUP JOIN clientips_lookup ON client_ip
1665+
| LOOKUP JOIN message_types_lookup ON message
1666+
| KEEP @timestamp, message, *e*
1667+
| SORT @timestamp
1668+
| DROP *e""",
1669+
Set.of("client_ip", "client_ip.*", "message", "message.*", "@timestamp", "@timestamp.*", "*e*", "*e", "*e.*"),
1670+
Set.of()
1671+
);
1672+
}
1673+
16351674
private Set<String> fieldNames(String query, Set<String> enrichPolicyMatchFields) {
16361675
var preAnalysisResult = new EsqlSession.PreAnalysisResult(null);
16371676
return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames();

0 commit comments

Comments
 (0)