diff --git a/docs/changelog/112881.yaml b/docs/changelog/112881.yaml deleted file mode 100644 index a8a0d542f8201..0000000000000 --- a/docs/changelog/112881.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 112881 -summary: "ESQL: Remove parent from `FieldAttribute`" -area: ES|QL -type: enhancement -issues: [] diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index bf61752a1d771..dcf6f7aebdc65 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -175,7 +175,6 @@ static TransportVersion def(int id) { public static final TransportVersion ML_INFERENCE_ATTACH_TO_EXISTSING_DEPLOYMENT = def(8_771_00_0); public static final TransportVersion CONVERT_FAILURE_STORE_OPTIONS_TO_SELECTOR_OPTIONS_INTERNALLY = def(8_772_00_0); public static final TransportVersion REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_773_00_0); - public static final TransportVersion ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED = def(8_774_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.java index 1f7d03ba9d905..e33f9b1c20527 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.java @@ -9,7 +9,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -43,11 +42,11 @@ public Alias(Source source, String name, Expression child) { this(source, name, child, null); } - public Alias(Source source, String name, Expression child, @Nullable NameId id) { + public Alias(Source source, String name, Expression child, NameId id) { this(source, name, child, id, false); } - public Alias(Source source, String name, Expression child, @Nullable NameId id, boolean synthetic) { + public Alias(Source source, String name, Expression child, NameId id, boolean synthetic) { super(source, name, singletonList(child), id, synthetic); this.child = child; } @@ -56,7 +55,7 @@ public Alias(Source source, String name, Expression child, @Nullable NameId id, /** * Old constructor from when this had a qualifier string. Still needed to not break serialization. */ - private Alias(Source source, String name, String qualifier, Expression child, @Nullable NameId id, boolean synthetic) { + private Alias(Source source, String name, String qualifier, Expression child, NameId id, boolean synthetic) { this(source, name, child, id, synthetic); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java index 45f42a754910d..05c414298fd33 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.core.expression; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -42,15 +41,15 @@ public static List getNamedWriteables() { // can the attr be null - typically used in JOINs private final Nullability nullability; - public Attribute(Source source, String name, @Nullable NameId id) { + public Attribute(Source source, String name, NameId id) { this(source, name, Nullability.TRUE, id); } - public Attribute(Source source, String name, Nullability nullability, @Nullable NameId id) { + public Attribute(Source source, String name, Nullability nullability, NameId id) { this(source, name, nullability, id, false); } - public Attribute(Source source, String name, Nullability nullability, @Nullable NameId id, boolean synthetic) { + public Attribute(Source source, String name, Nullability nullability, NameId id, boolean synthetic) { super(source, name, emptyList(), id, synthetic); this.nullability = nullability; } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java index 4076acdb7e7b8..767d2f45f90e4 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.java @@ -6,25 +6,21 @@ */ package org.elasticsearch.xpack.esql.core.expression; -import org.elasticsearch.TransportVersions; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; +import org.elasticsearch.xpack.esql.core.util.StringUtils; import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Attribute for an ES field. * To differentiate between the different type of fields this class offers: @@ -41,31 +37,32 @@ public class FieldAttribute extends TypedAttribute { FieldAttribute::readFrom ); - private final String parentName; + private final FieldAttribute parent; + private final String path; private final EsField field; public FieldAttribute(Source source, String name, EsField field) { this(source, null, name, field); } - public FieldAttribute(Source source, @Nullable String parentName, String name, EsField field) { - this(source, parentName, name, field, Nullability.TRUE, null, false); + public FieldAttribute(Source source, FieldAttribute parent, String name, EsField field) { + this(source, parent, name, field, Nullability.TRUE, null, false); } - public FieldAttribute(Source source, @Nullable String parentName, String name, EsField field, boolean synthetic) { - this(source, parentName, name, field, Nullability.TRUE, null, synthetic); + public FieldAttribute(Source source, FieldAttribute parent, String name, EsField field, boolean synthetic) { + this(source, parent, name, field, Nullability.TRUE, null, synthetic); } public FieldAttribute( Source source, - @Nullable String parentName, + FieldAttribute parent, String name, EsField field, Nullability nullability, - @Nullable NameId id, + NameId id, boolean synthetic ) { - this(source, parentName, name, field.getDataType(), field, nullability, id, synthetic); + this(source, parent, name, field.getDataType(), field, nullability, id, synthetic); } /** @@ -74,16 +71,17 @@ public FieldAttribute( */ FieldAttribute( Source source, - @Nullable String parentName, + FieldAttribute parent, String name, DataType type, EsField field, Nullability nullability, - @Nullable NameId id, + NameId id, boolean synthetic ) { super(source, name, type, nullability, id, synthetic); - this.parentName = parentName; + this.path = parent != null ? parent.name() : StringUtils.EMPTY; + this.parent = parent; this.field = field; } @@ -93,16 +91,16 @@ public FieldAttribute( */ private FieldAttribute( Source source, - @Nullable String parentName, + FieldAttribute parent, String name, DataType type, EsField field, - @Nullable String qualifier, + String qualifier, Nullability nullability, - @Nullable NameId id, + NameId id, boolean synthetic ) { - this(source, parentName, name, type, field, nullability, id, synthetic); + this(source, parent, name, type, field, nullability, id, synthetic); } private FieldAttribute(StreamInput in) throws IOException { @@ -116,8 +114,8 @@ private FieldAttribute(StreamInput in) throws IOException { */ this( Source.readFrom((StreamInput & PlanStreamInput) in), - readParentName(in), - readCachedStringWithVersionCheck(in), + in.readOptionalWriteable(FieldAttribute::readFrom), + ((PlanStreamInput) in).readCachedString(), DataType.readFrom(in), EsField.readFrom(in), in.readOptionalString(), @@ -131,8 +129,8 @@ private FieldAttribute(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { if (((PlanStreamOutput) out).writeAttributeCacheHeader(this)) { Source.EMPTY.writeTo(out); - writeParentName(out); - writeCachedStringWithVersionCheck(out, name()); + out.writeOptionalWriteable(parent); + ((PlanStreamOutput) out).writeCachedString(name()); dataType().writeTo(out); field.writeTo(out); // We used to write the qualifier here. We can still do if needed in the future. @@ -147,26 +145,6 @@ public static FieldAttribute readFrom(StreamInput in) throws IOException { return ((PlanStreamInput) in).readAttributeWithCache(FieldAttribute::new); } - private void writeParentName(StreamOutput out) throws IOException { - if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED)) { - ((PlanStreamOutput) out).writeOptionalCachedString(parentName); - } else { - // Previous versions only used the parent field attribute to retrieve the parent's name, so we can use just any - // fake FieldAttribute here as long as the name is correct. - FieldAttribute fakeParent = parentName() == null ? null : new FieldAttribute(Source.EMPTY, parentName(), field()); - out.writeOptionalWriteable(fakeParent); - } - } - - private static String readParentName(StreamInput in) throws IOException { - if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED)) { - return ((PlanStreamInput) in).readOptionalCachedString(); - } - - FieldAttribute parent = in.readOptionalWriteable(FieldAttribute::readFrom); - return parent == null ? null : parent.name(); - } - @Override public String getWriteableName() { return ENTRY.name; @@ -174,22 +152,15 @@ public String getWriteableName() { @Override protected NodeInfo info() { - return NodeInfo.create( - this, - FieldAttribute::new, - parentName, - name(), - dataType(), - field, - (String) null, - nullable(), - id(), - synthetic() - ); + return NodeInfo.create(this, FieldAttribute::new, parent, name(), dataType(), field, (String) null, nullable(), id(), synthetic()); + } + + public FieldAttribute parent() { + return parent; } - public String parentName() { - return parentName; + public String path() { + return path; } /** @@ -203,7 +174,7 @@ public String fieldName() { if ((synthetic() || name().startsWith(SYNTHETIC_ATTRIBUTE_NAME_PREFIX)) == false) { return name(); } - return Strings.hasText(parentName) ? parentName + "." + field.getName() : field.getName(); + return Strings.hasText(path) ? path + "." + field.getName() : field.getName(); } public EsField.Exact getExactInfo() { @@ -219,13 +190,13 @@ public FieldAttribute exactAttribute() { } private FieldAttribute innerField(EsField type) { - return new FieldAttribute(source(), name(), name() + "." + type.getName(), type, nullable(), id(), synthetic()); + return new FieldAttribute(source(), this, name() + "." + type.getName(), type, nullable(), id(), synthetic()); } @Override protected Attribute clone(Source source, String name, DataType type, Nullability nullability, NameId id, boolean synthetic) { // Ignore `type`, this must be the same as the field's type. - return new FieldAttribute(source, parentName, name, field, nullability, id, synthetic); + return new FieldAttribute(source, parent, name, field, nullability, id, synthetic); } @Override @@ -235,13 +206,13 @@ public Attribute withDataType(DataType type) { @Override public int hashCode() { - return Objects.hash(super.hashCode(), parentName, field); + return Objects.hash(super.hashCode(), path, field); } @Override public boolean equals(Object obj) { return super.equals(obj) - && Objects.equals(parentName, ((FieldAttribute) obj).parentName) + && Objects.equals(path, ((FieldAttribute) obj).path) && Objects.equals(field, ((FieldAttribute) obj).field); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java index 3641812cd6cad..539c55ba341cf 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.java @@ -10,7 +10,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.IgnoredFieldMapper; @@ -60,7 +59,7 @@ public MetadataAttribute( String name, DataType dataType, Nullability nullability, - @Nullable NameId id, + NameId id, boolean synthetic, boolean searchable ) { @@ -80,9 +79,9 @@ private MetadataAttribute( Source source, String name, DataType dataType, - @Nullable String qualifier, + String qualifier, Nullability nullability, - @Nullable NameId id, + NameId id, boolean synthetic, boolean searchable ) { diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.java index 3b018f09e5ebd..ba467910bed0d 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.java @@ -8,7 +8,6 @@ import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.Source; import java.util.ArrayList; @@ -33,11 +32,11 @@ public static List getNamedWriteables() { private final NameId id; private final boolean synthetic; - public NamedExpression(Source source, String name, List children, @Nullable NameId id) { + public NamedExpression(Source source, String name, List children, NameId id) { this(source, name, children, id, false); } - public NamedExpression(Source source, String name, List children, @Nullable NameId id, boolean synthetic) { + public NamedExpression(Source source, String name, List children, NameId id, boolean synthetic) { super(source, children); this.name = name; this.id = id == null ? new NameId() : id; diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java index 3626c5d26f235..504e1eae8d880 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/ReferenceAttribute.java @@ -9,7 +9,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -32,14 +31,7 @@ public ReferenceAttribute(Source source, String name, DataType dataType) { this(source, name, dataType, Nullability.FALSE, null, false); } - public ReferenceAttribute( - Source source, - String name, - DataType dataType, - Nullability nullability, - @Nullable NameId id, - boolean synthetic - ) { + public ReferenceAttribute(Source source, String name, DataType dataType, Nullability nullability, NameId id, boolean synthetic) { super(source, name, dataType, nullability, id, synthetic); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/TypedAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/TypedAttribute.java index f8a041110798c..0350abef99992 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/TypedAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/TypedAttribute.java @@ -6,7 +6,6 @@ */ package org.elasticsearch.xpack.esql.core.expression; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -16,14 +15,7 @@ public abstract class TypedAttribute extends Attribute { private final DataType dataType; - protected TypedAttribute( - Source source, - String name, - DataType dataType, - Nullability nullability, - @Nullable NameId id, - boolean synthetic - ) { + protected TypedAttribute(Source source, String name, DataType dataType, Nullability nullability, NameId id, boolean synthetic) { super(source, name, nullability, id, synthetic); this.dataType = dataType; } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java index a971a15a23c86..d8a35adcbffde 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/UnresolvedAttribute.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.core.expression; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.capabilities.Unresolvable; import org.elasticsearch.xpack.esql.core.capabilities.UnresolvedException; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; @@ -34,7 +33,7 @@ public UnresolvedAttribute(Source source, String name, String unresolvedMessage) } @SuppressWarnings("this-escape") - public UnresolvedAttribute(Source source, String name, @Nullable NameId id, String unresolvedMessage, Object resolutionMetadata) { + public UnresolvedAttribute(Source source, String name, NameId id, String unresolvedMessage, Object resolutionMetadata) { super(source, name, id); this.customMessage = unresolvedMessage != null; this.unresolvedMsg = unresolvedMessage == null ? errorMessage(name(), null) : unresolvedMessage; diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java index 12699ca3ee720..cb1a7b2eb6fe0 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java @@ -14,6 +14,8 @@ import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper; import org.elasticsearch.xpack.esql.core.plugin.EsqlCorePlugin; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.math.BigInteger; @@ -30,8 +32,6 @@ import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toUnmodifiableMap; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; public enum DataType { /** @@ -535,12 +535,12 @@ public DataType counter() { } public void writeTo(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, typeName); + ((PlanStreamOutput) out).writeCachedString(typeName); } public static DataType readFrom(StreamInput in) throws IOException { // TODO: Use our normal enum serialization pattern - return readFrom(readCachedStringWithVersionCheck(in)); + return readFrom(((PlanStreamInput) in).readCachedString()); } /** diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DateEsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DateEsField.java index 3a81ec2a6f17d..7c4b98c5af84e 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DateEsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DateEsField.java @@ -8,13 +8,12 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.Map; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Information about a field in an ES index with the {@code date} type */ @@ -29,12 +28,12 @@ private DateEsField(String name, DataType dataType, Map propert } protected DateEsField(StreamInput in) throws IOException { - this(readCachedStringWithVersionCheck(in), DataType.DATETIME, in.readImmutableMap(EsField::readFrom), in.readBoolean()); + this(((PlanStreamInput) in).readCachedString(), DataType.DATETIME, in.readImmutableMap(EsField::readFrom), in.readBoolean()); } @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); + ((PlanStreamOutput) out).writeCachedString(getName()); out.writeMap(getProperties(), (o, x) -> x.writeTo(out)); out.writeBoolean(isAggregatable()); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java index 47dadcbb11de2..6235176d82de6 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/EsField.java @@ -18,9 +18,6 @@ import java.util.Map; import java.util.Objects; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Information about a field in an ES index. */ @@ -63,7 +60,7 @@ public EsField(String name, DataType esDataType, Map properties } public EsField(StreamInput in) throws IOException { - this.name = readCachedStringWithVersionCheck(in); + this.name = ((PlanStreamInput) in).readCachedString(); this.esDataType = readDataType(in); this.properties = in.readImmutableMap(EsField::readFrom); this.aggregatable = in.readBoolean(); @@ -71,7 +68,7 @@ public EsField(StreamInput in) throws IOException { } private DataType readDataType(StreamInput in) throws IOException { - String name = readCachedStringWithVersionCheck(in); + String name = ((PlanStreamInput) in).readCachedString(); if (in.getTransportVersion().before(TransportVersions.ESQL_NESTED_UNSUPPORTED) && name.equalsIgnoreCase("NESTED")) { /* * The "nested" data type existed in older versions of ESQL but was @@ -101,7 +98,7 @@ public void writeTo(StreamOutput out) throws IOException { * This needs to be overridden by subclasses for specific serialization */ public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, name); + ((PlanStreamOutput) out).writeCachedString(name); esDataType.writeTo(out); out.writeMap(properties, (o, x) -> x.writeTo(out)); out.writeBoolean(aggregatable); diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/InvalidMappedField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/InvalidMappedField.java index f83e4652ebebd..40825af56ccfe 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/InvalidMappedField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/InvalidMappedField.java @@ -10,6 +10,8 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.Map; @@ -18,9 +20,6 @@ import java.util.TreeMap; import java.util.stream.Collectors; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Representation of field mapped differently across indices. * Used during mapping discovery only. @@ -55,7 +54,7 @@ private InvalidMappedField(String name, String errorMessage, Map types() { @@ -64,7 +63,7 @@ public Set types() { @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); + ((PlanStreamOutput) out).writeCachedString(getName()); out.writeString(errorMessage); out.writeMap(getProperties(), (o, x) -> x.writeTo(out)); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/KeywordEsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/KeywordEsField.java index 8b88884a0ce17..48995bafec451 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/KeywordEsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/KeywordEsField.java @@ -8,6 +8,8 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.Collections; @@ -15,8 +17,6 @@ import java.util.Objects; import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; /** * Information about a field in an ES index with the {@code keyword} type. @@ -61,7 +61,7 @@ protected KeywordEsField( public KeywordEsField(StreamInput in) throws IOException { this( - readCachedStringWithVersionCheck(in), + ((PlanStreamInput) in).readCachedString(), KEYWORD, in.readImmutableMap(EsField::readFrom), in.readBoolean(), @@ -73,7 +73,7 @@ public KeywordEsField(StreamInput in) throws IOException { @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); + ((PlanStreamOutput) out).writeCachedString(getName()); out.writeMap(getProperties(), (o, x) -> x.writeTo(out)); out.writeBoolean(isAggregatable()); out.writeInt(precision); diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/MultiTypeEsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/MultiTypeEsField.java index 0d7f9ee425d6a..522cb682c0943 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/MultiTypeEsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/MultiTypeEsField.java @@ -10,6 +10,8 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.HashMap; @@ -17,9 +19,6 @@ import java.util.Objects; import java.util.Set; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * During IndexResolution it could occur that the same field is mapped to different types in different indices. * The class MultiTypeEfField.UnresolvedField holds that information and allows for later resolution of the field @@ -40,7 +39,7 @@ public MultiTypeEsField(String name, DataType dataType, boolean aggregatable, Ma protected MultiTypeEsField(StreamInput in) throws IOException { this( - readCachedStringWithVersionCheck(in), + ((PlanStreamInput) in).readCachedString(), DataType.readFrom(in), in.readBoolean(), in.readImmutableMap(i -> i.readNamedWriteable(Expression.class)) @@ -49,7 +48,7 @@ protected MultiTypeEsField(StreamInput in) throws IOException { @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); + ((PlanStreamOutput) out).writeCachedString(getName()); getDataType().writeTo(out); out.writeBoolean(isAggregatable()); out.writeMap(getIndexToConversionExpressions(), (o, v) -> out.writeNamedWriteable(v)); diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/TextEsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/TextEsField.java index ed0d32a7696eb..c6c494ef289bb 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/TextEsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/TextEsField.java @@ -10,6 +10,8 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Tuple; import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.Map; @@ -17,8 +19,6 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; /** * Information about a field in an es index with the {@code text} type. @@ -34,12 +34,12 @@ public TextEsField(String name, Map properties, boolean hasDocV } protected TextEsField(StreamInput in) throws IOException { - this(readCachedStringWithVersionCheck(in), in.readImmutableMap(EsField::readFrom), in.readBoolean(), in.readBoolean()); + this(((PlanStreamInput) in).readCachedString(), in.readImmutableMap(EsField::readFrom), in.readBoolean(), in.readBoolean()); } @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); + ((PlanStreamOutput) out).writeCachedString(getName()); out.writeMap(getProperties(), (o, x) -> x.writeTo(out)); out.writeBoolean(isAggregatable()); out.writeBoolean(isAlias()); diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/UnsupportedEsField.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/UnsupportedEsField.java index 02ce741243c20..980620cb98847 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/UnsupportedEsField.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/UnsupportedEsField.java @@ -8,15 +8,14 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamInput; +import org.elasticsearch.xpack.esql.core.util.PlanStreamOutput; import java.io.IOException; import java.util.Map; import java.util.Objects; import java.util.TreeMap; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Information about a field in an ES index that cannot be supported by ESQL. * All the subfields (properties) of an unsupported type are also be unsupported. @@ -38,8 +37,8 @@ public UnsupportedEsField(String name, String originalType, String inherited, Ma public UnsupportedEsField(StreamInput in) throws IOException { this( - readCachedStringWithVersionCheck(in), - readCachedStringWithVersionCheck(in), + ((PlanStreamInput) in).readCachedString(), + ((PlanStreamInput) in).readCachedString(), in.readOptionalString(), in.readImmutableMap(EsField::readFrom) ); @@ -47,8 +46,8 @@ public UnsupportedEsField(StreamInput in) throws IOException { @Override public void writeContent(StreamOutput out) throws IOException { - writeCachedStringWithVersionCheck(out, getName()); - writeCachedStringWithVersionCheck(out, getOriginalType()); + ((PlanStreamOutput) out).writeCachedString(getName()); + ((PlanStreamOutput) out).writeCachedString(getOriginalType()); out.writeOptionalString(getInherited()); out.writeMap(getProperties(), (o, x) -> x.writeTo(out)); } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamInput.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamInput.java index e8ccae3429001..826b0cbfa3498 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamInput.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamInput.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.core.util; -import org.elasticsearch.TransportVersions; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.xpack.esql.core.expression.Attribute; @@ -50,13 +49,4 @@ public interface PlanStreamInput { A readEsFieldWithCache() throws IOException; String readCachedString() throws IOException; - - static String readCachedStringWithVersionCheck(StreamInput planStreamInput) throws IOException { - if (planStreamInput.getTransportVersion().before(TransportVersions.ESQL_CACHED_STRING_SERIALIZATION)) { - return planStreamInput.readString(); - } - return ((PlanStreamInput) planStreamInput).readCachedString(); - } - - String readOptionalCachedString() throws IOException; } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamOutput.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamOutput.java index fb4af33d2fd60..e4797411c3796 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamOutput.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/PlanStreamOutput.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.esql.core.util; -import org.elasticsearch.TransportVersions; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.type.EsField; @@ -35,14 +33,4 @@ public interface PlanStreamOutput { boolean writeEsFieldCacheHeader(EsField field) throws IOException; void writeCachedString(String field) throws IOException; - - static void writeCachedStringWithVersionCheck(StreamOutput planStreamOutput, String string) throws IOException { - if (planStreamOutput.getTransportVersion().before(TransportVersions.ESQL_CACHED_STRING_SERIALIZATION)) { - planStreamOutput.writeString(string); - } else { - ((PlanStreamOutput) planStreamOutput).writeCachedString(string); - } - } - - void writeOptionalCachedString(String str) throws IOException; } diff --git a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/FieldAttributeTestUtils.java b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/FieldAttributeTestUtils.java index c7e5056ed0267..1662b7f973c9d 100644 --- a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/FieldAttributeTestUtils.java +++ b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/FieldAttributeTestUtils.java @@ -12,9 +12,9 @@ import org.elasticsearch.xpack.esql.core.type.EsField; public class FieldAttributeTestUtils { - public static FieldAttribute newFieldAttributeWithType( + public static final FieldAttribute newFieldAttributeWithType( Source source, - String parentName, + FieldAttribute parent, String name, DataType type, EsField field, @@ -22,6 +22,6 @@ public static FieldAttribute newFieldAttributeWithType( NameId id, boolean synthetic ) { - return new FieldAttribute(source, parentName, name, type, field, nullability, id, synthetic); + return new FieldAttribute(source, parent, name, type, field, nullability, id, synthetic); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index b18f58b0a43cb..fe7b945a9b3c1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -228,13 +228,13 @@ public static List mappingAsAttributes(Source source, Map list, Source source, String parentName, Map mapping) { + private static void mappingAsAttributes(List list, Source source, FieldAttribute parent, Map mapping) { for (Map.Entry entry : mapping.entrySet()) { String name = entry.getKey(); EsField t = entry.getValue(); if (t != null) { - name = parentName == null ? name : parentName + "." + name; + name = parent == null ? name : parent.fieldName() + "." + name; var fieldProperties = t.getProperties(); var type = t.getDataType().widenSmallNumeric(); // due to a bug also copy the field since the Attribute hierarchy extracts the data type @@ -245,14 +245,14 @@ private static void mappingAsAttributes(List list, Source source, Str FieldAttribute attribute = t instanceof UnsupportedEsField uef ? new UnsupportedAttribute(source, name, uef) - : new FieldAttribute(source, parentName, name, t); + : new FieldAttribute(source, parent, name, t); // primitive branch if (DataType.isPrimitive(type)) { list.add(attribute); } // allow compound object even if they are unknown if (fieldProperties.isEmpty() == false) { - mappingAsAttributes(list, source, attribute.name(), fieldProperties); + mappingAsAttributes(list, source, attribute, fieldProperties); } } } @@ -1252,7 +1252,7 @@ private Expression createIfDoesNotAlreadyExist( // NOTE: The name has to start with $$ to not break bwc with 8.15 - in that version, this is how we had to mark this as // synthetic to work around a bug. String unionTypedFieldName = Attribute.rawTemporaryName(fa.name(), "converted_to", resolvedField.getDataType().typeName()); - FieldAttribute unionFieldAttribute = new FieldAttribute(fa.source(), fa.parentName(), unionTypedFieldName, resolvedField, true); + FieldAttribute unionFieldAttribute = new FieldAttribute(fa.source(), fa.parent(), unionTypedFieldName, resolvedField, true); int existingIndex = unionFieldAttributes.indexOf(unionFieldAttribute); if (existingIndex >= 0) { // Do not generate multiple name/type combinations with different IDs @@ -1281,7 +1281,7 @@ private Expression typeSpecificConvert(AbstractConvertFunction convert, Source s FieldAttribute originalFieldAttr = (FieldAttribute) convert.field(); FieldAttribute resolvedAttr = new FieldAttribute( source, - originalFieldAttr.parentName(), + originalFieldAttr.parent(), originalFieldAttr.name(), field, originalFieldAttr.nullable(), diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java index d372eddb961ae..2c709de7717ce 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/UnsupportedAttribute.java @@ -11,7 +11,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.core.Nullable; import org.elasticsearch.xpack.esql.core.capabilities.Unresolvable; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.expression.Expression; @@ -30,9 +29,6 @@ import java.io.IOException; import java.util.Objects; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * Unsupported attribute meaning an attribute that has been found yet cannot be used (hence why UnresolvedAttribute * cannot be used) expect in special conditions (currently only in projections to allow it to flow through @@ -67,11 +63,11 @@ public UnsupportedAttribute(Source source, String name, UnsupportedEsField field this(source, name, field, null); } - public UnsupportedAttribute(Source source, String name, UnsupportedEsField field, @Nullable String customMessage) { + public UnsupportedAttribute(Source source, String name, UnsupportedEsField field, String customMessage) { this(source, name, field, customMessage, null); } - public UnsupportedAttribute(Source source, String name, UnsupportedEsField field, @Nullable String customMessage, @Nullable NameId id) { + public UnsupportedAttribute(Source source, String name, UnsupportedEsField field, String customMessage, NameId id) { super(source, null, name, field, Nullability.TRUE, id, false); this.hasCustomMessage = customMessage != null; this.message = customMessage == null ? errorMessage(name(), field) : customMessage; @@ -80,7 +76,7 @@ public UnsupportedAttribute(Source source, String name, UnsupportedEsField field private UnsupportedAttribute(StreamInput in) throws IOException { this( Source.readFrom((PlanStreamInput) in), - readCachedStringWithVersionCheck(in), + ((PlanStreamInput) in).readCachedString(), in.getTransportVersion().onOrAfter(TransportVersions.ESQL_ES_FIELD_CACHED_SERIALIZATION) || in.getTransportVersion().isPatchFrom(TransportVersions.V_8_15_2) ? EsField.readFrom(in) : new UnsupportedEsField(in), in.readOptionalString(), @@ -92,7 +88,7 @@ private UnsupportedAttribute(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { if (((PlanStreamOutput) out).writeAttributeCacheHeader(this)) { Source.EMPTY.writeTo(out); - writeCachedStringWithVersionCheck(out, name()); + ((PlanStreamOutput) out).writeCachedString(name()); if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_ES_FIELD_CACHED_SERIALIZATION) || out.getTransportVersion().isPatchFrom(TransportVersions.V_8_15_2)) { field().writeTo(out); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamInput.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamInput.java index 1e1cc3b86a9d5..9003cbec12d1e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamInput.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamInput.java @@ -37,8 +37,6 @@ import java.util.Map; import java.util.function.LongFunction; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamInput.readCachedStringWithVersionCheck; - /** * A customized stream input used to deserialize ESQL physical plan fragments. Complements stream * input with methods that read plan nodes, Attributes, Expressions, etc. @@ -226,7 +224,7 @@ public A readEsFieldWithCache() throws IOException { // it's safe to cast to int, since the max value for this is {@link PlanStreamOutput#MAX_SERIALIZED_ATTRIBUTES} int cacheId = Math.toIntExact(readZLong()); if (cacheId < 0) { - String className = readCachedStringWithVersionCheck(this); + String className = readCachedString(); Writeable.Reader reader = EsField.getReader(className); cacheId = -1 - cacheId; EsField result = reader.read(this); @@ -236,7 +234,7 @@ public A readEsFieldWithCache() throws IOException { return (A) esFieldFromCache(cacheId); } } else { - String className = readCachedStringWithVersionCheck(this); + String className = readCachedString(); Writeable.Reader reader = EsField.getReader(className); return (A) reader.read(this); } @@ -247,6 +245,9 @@ public A readEsFieldWithCache() throws IOException { */ @Override public String readCachedString() throws IOException { + if (getTransportVersion().before(TransportVersions.ESQL_CACHED_STRING_SERIALIZATION)) { + return readString(); + } int cacheId = Math.toIntExact(readZLong()); if (cacheId < 0) { String string = readString(); @@ -258,11 +259,6 @@ public String readCachedString() throws IOException { } } - @Override - public String readOptionalCachedString() throws IOException { - return readBoolean() ? readCachedString() : null; - } - private EsField esFieldFromCache(int id) throws IOException { EsField field = esFieldsCache[id]; if (field == null) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutput.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutput.java index 615c4266620c7..b633b10122eb3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutput.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutput.java @@ -30,8 +30,6 @@ import java.util.IdentityHashMap; import java.util.Map; -import static org.elasticsearch.xpack.esql.core.util.PlanStreamOutput.writeCachedStringWithVersionCheck; - /** * A customized stream output used to serialize ESQL physical plan fragments. Complements stream * output with methods that write plan nodes, Attributes, Expressions, etc. @@ -197,7 +195,7 @@ public boolean writeEsFieldCacheHeader(EsField field) throws IOException { cacheId = cacheEsField(field); writeZLong(-1 - cacheId); } - writeCachedStringWithVersionCheck(this, field.getWriteableName()); + writeCachedString(field.getWriteableName()); return true; } @@ -209,6 +207,10 @@ public boolean writeEsFieldCacheHeader(EsField field) throws IOException { */ @Override public void writeCachedString(String string) throws IOException { + if (getTransportVersion().before(TransportVersions.ESQL_CACHED_STRING_SERIALIZATION)) { + writeString(string); + return; + } Integer cacheId = stringCache.get(string); if (cacheId != null) { writeZLong(cacheId); @@ -224,16 +226,6 @@ public void writeCachedString(String string) throws IOException { writeString(string); } - @Override - public void writeOptionalCachedString(String str) throws IOException { - if (str == null) { - writeBoolean(false); - } else { - writeBoolean(true); - writeCachedString(str); - } - } - private Integer esFieldIdFromCache(EsField field) { return cachedEsFields.get(field); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsRelation.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsRelation.java index eb72009638396..951fc7ad1cf29 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsRelation.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsRelation.java @@ -112,12 +112,7 @@ private static List flatten(Source source, Map mappi EsField t = entry.getValue(); if (t != null) { - FieldAttribute f = new FieldAttribute( - source, - parent != null ? parent.name() : null, - parent != null ? parent.name() + "." + name : name, - t - ); + FieldAttribute f = new FieldAttribute(source, parent, parent != null ? parent.name() + "." + name : name, t); list.add(f); // object or nested if (t.getProperties().isEmpty() == false) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FieldAttributeTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FieldAttributeTests.java index 6b2040f58f84c..e8f0333791844 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FieldAttributeTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/FieldAttributeTests.java @@ -20,7 +20,7 @@ public class FieldAttributeTests extends AbstractAttributeTestCase { public static FieldAttribute createFieldAttribute(int maxDepth, boolean onlyRepresentable) { Source source = Source.EMPTY; - String parentName = maxDepth == 0 || randomBoolean() ? null : randomAlphaOfLength(3); + FieldAttribute parent = maxDepth == 0 || randomBoolean() ? null : createFieldAttribute(maxDepth - 1, onlyRepresentable); String name = randomAlphaOfLength(5); DataType type = onlyRepresentable ? randomValueOtherThanMany(t -> false == DataType.isRepresentable(t), () -> randomFrom(DataType.types())) @@ -28,7 +28,7 @@ public static FieldAttribute createFieldAttribute(int maxDepth, boolean onlyRepr EsField field = AbstractEsFieldTypeTests.randomAnyEsField(maxDepth); Nullability nullability = randomFrom(Nullability.values()); boolean synthetic = randomBoolean(); - return newFieldAttributeWithType(source, parentName, name, type, field, nullability, new NameId(), synthetic); + return newFieldAttributeWithType(source, parent, name, type, field, nullability, new NameId(), synthetic); } @Override @@ -39,20 +39,20 @@ protected FieldAttribute create() { @Override protected FieldAttribute mutate(FieldAttribute instance) { Source source = instance.source(); - String parentName = instance.parentName(); + FieldAttribute parent = instance.parent(); String name = instance.name(); DataType type = instance.dataType(); EsField field = instance.field(); Nullability nullability = instance.nullable(); boolean synthetic = instance.synthetic(); switch (between(0, 5)) { - case 0 -> parentName = randomValueOtherThan(parentName, () -> randomBoolean() ? null : randomAlphaOfLength(2)); + case 0 -> parent = randomValueOtherThan(parent, () -> randomBoolean() ? null : createFieldAttribute(2, false)); case 1 -> name = randomAlphaOfLength(name.length() + 1); case 2 -> type = randomValueOtherThan(type, () -> randomFrom(DataType.types())); case 3 -> field = randomValueOtherThan(field, () -> AbstractEsFieldTypeTests.randomAnyEsField(3)); case 4 -> nullability = randomValueOtherThan(nullability, () -> randomFrom(Nullability.values())); case 5 -> synthetic = false == synthetic; } - return newFieldAttributeWithType(source, parentName, name, type, field, nullability, new NameId(), synthetic); + return newFieldAttributeWithType(source, parent, name, type, field, nullability, new NameId(), synthetic); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/index/EsIndexSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/index/EsIndexSerializationTests.java index 82dd5a88ffaf1..687b83370f571 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/index/EsIndexSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/index/EsIndexSerializationTests.java @@ -182,51 +182,4 @@ private void testManyTypeConflicts(boolean withParent, ByteSizeValue expected) t assertThat(ByteSizeValue.ofBytes(out.bytes().length()), byteSizeEquals(expected)); } } - - public static EsIndex deeplyNestedIndex(int depth, int childrenPerLevel) { - String rootFieldName = "root"; - Map fields = Map.of(rootFieldName, fieldWithRecursiveChildren(depth, childrenPerLevel, rootFieldName)); - - return new EsIndex("deeply-nested", fields); - } - - private static EsField fieldWithRecursiveChildren(int depth, int childrenPerLevel, String name) { - assert depth >= 1; - - Map children = new TreeMap<>(); - String childName; - if (depth == 1) { - for (int i = 0; i < childrenPerLevel; i++) { - childName = "leaf" + i; - children.put(childName, new EsField(childName, DataType.KEYWORD, Map.of(), true)); - } - } else { - for (int i = 0; i < childrenPerLevel; i++) { - childName = "level" + depth + "child" + i; - children.put(childName, fieldWithRecursiveChildren(depth - 1, childrenPerLevel, childName)); - } - } - - return new EsField(name, DataType.OBJECT, children, false); - } - - /** - * Test de-/serialization and size on the wire for an index that has multiple levels of children: - * A single root with 9 children, each of which has 9 children etc. 6 levels deep. - */ - public void testDeeplyNestedFields() throws IOException { - ByteSizeValue expectedSize = ByteSizeValue.ofBytes(9425494); - /* - * History: - * 9425494b - string serialization #112929 - */ - - int depth = 6; - int childrenPerLevel = 9; - - try (BytesStreamOutput out = new BytesStreamOutput(); var pso = new PlanStreamOutput(out, null)) { - deeplyNestedIndex(depth, childrenPerLevel).writeTo(pso); - assertThat(ByteSizeValue.ofBytes(out.bytes().length()), byteSizeEquals(expectedSize)); - } - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutputTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutputTests.java index d3e1710a715af..33252b9dbaaa3 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutputTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanStreamOutputTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.xpack.esql.Column; import org.elasticsearch.xpack.esql.core.InvalidArgumentException; import org.elasticsearch.xpack.esql.core.expression.Attribute; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; import org.elasticsearch.xpack.esql.core.expression.NameId; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.type.EsField; @@ -43,6 +44,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; public class PlanStreamOutputTests extends ESTestCase { @@ -116,13 +118,26 @@ public void testWriteAttributeMultipleTimes() throws IOException { for (int i = 0; i < occurrences; i++) { planStream.writeNamedWriteable(attribute); } - assertThat(planStream.cachedAttributes.size(), is(1)); + int depth = 0; + Attribute parent = attribute; + while (parent != null) { + depth++; + parent = parent instanceof FieldAttribute f ? f.parent() : null; + } + assertThat(planStream.cachedAttributes.size(), is(depth)); try (PlanStreamInput in = new PlanStreamInput(out.bytes().streamInput(), REGISTRY, configuration)) { Attribute first = in.readNamedWriteable(Attribute.class); for (int i = 1; i < occurrences; i++) { Attribute next = in.readNamedWriteable(Attribute.class); assertThat(first, sameInstance(next)); } + for (int i = 0; i < depth; i++) { + assertThat(first, equalTo(attribute)); + first = first instanceof FieldAttribute f ? f.parent() : null; + attribute = attribute instanceof FieldAttribute f ? f.parent() : null; + } + assertThat(first, is(nullValue())); + assertThat(attribute, is(nullValue())); } } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/ExchangeSinkExecSerializationTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/ExchangeSinkExecSerializationTests.java index 5989c0de6b61d..1f52795dbacd7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/ExchangeSinkExecSerializationTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/physical/ExchangeSinkExecSerializationTests.java @@ -80,67 +80,20 @@ public void testManyTypeConflicts() throws IOException { * See {@link #testManyTypeConflicts(boolean, ByteSizeValue)} for more. */ public void testManyTypeConflictsWithParent() throws IOException { - testManyTypeConflicts(true, ByteSizeValue.ofBytes(2774192)); + testManyTypeConflicts(true, ByteSizeValue.ofBytes(2774214)); /* * History: * 2 gb+ - start * 43.3mb - Cache attribute subclasses #111447 * 5.6mb - shorten error messages for UnsupportedAttributes #111973 * 3.1mb - cache EsFields #112008 - * 2774214b - string serialization #112929 - * 2774192b - remove field attribute #112881 + * 2.6mb - string serialization #112929 */ } - private void testManyTypeConflicts(boolean withParent, ByteSizeValue expected) throws IOException { - EsIndex index = EsIndexSerializationTests.indexWithManyConflicts(withParent); - testSerializePlanWithIndex(index, expected); - } - - /** - * Test the size of serializing a plan like - * FROM index | LIMIT 10 - * with a single root field that has many children, grandchildren etc. - */ - public void testDeeplyNestedFields() throws IOException { - ByteSizeValue expected = ByteSizeValue.ofBytes(47252411); - /* - * History: - * 48223371b - string serialization #112929 - * 47252411b - remove field attribute #112881 - */ - - int depth = 6; - int childrenPerLevel = 8; - - EsIndex index = EsIndexSerializationTests.deeplyNestedIndex(depth, childrenPerLevel); - testSerializePlanWithIndex(index, expected); - } - /** - * Test the size of serializing a plan like - * FROM index | LIMIT 10 | KEEP one_single_field - * with a single root field that has many children, grandchildren etc. - */ - public void testDeeplyNestedFieldsKeepOnlyOne() throws IOException { - ByteSizeValue expected = ByteSizeValue.ofBytes(9425806); - /* - * History: - * 9426058b - string serialization #112929 - * 9425806b - remove field attribute #112881 - */ - - int depth = 6; - int childrenPerLevel = 9; - - EsIndex index = EsIndexSerializationTests.deeplyNestedIndex(depth, childrenPerLevel); - testSerializePlanWithIndex(index, expected, false); - } - - /** - * Test the size of serializing the physical plan that will be sent to a data node. - * The plan corresponds to `FROM index | LIMIT 10`. - * Callers of this method intentionally use a very precise size for the serialized + * Test the size of serializing a plan with many conflicts. Callers of + * this method intentionally use a very precise size for the serialized * data so a programmer making changes has to think when this size changes. *

* In general, shrinking the over the wire size is great and the precise @@ -155,14 +108,10 @@ public void testDeeplyNestedFieldsKeepOnlyOne() throws IOException { * ESQL impossible to use at all for big mappings with many conflicts. *

*/ - private void testSerializePlanWithIndex(EsIndex index, ByteSizeValue expected) throws IOException { - testSerializePlanWithIndex(index, expected, true); - } - - private void testSerializePlanWithIndex(EsIndex index, ByteSizeValue expected, boolean keepAllFields) throws IOException { - List allAttributes = Analyzer.mappingAsAttributes(randomSource(), index.mapping()); - List keepAttributes = keepAllFields ? allAttributes : List.of(allAttributes.get(0)); - EsRelation relation = new EsRelation(randomSource(), index, keepAttributes, IndexMode.STANDARD); + private void testManyTypeConflicts(boolean withParent, ByteSizeValue expected) throws IOException { + EsIndex index = EsIndexSerializationTests.indexWithManyConflicts(withParent); + List attributes = Analyzer.mappingAsAttributes(randomSource(), index.mapping()); + EsRelation relation = new EsRelation(randomSource(), index, attributes, IndexMode.STANDARD); Limit limit = new Limit(randomSource(), new Literal(randomSource(), 10, DataType.INTEGER), relation); Project project = new Project(randomSource(), limit, limit.output()); FragmentExec fragmentExec = new FragmentExec(project);