Skip to content

Commit de1351d

Browse files
nik9000nik9000
authored
ESQL: Remove two unused fields from transport (#127854)
Removes two unused fields from the transport protocol. They haven't been used for a while.
1 parent 746b1df commit de1351d

File tree

5 files changed

+49
-119
lines changed

5 files changed

+49
-119
lines changed

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ static TransportVersion def(int id) {
251251
public static final TransportVersion FILE_SETTINGS_HEALTH_INFO = def(9_072_0_00);
252252
public static final TransportVersion FIELD_CAPS_ADD_CLUSTER_ALIAS = def(9_073_0_00);
253253
public static final TransportVersion INFERENCE_ADD_TIMEOUT_PUT_ENDPOINT = def(9_074_00_0);
254+
public static final TransportVersion ESQL_FIELD_ATTRIBUTE_DROP_TYPE = def(9_075_00_0);
254255

255256
/*
256257
* STOP! READ THIS FIRST! No, really,

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

Lines changed: 26 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
*/
77
package org.elasticsearch.xpack.esql.core.expression;
88

9-
import org.elasticsearch.TransportVersions;
109
import org.elasticsearch.common.Strings;
1110
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1211
import org.elasticsearch.common.io.stream.StreamInput;
@@ -22,6 +21,7 @@
2221
import java.io.IOException;
2322
import java.util.Objects;
2423

24+
import static org.elasticsearch.TransportVersions.ESQL_FIELD_ATTRIBUTE_DROP_TYPE;
2525
import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck;
2626
import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck;
2727

@@ -65,47 +65,12 @@ public FieldAttribute(
6565
@Nullable NameId id,
6666
boolean synthetic
6767
) {
68-
this(source, parentName, name, field.getDataType(), field, nullability, id, synthetic);
69-
}
70-
71-
/**
72-
* Used only for testing. Do not use this otherwise, as an explicitly set type will be ignored the next time this FieldAttribute is
73-
* {@link FieldAttribute#clone}d.
74-
*/
75-
FieldAttribute(
76-
Source source,
77-
@Nullable String parentName,
78-
String name,
79-
DataType type,
80-
EsField field,
81-
Nullability nullability,
82-
@Nullable NameId id,
83-
boolean synthetic
84-
) {
85-
super(source, name, type, nullability, id, synthetic);
68+
super(source, name, field.getDataType(), nullability, id, synthetic);
8669
this.parentName = parentName;
8770
this.field = field;
8871
}
8972

90-
@Deprecated
91-
/**
92-
* Old constructor from when this had a qualifier string. Still needed to not break serialization.
93-
*/
94-
private FieldAttribute(
95-
Source source,
96-
@Nullable String parentName,
97-
String name,
98-
DataType type,
99-
EsField field,
100-
@Nullable String qualifier,
101-
Nullability nullability,
102-
@Nullable NameId id,
103-
boolean synthetic
104-
) {
105-
this(source, parentName, name, type, field, nullability, id, synthetic);
106-
}
107-
108-
private FieldAttribute(StreamInput in) throws IOException {
73+
private static FieldAttribute innerReadFrom(StreamInput in) throws IOException {
10974
/*
11075
* The funny casting dance with `(StreamInput & PlanStreamInput) in` is required
11176
* because we're in esql-core here and the real PlanStreamInput is in
@@ -114,57 +79,44 @@ private FieldAttribute(StreamInput in) throws IOException {
11479
* and NameId. This should become a hard cast when we move everything out
11580
* of esql-core.
11681
*/
117-
this(
118-
Source.readFrom((StreamInput & PlanStreamInput) in),
119-
readParentName(in),
120-
readCachedStringWithVersionCheck(in),
121-
DataType.readFrom(in),
122-
EsField.readFrom(in),
123-
in.readOptionalString(),
124-
in.readEnum(Nullability.class),
125-
NameId.readFrom((StreamInput & PlanStreamInput) in),
126-
in.readBoolean()
127-
);
82+
Source source = Source.readFrom((StreamInput & PlanStreamInput) in);
83+
String parentName = ((PlanStreamInput) in).readOptionalCachedString();
84+
String name = readCachedStringWithVersionCheck(in);
85+
if (in.getTransportVersion().before(ESQL_FIELD_ATTRIBUTE_DROP_TYPE)) {
86+
DataType.readFrom(in);
87+
}
88+
EsField field = EsField.readFrom(in);
89+
if (in.getTransportVersion().before(ESQL_FIELD_ATTRIBUTE_DROP_TYPE)) {
90+
in.readOptionalString();
91+
}
92+
Nullability nullability = in.readEnum(Nullability.class);
93+
NameId nameId = NameId.readFrom((StreamInput & PlanStreamInput) in);
94+
boolean synthetic = in.readBoolean();
95+
return new FieldAttribute(source, parentName, name, field, nullability, nameId, synthetic);
12896
}
12997

13098
@Override
13199
public void writeTo(StreamOutput out) throws IOException {
132100
if (((PlanStreamOutput) out).writeAttributeCacheHeader(this)) {
133101
Source.EMPTY.writeTo(out);
134-
writeParentName(out);
102+
((PlanStreamOutput) out).writeOptionalCachedString(parentName);
135103
writeCachedStringWithVersionCheck(out, name());
136-
dataType().writeTo(out);
104+
if (out.getTransportVersion().before(ESQL_FIELD_ATTRIBUTE_DROP_TYPE)) {
105+
dataType().writeTo(out);
106+
}
137107
field.writeTo(out);
138-
// We used to write the qualifier here. We can still do if needed in the future.
139-
out.writeOptionalString(null);
108+
if (out.getTransportVersion().before(ESQL_FIELD_ATTRIBUTE_DROP_TYPE)) {
109+
// We used to write the qualifier here. We can still do if needed in the future.
110+
out.writeOptionalString(null);
111+
}
140112
out.writeEnum(nullable());
141113
id().writeTo(out);
142114
out.writeBoolean(synthetic());
143115
}
144116
}
145117

146118
public static FieldAttribute readFrom(StreamInput in) throws IOException {
147-
return ((PlanStreamInput) in).readAttributeWithCache(FieldAttribute::new);
148-
}
149-
150-
private void writeParentName(StreamOutput out) throws IOException {
151-
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_17_0)) {
152-
((PlanStreamOutput) out).writeOptionalCachedString(parentName);
153-
} else {
154-
// Previous versions only used the parent field attribute to retrieve the parent's name, so we can use just any
155-
// fake FieldAttribute here as long as the name is correct.
156-
FieldAttribute fakeParent = parentName() == null ? null : new FieldAttribute(Source.EMPTY, parentName(), field());
157-
out.writeOptionalWriteable(fakeParent);
158-
}
159-
}
160-
161-
private static String readParentName(StreamInput in) throws IOException {
162-
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_17_0)) {
163-
return ((PlanStreamInput) in).readOptionalCachedString();
164-
}
165-
166-
FieldAttribute parent = in.readOptionalWriteable(FieldAttribute::readFrom);
167-
return parent == null ? null : parent.name();
119+
return ((PlanStreamInput) in).readAttributeWithCache(FieldAttribute::innerReadFrom);
168120
}
169121

