Reapply "ESQL: Remove parent from FieldAttribute (#112881)" (#115006) (#115007) (#115035)

This reverts commit 17ecb66 and
reapplies #112881 once the
previous, non-backported transport version bump is dealt with.

Author: alex-spies

docs/changelog/112881.yaml

@@ -0,0 +1,5 @@
+pr: 112881
+summary: "ESQL: Remove parent from `FieldAttribute`"
+area: ES|QL
+type: enhancement
+issues: []

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

@@ -176,6 +176,7 @@ static TransportVersion def(int id) {
     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 REVERT_REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_774_00_0);
+    public static final TransportVersion ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED = def(8_775_00_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

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

@@ -9,6 +9,7 @@
 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;
@@ -42,11 +43,11 @@ public Alias(Source source, String name, Expression child) {
         this(source, name, child, null);
     }
 
-    public Alias(Source source, String name, Expression child, NameId id) {
+    public Alias(Source source, String name, Expression child, @Nullable NameId id) {
         this(source, name, child, id, false);
     }
 
-    public Alias(Source source, String name, Expression child, NameId id, boolean synthetic) {
+    public Alias(Source source, String name, Expression child, @Nullable NameId id, boolean synthetic) {
         super(source, name, singletonList(child), id, synthetic);
         this.child = child;
     }
@@ -55,7 +56,7 @@ public Alias(Source source, String name, Expression child, NameId id, boolean sy
     /**
      * 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, NameId id, boolean synthetic) {
+    private Alias(Source source, String name, String qualifier, Expression child, @Nullable NameId id, boolean synthetic) {
         this(source, name, child, id, synthetic);
     }
 

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

@@ -7,6 +7,7 @@
 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;
 
@@ -41,15 +42,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, NameId id) {
+    public Attribute(Source source, String name, @Nullable NameId id) {
         this(source, name, Nullability.TRUE, id);
     }
 
-    public Attribute(Source source, String name, Nullability nullability, NameId id) {
+    public Attribute(Source source, String name, Nullability nullability, @Nullable NameId id) {
         this(source, name, nullability, id, false);
     }
 
-    public Attribute(Source source, String name, Nullability nullability, NameId id, boolean synthetic) {
+    public Attribute(Source source, String name, Nullability nullability, @Nullable 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/FieldAttribute.java

@@ -6,21 +6,25 @@
  */
 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:
@@ -37,32 +41,31 @@ public class FieldAttribute extends TypedAttribute {
         FieldAttribute::readFrom
     );
 
-    private final FieldAttribute parent;
-    private final String path;
+    private final String parentName;
     private final EsField field;
 
     public FieldAttribute(Source source, String name, EsField field) {
         this(source, null, name, field);
     }
 
-    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) {
+        this(source, parentName, name, field, Nullability.TRUE, null, false);
     }
 
-    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, String name, EsField field, boolean synthetic) {
+        this(source, parentName, name, field, Nullability.TRUE, null, synthetic);
     }
 
     public FieldAttribute(
         Source source,
-        FieldAttribute parent,
+        @Nullable String parentName,
         String name,
         EsField field,
         Nullability nullability,
-        NameId id,
+        @Nullable NameId id,
         boolean synthetic
     ) {
-        this(source, parent, name, field.getDataType(), field, nullability, id, synthetic);
+        this(source, parentName, name, field.getDataType(), field, nullability, id, synthetic);
     }
 
     /**
@@ -71,17 +74,16 @@ public FieldAttribute(
      */
     FieldAttribute(
         Source source,
-        FieldAttribute parent,
+        @Nullable String parentName,
         String name,
         DataType type,
         EsField field,
         Nullability nullability,
-        NameId id,
+        @Nullable NameId id,
         boolean synthetic
     ) {
         super(source, name, type, nullability, id, synthetic);
-        this.path = parent != null ? parent.name() : StringUtils.EMPTY;
-        this.parent = parent;
+        this.parentName = parentName;
         this.field = field;
     }
 
@@ -91,16 +93,16 @@ public FieldAttribute(
      */
     private FieldAttribute(
         Source source,
-        FieldAttribute parent,
+        @Nullable String parentName,
         String name,
         DataType type,
         EsField field,
-        String qualifier,
+        @Nullable String qualifier,
         Nullability nullability,
-        NameId id,
+        @Nullable NameId id,
         boolean synthetic
     ) {
-        this(source, parent, name, type, field, nullability, id, synthetic);
+        this(source, parentName, name, type, field, nullability, id, synthetic);
     }
 
     private FieldAttribute(StreamInput in) throws IOException {
@@ -114,8 +116,8 @@ private FieldAttribute(StreamInput in) throws IOException {
          */
         this(
             Source.readFrom((StreamInput & PlanStreamInput) in),
-            in.readOptionalWriteable(FieldAttribute::readFrom),
-            ((PlanStreamInput) in).readCachedString(),
+            readParentName(in),
+            readCachedStringWithVersionCheck(in),
             DataType.readFrom(in),
             EsField.readFrom(in),
             in.readOptionalString(),
@@ -129,8 +131,8 @@ private FieldAttribute(StreamInput in) throws IOException {
     public void writeTo(StreamOutput out) throws IOException {
         if (((PlanStreamOutput) out).writeAttributeCacheHeader(this)) {
             Source.EMPTY.writeTo(out);
-            out.writeOptionalWriteable(parent);
-            ((PlanStreamOutput) out).writeCachedString(name());
+            writeParentName(out);
+            writeCachedStringWithVersionCheck(out, name());
             dataType().writeTo(out);
             field.writeTo(out);
             // We used to write the qualifier here. We can still do if needed in the future.
@@ -145,22 +147,49 @@ 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, parent, name(), dataType(), field, (String) null, nullable(), id(), synthetic());
-    }
-
-    public FieldAttribute parent() {
-        return parent;
+        return NodeInfo.create(
+            this,
+            FieldAttribute::new,
+            parentName,
+            name(),
+            dataType(),
+            field,
+            (String) null,
+            nullable(),
+            id(),
+            synthetic()
+        );
     }
 
-    public String path() {
-        return path;
+    public String parentName() {
+        return parentName;
     }
 
     /**
@@ -174,7 +203,7 @@ public String fieldName() {
         if ((synthetic() || name().startsWith(SYNTHETIC_ATTRIBUTE_NAME_PREFIX)) == false) {
             return name();
         }
-        return Strings.hasText(path) ? path + "." + field.getName() : field.getName();
+        return Strings.hasText(parentName) ? parentName + "." + field.getName() : field.getName();
     }
 
     public EsField.Exact getExactInfo() {
@@ -190,13 +219,13 @@ public FieldAttribute exactAttribute() {
     }
 
     private FieldAttribute innerField(EsField type) {
-        return new FieldAttribute(source(), this, name() + "." + type.getName(), type, nullable(), id(), synthetic());
+        return new FieldAttribute(source(), name(), 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, parent, name, field, nullability, id, synthetic);
+        return new FieldAttribute(source, parentName, name, field, nullability, id, synthetic);
     }
 
     @Override
@@ -206,13 +235,13 @@ public Attribute withDataType(DataType type) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), path, field);
+        return Objects.hash(super.hashCode(), parentName, field);
     }
 
     @Override
     public boolean equals(Object obj) {
         return super.equals(obj)
-            && Objects.equals(path, ((FieldAttribute) obj).path)
+            && Objects.equals(parentName, ((FieldAttribute) obj).parentName)
             && Objects.equals(field, ((FieldAttribute) obj).field);
     }
 

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

@@ -10,6 +10,7 @@
 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;
@@ -59,7 +60,7 @@ public MetadataAttribute(
         String name,
         DataType dataType,
         Nullability nullability,
-        NameId id,
+        @Nullable NameId id,
         boolean synthetic,
         boolean searchable
     ) {
@@ -79,9 +80,9 @@ private MetadataAttribute(
         Source source,
         String name,
         DataType dataType,
-        String qualifier,
+        @Nullable String qualifier,
         Nullability nullability,
-        NameId id,
+        @Nullable NameId id,
         boolean synthetic,
         boolean searchable
     ) {

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

@@ -8,6 +8,7 @@
 
 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;
@@ -32,11 +33,11 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
     private final NameId id;
     private final boolean synthetic;
 
-    public NamedExpression(Source source, String name, List<Expression> children, NameId id) {
+    public NamedExpression(Source source, String name, List<Expression> children, @Nullable NameId id) {
         this(source, name, children, id, false);
     }
 
-    public NamedExpression(Source source, String name, List<Expression> children, NameId id, boolean synthetic) {
+    public NamedExpression(Source source, String name, List<Expression> children, @Nullable 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/ReferenceAttribute.java

@@ -9,6 +9,7 @@
 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;
@@ -31,7 +32,14 @@ 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, NameId id, boolean synthetic) {
+    public ReferenceAttribute(
+        Source source,
+        String name,
+        DataType dataType,
+        Nullability nullability,
+        @Nullable NameId id,
+        boolean synthetic
+    ) {
         super(source, name, dataType, nullability, id, synthetic);
     }