ESQL: Fix attribute set equals (#118823)

Also add a test that uses this, for lookup join field attribute ids.
This commit is contained in:
Alexander Spies 2024-12-19 10:22:13 +01:00 committed by GitHub
parent 90f038d802
commit 9cc6cd4229
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 85 additions and 3 deletions

View file

@ -0,0 +1,5 @@
pr: 118823
summary: Fix attribute set equals
area: ES|QL
type: bug
issues: []

View file

@ -174,8 +174,12 @@ public class AttributeSet implements Set<Attribute> {
}
@Override
public boolean equals(Object o) {
return delegate.equals(o);
public boolean equals(Object obj) {
if (obj instanceof AttributeSet as) {
obj = as.delegate;
}
return delegate.equals(obj);
}
@Override

View file

@ -30,7 +30,7 @@ import static org.hamcrest.Matchers.sameInstance;
public class AttributeMapTests extends ESTestCase {
private static Attribute a(String name) {
static Attribute a(String name) {
return new UnresolvedAttribute(Source.EMPTY, name);
}

View file

@ -0,0 +1,42 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.xpack.esql.core.expression;
import org.elasticsearch.test.ESTestCase;
import java.util.List;
import static org.elasticsearch.xpack.esql.core.expression.AttributeMapTests.a;
public class AttributeSetTests extends ESTestCase {
public void testEquals() {
Attribute a1 = a("1");
Attribute a2 = a("2");
AttributeSet first = new AttributeSet(List.of(a1, a2));
assertEquals(first, first);
AttributeSet second = new AttributeSet();
second.add(a1);
second.add(a2);
assertEquals(first, second);
assertEquals(second, first);
AttributeSet third = new AttributeSet();
third.add(a("1"));
third.add(a("2"));
assertNotEquals(first, third);
assertNotEquals(third, first);
assertEquals(AttributeSet.EMPTY, AttributeSet.EMPTY);
assertEquals(AttributeSet.EMPTY, first.intersect(third));
assertEquals(third.intersect(first), AttributeSet.EMPTY);
}
}

View file

@ -20,6 +20,7 @@ import org.elasticsearch.xpack.esql.VerificationException;
import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
import org.elasticsearch.xpack.esql.core.expression.Alias;
import org.elasticsearch.xpack.esql.core.expression.Attribute;
import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
import org.elasticsearch.xpack.esql.core.expression.Expressions;
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.Literal;
@ -2190,6 +2191,36 @@ public class AnalyzerTests extends ESTestCase {
assertThat(e.getMessage(), containsString(errorMessage3 + "right side of join"));
}
public void testMultipleLookupJoinsGiveDifferentAttributes() {
assumeTrue("requires LOOKUP JOIN capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V8.isEnabled());
// The field attributes that get contributed by different LOOKUP JOIN commands must have different name ids,
// even if they have the same names. Otherwise, things like dependency analysis - like in PruneColumns - cannot work based on
// name ids and shadowing semantics proliferate into all kinds of optimizer code.
String query = "FROM test"
+ "| EVAL language_code = languages"
+ "| LOOKUP JOIN languages_lookup ON language_code"
+ "| LOOKUP JOIN languages_lookup ON language_code";
LogicalPlan analyzedPlan = analyze(query);
List<AttributeSet> lookupFields = new ArrayList<>();
List<Set<String>> lookupFieldNames = new ArrayList<>();
analyzedPlan.forEachUp(EsRelation.class, esRelation -> {
if (esRelation.indexMode() == IndexMode.LOOKUP) {
lookupFields.add(esRelation.outputSet());
lookupFieldNames.add(esRelation.outputSet().stream().map(NamedExpression::name).collect(Collectors.toSet()));
}
});
assertEquals(lookupFieldNames.size(), 2);
assertEquals(lookupFieldNames.get(0), lookupFieldNames.get(1));
assertEquals(lookupFields.size(), 2);
AttributeSet intersection = lookupFields.get(0).intersect(lookupFields.get(1));
assertEquals(AttributeSet.EMPTY, intersection);
}
public void testLookupJoinIndexMode() {
assumeTrue("requires LOOKUP JOIN capability", EsqlCapabilities.Cap.JOIN_LOOKUP_V8.isEnabled());