Skip to content

Commit 1315169

Browse files
committed
Merge branch 'main' into simplify_testSearchWhileRelocating
2 parents ad6a9e9 + 54f2668 commit 1315169

File tree

11 files changed

+123
-20
lines changed

11 files changed

+123
-20
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

docs/changelog/127991.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 127991
2+
summary: Avoid nested docs in painless execute api
3+
area: Infra/Scripting
4+
type: bug
5+
issues:
6+
- 41004

modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,8 @@ private static Response prepareRamIndex(
831831
// This is a problem especially for indices that have no mappings, as no fields will be accessible, neither through doc
832832
// nor _source (if there are no mappings there are no metadata fields).
833833
ParsedDocument parsedDocument = documentMapper.parse(sourceToParse);
834-
indexWriter.addDocuments(parsedDocument.docs());
834+
// only index the root doc since nested docs are not supported in painless anyways
835+
indexWriter.addDocuments(List.of(parsedDocument.rootDoc()));
835836
try (IndexReader indexReader = DirectoryReader.open(indexWriter)) {
836837
final IndexSearcher searcher = new IndexSearcher(indexReader);
837838
searcher.setQueryCache(null);

modules/lang-painless/src/test/java/org/elasticsearch/painless/action/PainlessExecuteApiTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,18 @@ public void testDefaults() throws IOException {
7070
assertThat(e.getCause().getMessage(), equalTo("cannot resolve symbol [doc]"));
7171
}
7272

73+
public void testNestedDocs() throws IOException {
74+
ScriptService scriptService = getInstanceFromNode(ScriptService.class);
75+
IndexService indexService = createIndex("index", Settings.EMPTY, "doc", "rank", "type=long", "nested", "type=nested");
76+
77+
Request.ContextSetup contextSetup = new Request.ContextSetup("index", new BytesArray("""
78+
{"rank": 4.0, "nested": [{"text": "foo"}, {"text": "bar"}]}"""), new MatchAllQueryBuilder());
79+
contextSetup.setXContentType(XContentType.JSON);
80+
Request request = new Request(new Script(ScriptType.INLINE, "painless", "doc['rank'].value", Map.of()), "score", contextSetup);
81+
Response response = innerShardOperation(request, scriptService, indexService);
82+
assertThat(response.getResult(), equalTo(4.0D));
83+
}
84+
7385
public void testFilterExecutionContext() throws IOException {
7486
ScriptService scriptService = getInstanceFromNode(ScriptService.class);
7587
IndexService indexService = createIndex("index", Settings.EMPTY, "doc", "field", "type=long");

muted-tests.yml

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -447,15 +447,6 @@ tests:
447447
- class: org.elasticsearch.indices.stats.IndexStatsIT
448448
method: testThrottleStats
449449
issue: https://github.com/elastic/elasticsearch/issues/126359
450-
- class: org.elasticsearch.search.vectors.IVFKnnFloatVectorQueryTests
451-
method: testRandomWithFilter
452-
issue: https://github.com/elastic/elasticsearch/issues/127963
453-
- class: org.elasticsearch.search.vectors.IVFKnnFloatVectorQueryTests
454-
method: testSearchBoost
455-
issue: https://github.com/elastic/elasticsearch/issues/127969
456-
- class: org.elasticsearch.search.vectors.IVFKnnFloatVectorQueryTests
457-
method: testFindFewer
458-
issue: https://github.com/elastic/elasticsearch/issues/128002
459450
- class: org.elasticsearch.xpack.remotecluster.RemoteClusterSecurityRestIT
460451
method: testTaskCancellation
461452
issue: https://github.com/elastic/elasticsearch/issues/128009

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
@@ -51,7 +51,6 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
5151
"Unknown column \\[<all-fields-projected>\\]", // https://github.com/elastic/elasticsearch/issues/121741,
5252
"Plan \\[ProjectExec\\[\\[<no-fields>.* optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/125866
5353
"optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/116781
54-
"No matches found for pattern", // https://github.com/elastic/elasticsearch/issues/126418
5554
"Unknown column", // https://github.com/elastic/elasticsearch/issues/127467
5655
"only supports KEYWORD or TEXT values", // https://github.com/elastic/elasticsearch/issues/127468
5756
"The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework

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
@@ -1650,3 +1650,26 @@ event_duration:long
16501650
2764889
16511651
3450233
16521652
;
1653+
1654+
dropAgainWithWildcardAfterEval2
1655+
required_capability: join_lookup_v12
1656+
required_capability: drop_again_with_wildcard_after_eval
1657+
from addresses,cartesian_multipolygons,hosts
1658+
| rename city.name as message
1659+
| lookup join message_types_lookup on message
1660+
| eval card = -6013949614291505456, hOntTwnVC = null, PQAF = null, DXkxCFXyw = null, number = -7336429038807752405
1661+
| eval dewAwHC = -1186293612, message = null
1662+
| sort number ASC, street ASC, ip0 DESC, name ASC NULLS FIRST, host ASC
1663+
| drop number, host_group, *umber, `city.country.continent.name`, dewAwHC, `zip_code`, `message`, city.country.continent.planet.name, `name`, `ip1`, message, zip_code
1664+
| drop description, *e, id
1665+
| keep `hOntTwnVC`, city.country.continent.planet.galaxy, street
1666+
| limit 5
1667+
;
1668+
1669+
hOntTwnVC:null | city.country.continent.planet.galaxy:keyword | street:keyword
1670+
null | Milky Way | Kearny St
1671+
null | Milky Way | Keizersgracht
1672+
null | Milky Way | Marunouchi
1673+
null | null | null
1674+
null | null | null
1675+
;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,12 @@ public enum Cap {
10361036
*/
10371037
FIX_JOIN_MASKING_EVAL,
10381038

1039+
/**
1040+
* Support for keeping `DROP` attributes when resolving field names.
1041+
* see <a href="https://github.com/elastic/elasticsearch/issues/126418"> ES|QL: no matches for pattern #126418 </a>
1042+
*/
1043+
DROP_AGAIN_WITH_WILDCARD_AFTER_EVAL,
1044+
10391045
/**
10401046
* Support last_over_time aggregation that gets evaluated per time-series
10411047
*/

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) {
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
});

0 commit comments

Comments
 (0)