Skip to content

Commit 619d753

Browse files
kanoshiouastefan
authored andcommitted
ESQL: Keep DROP attributes when resolving field names (elastic#127009)
(cherry picked from commit 54f2668)
1 parent 2c6e3a4 commit 619d753

File tree

7 files changed

+126
-11
lines changed

7 files changed

+126
-11
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: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,3 +1569,48 @@ French
15691569
Spanish
15701570
German
15711571
;
1572+
1573+
1574+
pruningJoinResult
1575+
required_capability: join_lookup_v12
1576+
required_capability: fix_join_masking_eval
1577+
from sample_data
1578+
| eval type = 1000
1579+
| lookup join message_types_lookup on message
1580+
| grok type "%{WORD:foo}"
1581+
| keep event_duration
1582+
;
1583+
1584+
ignoreOrder: true
1585+
event_duration:long
1586+
1756467
1587+
5033755
1588+
8268153
1589+
725448
1590+
1232382
1591+
2764889
1592+
3450233
1593+
;
1594+
1595+
dropAgainWithWildcardAfterEval2
1596+
required_capability: join_lookup_v12
1597+
required_capability: drop_again_with_wildcard_after_eval
1598+
from addresses,cartesian_multipolygons,hosts
1599+
| rename city.name as message
1600+
| lookup join message_types_lookup on message
1601+
| eval card = -6013949614291505456, hOntTwnVC = null, PQAF = null, DXkxCFXyw = null, number = -7336429038807752405
1602+
| eval dewAwHC = -1186293612, message = null
1603+
| sort number ASC, street ASC, ip0 DESC, name ASC NULLS FIRST, host ASC
1604+
| drop number, host_group, *umber, `city.country.continent.name`, dewAwHC, `zip_code`, `message`, city.country.continent.planet.name, `name`, `ip1`, message, zip_code
1605+
| drop description, *e, id
1606+
| keep `hOntTwnVC`, city.country.continent.planet.galaxy, street
1607+
| limit 5
1608+
;
1609+
1610+
hOntTwnVC:null | city.country.continent.planet.galaxy:keyword | street:keyword
1611+
null | Milky Way | Kearny St
1612+
null | Milky Way | Keizersgracht
1613+
null | Milky Way | Marunouchi
1614+
null | null | null
1615+
null | null | null
1616+
;

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
@@ -889,7 +889,13 @@ public enum Cap {
889889
* During resolution (pre-analysis) we have to consider that joins or enriches can override EVALuated values
890890
* https://github.com/elastic/elasticsearch/issues/126419
891891
*/
892-
FIX_JOIN_MASKING_EVAL;
892+
FIX_JOIN_MASKING_EVAL,
893+
894+
/**
895+
* Support for keeping `DROP` attributes when resolving field names.
896+
* see <a href="https://github.com/elastic/elasticsearch/issues/126418"> ES|QL: no matches for pattern #126418 </a>
897+
*/
898+
DROP_AGAIN_WITH_WILDCARD_AFTER_EVAL;
893899

894900
private final boolean enabled;
895901

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
@@ -578,9 +578,15 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
578578
}
579579

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

@@ -605,12 +611,12 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
605611
if (join.config().type() instanceof JoinTypes.UsingJoinType usingJoinType) {
606612
keepJoinRefsBuilder.addAll(usingJoinType.columns());
607613
}
608-
if (keepCommandRefsBuilder.isEmpty()) {
614+
if (shadowingRefsBuilder.isEmpty()) {
609615
// No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution
610616
wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern());
611617
} else {
612618
// Keep commands can reference the join columns with names that shadow aliases, so we block their removal
613-
keepJoinRefsBuilder.addAll(keepCommandRefsBuilder);
619+
keepJoinRefsBuilder.addAll(shadowingRefsBuilder);
614620
}
615621
} else {
616622
referencesBuilder.addAll(p.references());
@@ -622,12 +628,10 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
622628
p.forEachExpression(UnresolvedNamePattern.class, up -> {
623629
var ua = new UnresolvedAttribute(up.source(), up.name());
624630
referencesBuilder.add(ua);
625-
if (p instanceof Keep) {
626-
keepCommandRefsBuilder.add(ua);
627-
}
631+
shadowingRefsBuilder.add(ua);
628632
});
629633
if (p instanceof Keep) {
630-
keepCommandRefsBuilder.addAll(p.references());
634+
shadowingRefsBuilder.addAll(p.references());
631635
}
632636
}
633637

@@ -657,7 +661,7 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set<String> enrichPolicy
657661
if (fieldNames.contains(alias.name())) {
658662
return;
659663
}
660-
referencesBuilder.removeIf(attr -> matchByName(attr, alias.name(), keepCommandRefsBuilder.contains(attr)));
664+
referencesBuilder.removeIf(attr -> matchByName(attr, alias.name(), shadowingRefsBuilder.contains(attr)));
661665
});
662666
}
663667
});

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
@@ -1710,6 +1710,45 @@ public void testEnrichAndJoinMaskingEvalWh() {
17101710
| keep emp_no, language_name""", Set.of("emp_no", "language_name", "languages", "language_name.*", "languages.*", "emp_no.*"));
17111711
}
17121712

1713+
public void testDropAgainWithWildcardAfterEval() {
1714+
assertFieldNames("""
1715+
from employees
1716+
| eval full_name = 12
1717+
| drop full_name
1718+
| drop *name
1719+
| keep emp_no
1720+
""", Set.of("emp_no", "emp_no.*", "*name", "*name.*"));
1721+
}
1722+
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+
assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V12.isEnabled());
1738+
assertFieldNames(
1739+
"""
1740+
FROM sample_data
1741+
| EVAL client_ip = client_ip::keyword
1742+
| LOOKUP JOIN clientips_lookup ON client_ip
1743+
| LOOKUP JOIN message_types_lookup ON message
1744+
| KEEP @timestamp, message, *e*
1745+
| SORT @timestamp
1746+
| DROP *e""",
1747+
Set.of("client_ip", "client_ip.*", "message", "message.*", "@timestamp", "@timestamp.*", "*e*", "*e", "*e.*"),
1748+
Set.of()
1749+
);
1750+
}
1751+
17131752
private Set<String> fieldNames(String query, Set<String> enrichPolicyMatchFields) {
17141753
var preAnalysisResult = new EsqlSession.PreAnalysisResult(null);
17151754
return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames();

0 commit comments

Comments
 (0)