Skip to content

Commit 1204791

Browse files
GalLaloucheGalLalouche
committed
TEMP
1 parent faef67a commit 1204791

File tree

9 files changed

+109
-65
lines changed

9 files changed

+109
-65
lines changed

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ FROM partial_mapping_sample_data
5555
2024-10-23T12:15:03.360Z | Connected to 10.1.0.3!
5656
;
5757

58+
unmappedFieldAppearsLast
59+
required_capability: unmapped_fields
60+
FROM partial_mapping_sample_data
61+
| INSIST_🐔 event_duration
62+
| SORT @timestamp DESC
63+
| Limit 1
64+
;
65+
66+
@timestamp:date | client_ip:ip | message:keyword | event_duration:long
67+
2024-10-23T13:55:01.543Z | 173.21.3.15 | Connected to 10.1.0.1! | 1756466
68+
;
69+
5870
fieldDoesNotExistSingleIndex
5971
required_capability: unmapped_fields
6072
FROM partial_mapping_sample_data
@@ -131,7 +143,7 @@ FROM partial_mapping_excluded_source_sample_data
131143
#####################
132144

133145
fieldDoesNotExistMultiIndex
134-
required_capability: metadata_fields
146+
required_capability: index_metadata_field
135147
required_capability: unmapped_fields
136148
FROM partial_mapping_sample_data, sample_data METADATA _index
137149
| INSIST_🐔 foo
@@ -157,7 +169,7 @@ sample_data | 2023-10-23T12:15:03.360Z | null
157169
;
158170

159171
fieldIsUnmappedMultiIndex
160-
required_capability: metadata_fields
172+
required_capability: index_metadata_field
161173
required_capability: unmapped_fields
162174
FROM partial_mapping_sample_data, sample_data METADATA _index
163175
| INSIST_🐔 unmapped_message
@@ -184,7 +196,7 @@ FROM partial_mapping_sample_data, sample_data METADATA _index
184196

185197

186198
fieldIsMappedToDifferentTypesNoCastMultiIndex
187-
required_capability: metadata_fields
199+
required_capability: index_metadata_field
188200
required_capability: unmapped_fields
189201
FROM sample_data_ts_long, sample_data METADATA _index
190202
| INSIST_🐔 @timestamp
@@ -210,7 +222,7 @@ sample_data_ts_long | null
210222
;
211223

212224
fieldIsMappedToDifferentTypesButDropped
213-
required_capability: metadata_fields
225+
required_capability: index_metadata_field
214226
required_capability: unmapped_fields
215227
FROM sample_data_ts_long, sample_data METADATA _index
216228
| INSIST_🐔 @timestamp
@@ -238,7 +250,7 @@ sample_data_ts_long | 42
238250
;
239251

240252
fieldIsPartiallyUnmappedMultiIndex
241-
required_capability: metadata_fields
253+
required_capability: index_metadata_field
242254
required_capability: unmapped_fields
243255
FROM sample_data, no_mapping_sample_data METADATA _index
244256
| INSIST_🐔 message
@@ -290,7 +302,7 @@ Connected to 10.1.0.1
290302
;
291303

292304
fieldIsPartiallyUnmappedPartiallySourceIsDisabledMultiIndex
293-
required_capability: metadata_fields
305+
required_capability: index_metadata_field
294306
required_capability: source_field_mapping
295307
required_capability: unmapped_fields
296308
FROM partial_mapping_sample_data,partial_mapping_no_source_sample_data METADATA _index
@@ -317,7 +329,7 @@ partial_mapping_sample_data | 2024-10-23T13:55:01.543Z | Connected to
317329
;
318330

319331
partialMappingStats
320-
required_capability: metadata_fields
332+
required_capability: index_metadata_field
321333
required_capability: source_field_mapping
322334
required_capability: unmapped_fields
323335
FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data METADATA _index
@@ -336,7 +348,7 @@ max(@timestamp):date | count(*):long | message:keyword
336348
;
337349

338350
partialMappingCoalesce
339-
required_capability: metadata_fields
351+
required_capability: index_metadata_field
340352
required_capability: source_field_mapping
341353
required_capability: unmapped_fields
342354
FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data METADATA _index
@@ -365,7 +377,7 @@ FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data MET
365377
;
366378

367379
partialMappingUnionTypes
368-
required_capability: metadata_fields
380+
required_capability: index_metadata_field
369381
required_capability: source_field_mapping
370382
required_capability: unmapped_fields
371383
FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data METADATA _index
@@ -393,7 +405,7 @@ FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data MET
393405
;
394406

395407
partialMappingUnionTypesStats
396-
required_capability: metadata_fields
408+
required_capability: index_metadata_field
397409
required_capability: source_field_mapping
398410
required_capability: unmapped_fields
399411
FROM partial_mapping_sample_data,partial_mapping_excluded_source_sample_data

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@
111111
import java.util.List;
112112
import java.util.Map;
113113
import java.util.Set;
114-
import java.util.function.Consumer;
115114
import java.util.function.Function;
116115
import java.util.function.Predicate;
117116
import java.util.stream.Collectors;
@@ -686,40 +685,43 @@ private List<Attribute> resolveUsingColumns(List<Attribute> cols, List<Attribute
686685
}
687686

