Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.xpack.esql.core.expression.Expressions;
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.Literal;
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
import org.elasticsearch.xpack.esql.core.expression.NameId;
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
Expand Down Expand Up @@ -282,7 +283,9 @@ private static LinkedHashSet<UnresolvedAttribute> unresolvedLinkedSet(List<Unres
public static List<UnresolvedAttribute> collectUnresolved(LogicalPlan plan) {
List<UnresolvedAttribute> unresolved = new ArrayList<>();
Consumer<UnresolvedAttribute> collectUnresolved = ua -> {
if ((ua instanceof UnresolvedPattern || ua instanceof UnresolvedTimestamp) == false) {
// Exclude metadata fields so they fail with a proper verification error instead of being silently nullified/loaded.
if ((ua instanceof UnresolvedPattern || ua instanceof UnresolvedTimestamp) == false
&& MetadataAttribute.isSupported(ua.name()) == false) {
unresolved.add(ua);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
import org.elasticsearch.xpack.esql.core.expression.Literal;
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
import org.elasticsearch.xpack.esql.core.expression.UnresolvedTimestamp;
import org.elasticsearch.xpack.esql.core.type.DataType;
Expand Down Expand Up @@ -62,6 +63,7 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
Expand Down Expand Up @@ -3303,6 +3305,141 @@ public void testRow() {
assertThat(Expressions.name(row.fields().getFirst()), is("x"));
}

public void testFailMetadataFieldInKeep() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice, I'd suggest a couple more complex query shapes though.

For nullify, we don't disallow FORK, LOOKUP JOIN, subqueries, etc.

I'd also suggest trying to refer to the missing metadata field after (not inside) pipeline breakers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Added a few

for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | KEEP " + field;
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldInEval() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | EVAL x = " + field;
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldInWhere() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | WHERE " + field + " IS NOT NULL";
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldInSort() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | SORT " + field;
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldInStats() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | STATS x = COUNT(" + field + ")";
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldInRename() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var query = "FROM test | RENAME " + field + " AS renamed";
var failure = "Unknown column [" + field + "]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}
}

public void testFailMetadataFieldAfterStats() {
var query = """
FROM test
| STATS c = COUNT(*)
| KEEP _score
""";
var failure = "Unknown column [_score]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}

public void testFailMetadataFieldInFork() {
var query = """
FROM test
| FORK (WHERE _score > 1)
(WHERE salary > 50000)
""";
var failure = "Unknown column [_score]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}

public void testFailMetadataFieldInSubquery() {
assumeTrue("Requires subquery in FROM command support", EsqlCapabilities.Cap.SUBQUERY_IN_FROM_COMMAND.isEnabled());

var query = """
FROM
(FROM test
| WHERE _score > 1)
""";
var failure = "Unknown column [_score]";
verificationFailure(setUnmappedNullify(query), failure);
verificationFailure(setUnmappedLoad(query), failure);
}

/*
* Limit[1000[INTEGER],false,false]
* \_Project[[_score{m}#5]]
* \_EsRelation[test][_meta_field{f}#11, emp_no{f}#5, ...]
*/
public void testMetadataFieldDeclaredNullify() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var plan = analyzeStatement(setUnmappedNullify("FROM test METADATA " + field + " | KEEP " + field));

var limit = as(plan, Limit.class);
assertThat(limit.limit().fold(FoldContext.small()), is(1000));

var project = as(limit.child(), Project.class);
assertThat(project.projections(), hasSize(1));
assertThat(Expressions.name(project.projections().getFirst()), is(field));
assertThat(project.projections().getFirst(), instanceOf(MetadataAttribute.class));

// No Eval(NULL) — the field was resolved via METADATA, not nullified
var relation = as(project.child(), EsRelation.class);
assertThat(relation.indexPattern(), is("test"));
}
}

/*
* Limit[1000[INTEGER],false,false]
* \_Project[[_score{m}#5]]
* \_EsRelation[test][_meta_field{f}#11, emp_no{f}#5, ...]
*/
public void testMetadataFieldDeclaredLoad() {
for (String field : MetadataAttribute.ATTRIBUTES_MAP.keySet()) {
var plan = analyzeStatement(setUnmappedLoad("FROM test METADATA " + field + " | KEEP " + field));

var limit = as(plan, Limit.class);
assertThat(limit.limit().fold(FoldContext.small()), is(1000));

var project = as(limit.child(), Project.class);
assertThat(project.projections(), hasSize(1));
assertThat(Expressions.name(project.projections().getFirst()), is(field));
assertThat(project.projections().getFirst(), instanceOf(MetadataAttribute.class));

// The field was resolved via METADATA, not loaded as an unmapped field into EsRelation
var relation = as(project.child(), EsRelation.class);
assertThat(relation.indexPattern(), is("test"));
}
}

public void testChangedTimestmapFieldWithRate() {
verificationFailure(setUnmappedNullify("""
TS k8s
Expand Down