Skip to content

Commit 5e696aa

Browse files
alex-spiesalex-spies
committed
Do not override toString
Just use the attribute accessor method. Rename it to `string` to not repeat fieldName.name() (we know it's a name) but rather express that this is the string representation.
1 parent be97463 commit 5e696aa

File tree

11 files changed

+39
-44
lines changed

11 files changed

+39
-44
lines changed

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,7 @@ public class FieldAttribute extends TypedAttribute {
4343
* A field name, as found in the mapping. Includes the whole path from the root of the document.
4444
* Implemented as a wrapper around {@link String} to distinguish from the attribute name (which sometimes differs!) at compile time.
4545
*/
46-
public record FieldName(String name) {
47-
@Override
48-
public String toString() {
49-
return name;
50-
}
51-
};
46+
public record FieldName(String string) {};
5247

5348
static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
5449
Attribute.class,

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,22 +351,22 @@ private boolean isConfigationSet(Config config, String field) {
351351

352352
@Override
353353
public boolean exists(FieldName field) {
354-
return isConfigationSet(Config.EXISTS, field.toString());
354+
return isConfigationSet(Config.EXISTS, field.string());
355355
}
356356

357357
@Override
358358
public boolean isIndexed(FieldName field) {
359-
return isConfigationSet(Config.INDEXED, field.toString());
359+
return isConfigationSet(Config.INDEXED, field.string());
360360
}
361361

362362
@Override
363363
public boolean hasDocValues(FieldName field) {
364-
return isConfigationSet(Config.DOC_VALUES, field.toString());
364+
return isConfigationSet(Config.DOC_VALUES, field.string());
365365
}
366366

367367
@Override
368368
public boolean hasExactSubfield(FieldName field) {
369-
return isConfigationSet(Config.EXACT_SUBFIELD, field.toString());
369+
return isConfigationSet(Config.EXACT_SUBFIELD, field.string());
370370
}
371371

372372
@Override
@@ -502,7 +502,7 @@ private static SearchStats fieldMatchingExistOrMissing(boolean exists, String...
502502

503503
@Override
504504
public boolean exists(FieldName field) {
505-
return fields.contains(field.toString()) == exists;
505+
return fields.contains(field.string()) == exists;
506506
}
507507
};
508508
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ private Tuple<List<Attribute>, List<EsStatsQueryExec.Stat>> pushableStats(
9797
if (target instanceof FieldAttribute fa) {
9898
var fName = fa.fieldName();
9999
if (context.searchStats().isSingleValue(fName)) {
100-
fieldName = fName.toString();
100+
fieldName = fName.string();
101101
query = QueryBuilders.existsQuery(fieldName);
102102
}
103103
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public final PhysicalOperation fieldExtractPhysicalOperation(FieldExtractExec fi
152152

153153
private static String getFieldName(Attribute attr) {
154154
// Do not use the field attribute name, this can deviate from the field name for union types.
155-
return attr instanceof FieldAttribute fa ? fa.fieldName().toString() : attr.name();
155+
return attr instanceof FieldAttribute fa ? fa.fieldName().string() : attr.name();
156156
}
157157

158158
private BlockLoader getBlockLoaderFor(int shardId, Attribute attr, MappedFieldType.FieldExtractPreference fieldExtractPreference) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/stats/SearchContextStats.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,23 +130,23 @@ private boolean fastNoCacheFieldExists(String field) {
130130

131131
@Override
132132
public boolean exists(FieldName field) {
133-
var stat = cache.get(field.toString());
134-
return stat != null ? stat.config.exists : fastNoCacheFieldExists(field.toString());
133+
var stat = cache.get(field.string());
134+
return stat != null ? stat.config.exists : fastNoCacheFieldExists(field.string());
135135
}
136136

137137
@Override
138138
public boolean isIndexed(FieldName field) {
139-
return cache.computeIfAbsent(field.toString(), this::makeFieldStats).config.indexed;
139+
return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.indexed;
140140
}
141141

142142
@Override
143143
public boolean hasDocValues(FieldName field) {
144-
return cache.computeIfAbsent(field.toString(), this::makeFieldStats).config.hasDocValues;
144+
return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.hasDocValues;
145145
}
146146

147147
@Override
148148
public boolean hasExactSubfield(FieldName field) {
149-
return cache.computeIfAbsent(field.toString(), this::makeFieldStats).config.hasExactSubfield;
149+
return cache.computeIfAbsent(field.string(), this::makeFieldStats).config.hasExactSubfield;
150150
}
151151

152152
public long count() {
@@ -160,11 +160,11 @@ public long count() {
160160

161161
@Override
162162
public long count(FieldName field) {
163-
var stat = cache.computeIfAbsent(field.toString(), this::makeFieldStats);
163+
var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats);
164164
if (stat.count == null) {
165165
var count = new long[] { 0 };
166166
boolean completed = doWithContexts(r -> {
167-
count[0] += countEntries(r, field.toString());
167+
count[0] += countEntries(r, field.string());
168168
return true;
169169
}, false);
170170
stat.count = completed ? count[0] : -1;
@@ -175,7 +175,7 @@ public long count(FieldName field) {
175175
@Override
176176
public long count(FieldName field, BytesRef value) {
177177
var count = new long[] { 0 };
178-
Term term = new Term(field.toString(), value);
178+
Term term = new Term(field.string(), value);
179179
boolean completed = doWithContexts(r -> {
180180
count[0] += r.docFreq(term);
181181
return true;
@@ -185,11 +185,11 @@ public long count(FieldName field, BytesRef value) {
185185

186186
@Override
187187
public byte[] min(FieldName field, DataType dataType) {
188-
var stat = cache.computeIfAbsent(field.toString(), this::makeFieldStats);
188+
var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats);
189189
if (stat.min == null) {
190190
var min = new byte[][] { null };
191191
doWithContexts(r -> {
192-
byte[] localMin = PointValues.getMinPackedValue(r, field.toString());
192+
byte[] localMin = PointValues.getMinPackedValue(r, field.string());
193193
// TODO: how to compare with the previous min
194194
if (localMin != null) {
195195
if (min[0] == null) {
@@ -208,11 +208,11 @@ public byte[] min(FieldName field, DataType dataType) {
208208

209209
@Override
210210
public byte[] max(FieldName field, DataType dataType) {
211-
var stat = cache.computeIfAbsent(field.toString(), this::makeFieldStats);
211+
var stat = cache.computeIfAbsent(field.string(), this::makeFieldStats);
212212
if (stat.max == null) {
213213
var max = new byte[][] { null };
214214
doWithContexts(r -> {
215-
byte[] localMax = PointValues.getMaxPackedValue(r, field.toString());
215+
byte[] localMax = PointValues.getMaxPackedValue(r, field.string());
216216
// TODO: how to compare with the previous max
217217
if (localMax != null) {
218218
if (max[0] == null) {
@@ -231,7 +231,7 @@ public byte[] max(FieldName field, DataType dataType) {
231231

232232
@Override
233233
public boolean isSingleValue(FieldName field) {
234-
String fieldName = field.toString();
234+
String fieldName = field.string();
235235
var stat = cache.computeIfAbsent(fieldName, this::makeFieldStats);
236236
if (stat.singleValue == null) {
237237
// there's no such field so no need to worry about multi-value fields
@@ -307,7 +307,7 @@ private boolean detectSingleValue(IndexReader r, MappedFieldType fieldType, Stri
307307
@Override
308308
public boolean canUseEqualityOnSyntheticSourceDelegate(FieldAttribute.FieldName name, String value) {
309309
for (SearchExecutionContext ctx : contexts) {
310-
MappedFieldType type = ctx.getFieldType(name.toString());
310+
MappedFieldType type = ctx.getFieldType(name.string());
311311
if (type == null) {
312312
return false;
313313
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ public void testReplaceUpperStringCasinqgWithInsensitiveRLike() {
653653
var filter = as(limit.child(), Filter.class);
654654
var rlike = as(filter.condition(), RLike.class);
655655
var field = as(rlike.field(), FieldAttribute.class);
656-
assertThat(field.fieldName().toString(), is("first_name"));
656+
assertThat(field.fieldName().string(), is("first_name"));
657657
assertThat(rlike.pattern().pattern(), is("VALÜ*"));
658658
assertThat(rlike.caseInsensitive(), is(true));
659659
var source = as(filter.child(), EsRelation.class);
@@ -667,7 +667,7 @@ public void testReplaceLowerStringCasingWithInsensitiveRLike() {
667667
var filter = as(limit.child(), Filter.class);
668668
var rlike = as(filter.condition(), RLike.class);
669669
var field = as(rlike.field(), FieldAttribute.class);
670-
assertThat(field.fieldName().toString(), is("first_name"));
670+
assertThat(field.fieldName().string(), is("first_name"));
671671
assertThat(rlike.pattern().pattern(), is("valü*"));
672672
assertThat(rlike.caseInsensitive(), is(true));
673673
var source = as(filter.child(), EsRelation.class);
@@ -692,7 +692,7 @@ public void testReplaceUpperStringCasingWithInsensitiveLike() {
692692
var filter = as(limit.child(), Filter.class);
693693
var wlike = as(filter.condition(), WildcardLike.class);
694694
var field = as(wlike.field(), FieldAttribute.class);
695-
assertThat(field.fieldName().toString(), is("first_name"));
695+
assertThat(field.fieldName().string(), is("first_name"));
696696
assertThat(wlike.pattern().pattern(), is("VALÜ*"));
697697
assertThat(wlike.caseInsensitive(), is(true));
698698
var source = as(filter.child(), EsRelation.class);
@@ -706,7 +706,7 @@ public void testReplaceLowerStringCasingWithInsensitiveLike() {
706706
var filter = as(limit.child(), Filter.class);
707707
var wlike = as(filter.condition(), WildcardLike.class);
708708
var field = as(wlike.field(), FieldAttribute.class);
709-
assertThat(field.fieldName().toString(), is("first_name"));
709+
assertThat(field.fieldName().string(), is("first_name"));
710710
assertThat(wlike.pattern().pattern(), is("valü*"));
711711
assertThat(wlike.caseInsensitive(), is(true));
712712
var source = as(filter.child(), EsRelation.class);
@@ -741,7 +741,7 @@ public void testUnionTypesInferNonNullAggConstraint() {
741741
var isNotNull = as(filter.condition(), IsNotNull.class);
742742
var unionTypeField = as(isNotNull.field(), FieldAttribute.class);
743743
assertEquals("$$integer_long_field$converted_to$long", unionTypeField.name());
744-
assertEquals("integer_long_field", unionTypeField.fieldName().toString());
744+
assertEquals("integer_long_field", unionTypeField.fieldName().string());
745745
}
746746

747747
private IsNotNull isNotNull(Expression field) {

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public boolean isSingleValue(FieldAttribute.FieldName field) {
169169

170170
@Override
171171
public String constantValue(FieldAttribute.FieldName name) {
172-
return name.toString().startsWith("constant_keyword") ? "foo" : null;
172+
return name.string().startsWith("constant_keyword") ? "foo" : null;
173173
}
174174
};
175175

@@ -550,7 +550,7 @@ public void testLocalAggOptimizedToLocalRelation() {
550550
var stats = new TestSearchStats() {
551551
@Override
552552
public boolean exists(FieldAttribute.FieldName field) {
553-
return "emp_no".equals(field.toString()) == false;
553+
return "emp_no".equals(field.string()) == false;
554554
}
555555
};
556556

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6341,7 +6341,7 @@ public void testReplaceStringCasingWithInsensitiveEqualsUnwrap() {
63416341
var filter = as(limit.child(), Filter.class);
63426342
var insensitive = as(filter.condition(), InsensitiveEquals.class);
63436343
var field = as(insensitive.left(), FieldAttribute.class);
6344-
assertThat(field.fieldName().toString(), is("first_name"));
6344+
assertThat(field.fieldName().string(), is("first_name"));
63456345
var bRef = as(insensitive.right().fold(FoldContext.small()), BytesRef.class);
63466346
assertThat(bRef.utf8ToString(), is("VALÜ"));
63476347
as(filter.child(), EsRelation.class);

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/PhysicalPlanOptimizerTests.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,7 +2442,7 @@ public void testPushDownEvalFilter() {
24422442
assertThat(range2.fieldName(), is("first_name"));
24432443
var sort = source.sorts();
24442444
assertThat(sort.size(), is(1));
2445-
assertThat(sort.get(0).field().fieldName().toString(), is("first_name"));
2445+
assertThat(sort.get(0).field().fieldName().string(), is("first_name"));
24462446
}
24472447

24482448
/**
@@ -2501,7 +2501,7 @@ public void testPushDownEvalSwapFilter() {
25012501
assertThat(exists.fieldName(), is("first_name"));
25022502
var sort = source.sorts();
25032503
assertThat(sort.size(), is(1));
2504-
assertThat(sort.get(0).field().fieldName().toString(), is("last_name"));
2504+
assertThat(sort.get(0).field().fieldName().string(), is("last_name"));
25052505
}
25062506

25072507
/**
@@ -3225,7 +3225,7 @@ public void testAggToLocalRelationOnDataNode() {
32253225
var stats = new EsqlTestUtils.TestSearchStats() {
32263226
@Override
32273227
public boolean exists(FieldAttribute.FieldName field) {
3228-
return "salary".equals(field.toString());
3228+
return "salary".equals(field.string());
32293229
}
32303230
};
32313231
var optimized = optimizedPlan(plan, stats);
@@ -5093,7 +5093,7 @@ public void testPushSpatialDistanceEvalToSource() {
50935093
assertThat(alias.name(), is("distance"));
50945094
var stDistance = as(alias.child(), StDistance.class);
50955095
var location = as(stDistance.left(), FieldAttribute.class);
5096-
assertThat(location.fieldName().toString(), is("location"));
5096+
assertThat(location.fieldName().string(), is("location"));
50975097

50985098
// Validate the filter condition
50995099
var and = as(filter.condition(), And.class);
@@ -5147,7 +5147,7 @@ public void testPushSpatialDistanceMultiEvalToSource() {
51475147
assertThat(alias2.name(), is("distance"));
51485148
var stDistance = as(alias2.child(), StDistance.class);
51495149
var location = as(stDistance.left(), FieldAttribute.class);
5150-
assertThat(location.fieldName().toString(), is("location"));
5150+
assertThat(location.fieldName().string(), is("location"));
51515151
var poiRef = as(stDistance.right(), Literal.class);
51525152
assertThat(poiRef.value(), instanceOf(BytesRef.class));
51535153
assertThat(poiRef.value().toString(), is(poi.value().toString()));
@@ -6503,7 +6503,7 @@ public void testPushCompoundTopNDistanceWithCompoundFilterAndCompoundEvalToSourc
65036503
assertThat(alias2.name(), is("distance"));
65046504
var stDistance = as(alias2.child(), StDistance.class);
65056505
var location = as(stDistance.left(), FieldAttribute.class);
6506-
assertThat(location.fieldName().toString(), is("location"));
6506+
assertThat(location.fieldName().string(), is("location"));
65076507
var poiRef = as(stDistance.right(), Literal.class);
65086508
assertThat(poiRef.value(), instanceOf(BytesRef.class));
65096509
assertThat(poiRef.value().toString(), is(poi.value().toString()));
@@ -6689,7 +6689,7 @@ public void testPushCompoundTopNDistanceWithCompoundFilterAndNestedCompoundEvalT
66896689
assertThat(alias2.name(), is("distance"));
66906690
var stDistance = as(alias2.child(), StDistance.class);
66916691
var location = as(stDistance.left(), FieldAttribute.class);
6692-
assertThat(location.fieldName().toString(), is("location"));
6692+
assertThat(location.fieldName().string(), is("location"));
66936693
var poiRef = as(stDistance.right(), Literal.class);
66946694
assertThat(poiRef.value(), instanceOf(BytesRef.class));
66956695
assertThat(poiRef.value().toString(), is(poi.value().toString()));

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushTopNToSourceTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ private static void assertPushdownSort(
452452
String name = ((Attribute) expectedSorts.get(i).child()).name();
453453
EsQueryExec.Sort sort = sorts.get(i);
454454
if (sort.field() != null) {
455-
String fieldName = sort.field().fieldName().toString();
455+
String fieldName = sort.field().fieldName().string();
456456
assertThat("Expect sort[" + i + "] name to match", fieldName, is(sortName(name, fieldMap)));
457457
}
458458
assertThat("Expect sort[" + i + "] direction to match", sort.direction(), is(expectedSorts.get(i).direction()));

0 commit comments

Comments
 (0)