688687
private LogicalPlan resolveInsist(Insist insist, List<Attribute> childrenOutput, IndexResolution indexResolution) {
689-
Attribute resolvedCol = maybeResolveAttribute(insist.attribute(), childrenOutput);
688+
return insist.withAttribute(resolveInsistAttribute(insist, childrenOutput, indexResolution));
689+
}
690+
691+
private Attribute resolveInsistAttribute(Insist insist, List<Attribute> childrenOutput, IndexResolution indexResolution) {
692+
assert insist.attribute() instanceof UnresolvedAttribute : "INSIST should be unresolved at this point";
693+
Attribute resolvedCol = maybeResolveAttribute((UnresolvedAttribute) insist.attribute(), childrenOutput);
690694
// Field isn't mapped anywhere.
691695
if (resolvedCol instanceof UnresolvedAttribute) {
692-
return mergeInsist(insist, attrs -> attrs.add(insistKeyword(insist)));
696+
return insistKeyword(insist);
693697
}
694698

699+
assert resolvedCol instanceof FieldAttribute : "Resolved INSIST attribute should resolve to a field attribute";
700+
var field = ((FieldAttribute) resolvedCol).field();
695701
String name = resolvedCol.name();
696702
// Field is partially unmapped.
697-
if (resolvedCol instanceof FieldAttribute f && indexResolution.get().isPartiallyUnmappedField(name)) {
698-
return mergeInsist(insist, attrs -> {
699-
var index = CollectionUtils.findFirstIndex(attrs, e -> e.name().equals(name)).getAsInt();
700-
Attribute attribute = f.field().getDataType() == KEYWORD ? insistKeyword(insist) : invalidInsistAttribute(insist, f);
701-
attrs.set(index, attribute);
702-
});
703+
if (indexResolution.get().isPartiallyUnmappedField(name)) {
704+
return field.getDataType() == KEYWORD
705+
? insistKeyword(insist)
706+
: invalidInsistAttribute(insist, (FieldAttribute) resolvedCol);
703707
}
704708

705709
// Field is mapped everywhere; we can safely ignore the INSIST command.
706-
return insist.child();
710+
return resolvedCol;
707711
}
708712

709-
private static EsRelation mergeInsist(Insist insist, Consumer<List<Attribute>> updateAttributes) {
713+
private static EsRelation mergeInsist(Insist insist, Function<List<Attribute>, List<Attribute>> updateAttributes) {
710714
assert insist.child() instanceof EsRelation : "INSIST should be on top of a relation (see LogicalPlanBuilder)";
711715
var relation = (EsRelation) insist.child();
712-
List<Attribute> newOutput = new ArrayList<>(relation.output());
713-
updateAttributes.accept(newOutput);
716+
var newOutput = updateAttributes.apply(relation.output());
714717
return relation.withAttributes(newOutput);
715718
}
716719

717-
private static UnsupportedAttribute invalidInsistAttribute(Insist insist, FieldAttribute fa) {
720+
private static FieldAttribute invalidInsistAttribute(Insist insist, FieldAttribute fa) {
718721
String name = fa.name();
719722
var messageFormat = "Cannot use field [%s] due to ambiguities caused by INSIST. "
720723
+ "Unmapped fields are treated as KEYWORD in unmapped indices, but field is mapped to another type";
721-
var field = new UnsupportedEsField(name, fa.field().getDataType().typeName());
722-
return new UnsupportedAttribute(insist.source(), name, field, Strings.format(messageFormat, name));
724+
return new FieldAttribute(insist.source(), name, new InvalidMappedField(name, Strings.format(messageFormat, name)));
723725
}
724726

725727
private static FieldAttribute insistKeyword(Insist insist) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.EsqlBinaryComparison;
3030
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
3131
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
32+
import org.elasticsearch.xpack.esql.plan.logical.Insist;
3233
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
3334
import org.elasticsearch.xpack.esql.plan.logical.Lookup;
3435
import org.elasticsearch.xpack.esql.plan.logical.Project;
@@ -132,7 +133,7 @@ else if (p.resolved()) {
132133

133134
e.forEachUp(ae -> {
134135
// Special handling for Project and unsupported/union types: disallow renaming them but pass them through otherwise.
135-
if (p instanceof Project) {
136+
if (p instanceof Project || p instanceof Insist) {
136137
if (ae instanceof Alias as && as.child() instanceof UnsupportedAttribute ua) {
137138
failures.add(fail(ae, ua.unresolvedMessage()));
138139
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/EsIndex.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.index.IndexMode;
1414
import org.elasticsearch.xpack.esql.core.type.EsField;
1515

16-
import java.beans.Transient;
1716
import java.io.IOException;
1817
import java.util.Map;
1918
import java.util.Set;
@@ -25,7 +24,7 @@ public record EsIndex(
2524
Map<String, EsField> mapping,
2625
Map<String, IndexMode> indexNameWithModes,
2726
/** Fields mapped only in some (but *not* all) indices. Since this is only used by the analyzer, it is not serialized. */
28-
@Transient Set<String> partiallyUnmappedFields
27+
Set<String> partiallyUnmappedFields
2928
) implements Writeable {
3029

3130
public EsIndex {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.xpack.esql.optimizer.rules.logical.ConstantFolding;
2121
import org.elasticsearch.xpack.esql.optimizer.rules.logical.ConvertStringToByteRef;
2222
import org.elasticsearch.xpack.esql.optimizer.rules.logical.ExtractAggregateCommonFilter;
23+
import org.elasticsearch.xpack.esql.optimizer.rules.logical.FoldInsist;
2324
import org.elasticsearch.xpack.esql.optimizer.rules.logical.FoldNull;
2425
import org.elasticsearch.xpack.esql.optimizer.rules.logical.LiteralsOnTheRight;
2526
import org.elasticsearch.xpack.esql.optimizer.rules.logical.PartiallyFoldCase;
@@ -155,6 +156,7 @@ protected static Batch<LogicalPlan> substitutions() {
155156
protected static Batch<LogicalPlan> operators() {
156157
return new Batch<>(
157158
"Operator Optimization",
159+
new FoldInsist(),
158160
new CombineProjections(),
159161
new CombineEvals(),
160162
new PruneEmptyPlans(),

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/FoldInsist.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.optimizer.rules.logical;
9+
10+
import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
11+
import org.elasticsearch.xpack.esql.plan.logical.Insist;
12+
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
13+
14+
// FIXME(gal, do-not-merge!) document
15+
public final class FoldInsist extends OptimizerRules.OptimizerRule<Insist> {
16+
public FoldInsist() {
17+
super(OptimizerRules.TransformDirection.UP);
18+
}
19+
20+
@Override
21+
protected LogicalPlan rule(Insist insist) {
22+
assert insist.child() instanceof EsRelation : "INSIST should be on top of a relation (see LogicalPlanBuilder)";
23+
var relation = (EsRelation) insist.child();
24+
return relation.withAttributes(insist.output());
25+
}
26+
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Insist.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,19 @@
99
import org.elasticsearch.common.io.stream.StreamOutput;
1010
import org.elasticsearch.core.Nullable;
1111
import org.elasticsearch.xpack.esql.core.expression.Attribute;
12-
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
1312
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1413
import org.elasticsearch.xpack.esql.core.tree.Source;
14+
import org.elasticsearch.xpack.esql.expression.NamedExpressions;
15+
import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
1516

1617
import java.io.IOException;
17-
import java.util.ArrayList;
1818
import java.util.List;
1919
import java.util.Objects;
2020

2121
public class Insist extends UnaryPlan {
22-
private final UnresolvedAttribute insistedAttribute;
22+
private final Attribute insistedAttribute;
2323

24-
public Insist(Source source, UnresolvedAttribute insistedAttribute, LogicalPlan child) {
24+
public Insist(Source source, Attribute insistedAttribute, LogicalPlan child) {
2525
super(source, child);
2626
this.insistedAttribute = insistedAttribute;
2727
}
@@ -37,12 +37,10 @@ public List<Attribute> output() {
3737
}
3838

3939
private List<Attribute> computeOutput() {
40-
var result = new ArrayList<>(child().output());
41-
result.add(insistedAttribute);
42-
return result;
40+
return NamedExpressions.mergeOutputAttributes(List.of(insistedAttribute), child().output());
4341
}
4442

45-
public UnresolvedAttribute attribute() {
43+
public Attribute attribute() {
4644
return insistedAttribute;
4745
}
4846

@@ -53,7 +51,8 @@ public Insist replaceChild(LogicalPlan newChild) {
5351

5452
@Override
5553
public boolean expressionsResolved() {
56-
return insistedAttribute.resolved();
54+
// Like EsqlProject, we allow unsupported attributes to flow through the engine.
55+
return attribute().resolved() || attribute() instanceof UnsupportedAttribute;
5756
}
5857

5958
@Override
@@ -63,12 +62,12 @@ protected NodeInfo<? extends LogicalPlan> info() {
6362

6463
@Override
6564
public void writeTo(StreamOutput out) throws IOException {
66-
throw new UnsupportedOperationException("doesn't escape the node");
65+
throw new UnsupportedOperationException("doesn't escape the coordinator node");
6766
}
6867

6968
@Override
7069
public String getWriteableName() {
71-
throw new UnsupportedOperationException("doesn't escape the node");
70+
throw new UnsupportedOperationException("doesn't escape the coordinator node");
7271
}
7372

7473
@Override
@@ -80,4 +79,8 @@ public int hashCode() {
8079
public boolean equals(Object obj) {
8180
return super.equals(obj) && ((Insist) obj).insistedAttribute.equals(insistedAttribute);
8281
}
82+
83+
public LogicalPlan withAttribute(Attribute attribute) {
84+
return new Insist(source(), attribute, child());
85+
}
8386
}

0 commit comments

Comments
 (0)