From ced420e95c97d2f569980a2ba42c7dfa9c065965 Mon Sep 17 00:00:00 2001 From: jihwan Date: Sun, 2 Mar 2025 22:15:55 +0900 Subject: [PATCH 1/2] Improve lookup join error messages in ES|QL --- .../esql/plan/logical/join/LookupJoin.java | 7 ++- .../xpack/esql/plan/logical/JoinTests.java | 58 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java index 5f1f569e3671b..eb24d50716159 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java @@ -89,9 +89,10 @@ public void postAnalysisVerification(Failures failures) { right().forEachDown(EsRelation.class, esr -> { var indexNameWithModes = esr.indexNameWithModes(); if (indexNameWithModes.size() != 1) { - failures.add( - fail(esr, "invalid [{}] resolution in lookup mode to [{}] indices", esr.indexPattern(), indexNameWithModes.size()) - ); + String errorMessage = indexNameWithModes.isEmpty() + ? "Index [" + esr.indexPattern() + "] exists, but no valid fields for LOOKUP JOIN were found" + : "Invalid [" + esr.indexPattern() + "] resolution in lookup mode to [" + indexNameWithModes.size() + "] indices"; + failures.add(fail(esr, errorMessage)); } else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) { failures.add( fail( diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/JoinTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/JoinTests.java index 13887fbd1740c..7867ebc2e4b60 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/JoinTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/plan/logical/JoinTests.java @@ -7,14 +7,21 @@ package org.elasticsearch.xpack.esql.plan.logical; +import java.util.Collections; +import java.util.Map; + +import org.elasticsearch.index.IndexMode; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.common.Failures; 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.Expression; +import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; import org.elasticsearch.xpack.esql.core.expression.Literal; 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.plan.logical.join.Join; import org.elasticsearch.xpack.esql.plan.logical.join.JoinConfig; import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes; @@ -23,6 +30,8 @@ import java.util.List; import java.util.Set; +import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin; + public class JoinTests extends ESTestCase { @AwaitsFix(bugUrl = "Test needs updating to the new JOIN planning") public void testExpressionsAndReferences() { @@ -99,4 +108,53 @@ public void testTransformExprs() { private static Alias aliasForLiteral(String name) { return new Alias(Source.EMPTY, name, new Literal(Source.EMPTY, 1, DataType.INTEGER)); } + + public void testLookupJoinErrorMessage() { + Failures failures = new Failures(); + + String indexPattern = "test1"; + Map indexNameWithModes = Collections.emptyMap(); + + EsRelation fakeRelation = new EsRelation(Source.EMPTY, indexPattern, IndexMode.LOOKUP, indexNameWithModes, List.of()); + + EsField empNoEsField = new EsField("emp_no", DataType.INTEGER, Collections.emptyMap(), false); + FieldAttribute empNoField = new FieldAttribute(Source.EMPTY, "emp_no", empNoEsField); + Alias empNoAlias = new Alias(Source.EMPTY, "emp_no", empNoField); + LogicalPlan left = new Row(Source.EMPTY, List.of(empNoAlias)); + + LookupJoin lookupJoin = new LookupJoin(Source.EMPTY, left, fakeRelation, + new JoinConfig(JoinTypes.LEFT, List.of(), List.of(), List.of())); + + lookupJoin.postAnalysisVerification(failures); + + String expectedMessage = "Index [test1] exists, but no valid fields for LOOKUP JOIN were found"; + assertTrue(failures.toString().contains(expectedMessage)); + } + + public void testLookupJoinErrorMessage_MultipleIndices() { + Failures failures = new Failures(); + + String indexPattern = "test1"; + Map indexNameWithModes = Map.of( + "test1", IndexMode.LOOKUP, + "test2", IndexMode.LOOKUP + ); + + EsField languagesEsField = new EsField("languages", DataType.KEYWORD, Collections.emptyMap(), true); + FieldAttribute languagesField = new FieldAttribute(Source.EMPTY, "languages", languagesEsField); + EsRelation fakeRelation = new EsRelation(Source.EMPTY, indexPattern, IndexMode.LOOKUP, indexNameWithModes, List.of(languagesField)); + + EsField empNoEsField = new EsField("emp_no", DataType.INTEGER, Collections.emptyMap(), false); + FieldAttribute empNoField = new FieldAttribute(Source.EMPTY, "emp_no", empNoEsField); + Alias empNoAlias = new Alias(Source.EMPTY, "emp_no", empNoField); + LogicalPlan left = new Row(Source.EMPTY, List.of(empNoAlias)); + + LookupJoin lookupJoin = new LookupJoin(Source.EMPTY, left, fakeRelation, + new JoinConfig(JoinTypes.LEFT, List.of(), List.of(), List.of())); + + lookupJoin.postAnalysisVerification(failures); + + String expectedMessage = "Invalid [test1] resolution in lookup mode to [2] indices"; + assertTrue(failures.toString().contains(expectedMessage)); + } } From c59599283f541847db4dbfec3a90b7254de4a34f Mon Sep 17 00:00:00 2001 From: jihwan Date: Sun, 2 Mar 2025 22:57:08 +0900 Subject: [PATCH 2/2] Improve lookup join error messages in ES|QL --- docs/changelog/123813.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/123813.yaml diff --git a/docs/changelog/123813.yaml b/docs/changelog/123813.yaml new file mode 100644 index 0000000000000..cac7a40ef692e --- /dev/null +++ b/docs/changelog/123813.yaml @@ -0,0 +1,5 @@ +pr: 123813 +summary: ensure that LOOKUP JOIN correctly enforces constraints on valid fields and index resolution. +area: ES|QL +type: enhancement +issues: [120189]