170122
@Override

‎x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/FieldAttributeTestUtils.java‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FieldAttributeTests.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,22 @@
1515
import org.elasticsearch.xpack.esql.core.type.EsField;
1616
import org.elasticsearch.xpack.esql.type.AbstractEsFieldTypeTests;
1717

18-
import static org.elasticsearch.xpack.esql.core.expression.FieldAttributeTestUtils.newFieldAttributeWithType;
19-
2018
public class FieldAttributeTests extends AbstractAttributeTestCase<FieldAttribute> {
2119
public static FieldAttribute createFieldAttribute(int maxDepth, boolean onlyRepresentable) {
2220
Source source = Source.EMPTY;
2321
String parentName = maxDepth == 0 || randomBoolean() ? null : randomAlphaOfLength(3);
2422
String name = randomAlphaOfLength(5);
25-
DataType type = onlyRepresentable
26-
? randomValueOtherThanMany(t -> false == DataType.isRepresentable(t), () -> randomFrom(DataType.types()))
27-
: randomFrom(DataType.types());
28-
EsField field = AbstractEsFieldTypeTests.randomAnyEsField(maxDepth);
23+
EsField field = onlyRepresentable ? randomRepresentableEsField(maxDepth) : AbstractEsFieldTypeTests.randomAnyEsField(maxDepth);
2924
Nullability nullability = randomFrom(Nullability.values());
3025
boolean synthetic = randomBoolean();
31-
return newFieldAttributeWithType(source, parentName, name, type, field, nullability, new NameId(), synthetic);
26+
return new FieldAttribute(source, parentName, name, field, nullability, new NameId(), synthetic);
27+
}
28+
29+
private static EsField randomRepresentableEsField(int maxDepth) {
30+
return randomValueOtherThanMany(
31+
f -> false == DataType.isRepresentable(f.getDataType()),
32+
() -> AbstractEsFieldTypeTests.randomAnyEsField(maxDepth)
33+
);
3234
}
3335

3436
@Override
@@ -41,18 +43,16 @@ protected FieldAttribute mutate(FieldAttribute instance) {
4143
Source source = instance.source();
4244
String parentName = instance.parentName();
4345
String name = instance.name();
44-
DataType type = instance.dataType();
4546
EsField field = instance.field();
4647
Nullability nullability = instance.nullable();
4748
boolean synthetic = instance.synthetic();
48-
switch (between(0, 5)) {
49+
switch (between(0, 4)) {
4950
case 0 -> parentName = randomValueOtherThan(parentName, () -> randomBoolean() ? null : randomAlphaOfLength(2));
5051
case 1 -> name = randomAlphaOfLength(name.length() + 1);
51-
case 2 -> type = randomValueOtherThan(type, () -> randomFrom(DataType.types()));
52-
case 3 -> field = randomValueOtherThan(field, () -> AbstractEsFieldTypeTests.randomAnyEsField(3));
53-
case 4 -> nullability = randomValueOtherThan(nullability, () -> randomFrom(Nullability.values()));
54-
case 5 -> synthetic = false == synthetic;
52+
case 2 -> field = randomValueOtherThan(field, () -> AbstractEsFieldTypeTests.randomAnyEsField(3));
53+
case 3 -> nullability = randomValueOtherThan(nullability, () -> randomFrom(Nullability.values()));
54+
case 4 -> synthetic = false == synthetic;
5555
}
56-
return newFieldAttributeWithType(source, parentName, name, type, field, nullability, new NameId(), synthetic);
56+
return new FieldAttribute(source, parentName, name, field, nullability, new NameId(), synthetic);
5757
}
5858
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/ExchangeSinkExecSerializationTests.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ public void testManyTypeConflicts() throws IOException {
7777
* 1.4mb - string serialization #112929
7878
* 1424046b - remove node-level plan #117422
7979
* 1040607b - remove EsIndex mapping serialization #119580
80+
* 1019093b - remove unused fields from FieldAttribute #127854
8081
*/
81-
testManyTypeConflicts(false, ByteSizeValue.ofBytes(1040607));
82+
testManyTypeConflicts(false, ByteSizeValue.ofBytes(1019093));
8283
}
8384

8485
/**
@@ -96,8 +97,9 @@ public void testManyTypeConflictsWithParent() throws IOException {
9697
* 2774192b - remove field attribute #112881
9798
* 2774190b - remove node-level plan #117422
9899
* 2007288b - remove EsIndex mapping serialization #119580
100+
* 1964273b - remove unused fields from FieldAttribute #127854
99101
*/
100-
testManyTypeConflicts(true, ByteSizeValue.ofBytes(2007288));
102+
testManyTypeConflicts(true, ByteSizeValue.ofBytes(1964273));
101103
}
102104

103105
private void testManyTypeConflicts(boolean withParent, ByteSizeValue expected) throws IOException {
@@ -117,13 +119,14 @@ public void testDeeplyNestedFields() throws IOException {
117119
* 47252411b - remove field attribute #112881
118120
* 47252409b - remove node-level plan #117422
119121
* 43927169b - remove EsIndex mapping serialization #119580
122+
* 43402881b - remove unused fields from FieldAttribute #127854
120123
*/
121124

122125
int depth = 6;
123126
int childrenPerLevel = 8;
124127

125128
EsIndex index = EsIndexSerializationTests.deeplyNestedIndex(depth, childrenPerLevel);
126-
testSerializePlanWithIndex(index, ByteSizeValue.ofBytes(43927169));
129+
testSerializePlanWithIndex(index, ByteSizeValue.ofBytes(43402881));
127130
}
128131

129132
/**
@@ -138,13 +141,14 @@ public void testDeeplyNestedFieldsKeepOnlyOne() throws IOException {
138141
* 9425806b - remove field attribute #112881
139142
* 9425804b - remove node-level plan #117422
140143
* 352b - remove EsIndex mapping serialization #119580
144+
* 350b - remove unused fields from FieldAttribute #127854
141145
*/
142146

143147
int depth = 6;
144148
int childrenPerLevel = 9;
145149

146150
EsIndex index = EsIndexSerializationTests.deeplyNestedIndex(depth, childrenPerLevel);
147-
testSerializePlanWithIndex(index, ByteSizeValue.ofBytes(352), false);
151+
testSerializePlanWithIndex(index, ByteSizeValue.ofBytes(350), false);
148152
}
149153

150154
/**

0 commit comments

Comments
 (0)