Skip to content

Commit

Permalink
Revert "ESQL: Remove parent from FieldAttribute (elastic#112881)" (el…
Browse files Browse the repository at this point in the history
…astic#115006)

This reverts commit caa16b4.
  • Loading branch information
alex-spies authored and georgewallace committed Oct 25, 2024
1 parent bb3f7f5 commit 5657fee
Show file tree
Hide file tree
Showing 30 changed files with 134 additions and 323 deletions.
5 changes: 0 additions & 5 deletions docs/changelog/112881.yaml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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);
}

/**
Expand All @@ -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;
}

Expand All @@ -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 {
Expand All @@ -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(),
Expand All @@ -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.
Expand All @@ -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;
}

/**
Expand All @@ -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() {
Expand All @@ -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
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,7 +59,7 @@ public MetadataAttribute(
String name,
DataType dataType,
Nullability nullability,
@Nullable NameId id,
NameId id,
boolean synthetic,
boolean searchable
) {
Expand All @@ -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
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down
Loading

0 comments on commit 5657fee

Please sign in to comment.