Revert "ESQL: Remove parent from FieldAttribute (#112881)" (#115006)

This reverts commit caa16b4.

Author: alex-spies

docs/changelog/112881.yaml

@@ -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,

server/src/main/java/org/elasticsearch/TransportVersions.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);
     }
 

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Alias.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<NamedWriteableRegistry.Entry> 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;
     }

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.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,49 +145,22 @@ 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;
     }
 
     @Override
     protected NodeInfo<FieldAttribute> 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);
     }
 

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/FieldAttribute.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
     ) {

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/MetadataAttribute.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<NamedWriteableRegistry.Entry> getNamedWriteables() {
     private final NameId id;
     private final boolean synthetic;
 
-    public NamedExpression(Source source, String name, List<Expression> children, @Nullable NameId id) {
+    public NamedExpression(Source source, String name, List<Expression> children, NameId id) {
         this(source, name, children, id, false);
     }
 
-    public NamedExpression(Source source, String name, List<Expression> children, @Nullable NameId id, boolean synthetic) {
+    public NamedExpression(Source source, String name, List<Expression> children, NameId id, boolean synthetic) {
         super(source, children);
         this.name = name;
         this.id = id == null ? new NameId() : id;

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/NamedExpression.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);
     }