From a4f3b5e7c85e20e580240463959cc487fef6c44f Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Thu, 2 Jan 2025 19:17:09 +0100 Subject: [PATCH 01/10] Initial support for TEXT fields in LOOKUP JOIN condition This is not sufficient, since we understand it does not consider the term query on the server side possibly performing a CONTAINS query. We should rather rewrite to the underlying keyword sub-field for exact matches. --- .../compute/operator/lookup/QueryList.java | 8 +++++++- .../src/main/resources/lookup-join.csv-spec | 14 +++++++++++++- .../xpack/esql/analysis/Verifier.java | 2 +- .../xpack/esql/enrich/LookupFromIndexService.java | 3 ++- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java index 5428863436535..bfdf6b3bca9cb 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.function.IntFunction; /** @@ -70,7 +71,12 @@ public static QueryList rawTermQueryList(MappedFieldType field, SearchExecutionC } case BYTES_REF -> offset -> { BytesRefBlock bytesRefBlock = (BytesRefBlock) block; - return bytesRefBlock.getBytesRef(offset, new BytesRef()); + BytesRef value = bytesRefBlock.getBytesRef(offset, new BytesRef()); + if (field.typeName().equals("text")) { + // Text fields involve case-insensitive contains queries, we need to use lowercase on the term query + return new BytesRef(value.utf8ToString().toLowerCase(Locale.ROOT)); + } + return value; }; case DOUBLE -> { DoubleBlock doubleBlock = ((DoubleBlock) block); diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index 9b1356438141c..384576c068d1a 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -461,7 +461,7 @@ language_code:integer | language_name:keyword | country:keyword ; ########################################################################### -# nested filed join behavior with languages_nested_fields index +# nested field join behavior with languages_nested_fields index ########################################################################### joinOnNestedField @@ -509,6 +509,18 @@ language.id:integer | language.name:text | language.name.keyword:keyword 1 | English | English ; +joinOnNestedNestedFieldRowImplicitKeyword +required_capability: join_lookup_v10 + +ROW language.name = "English" +| LOOKUP JOIN languages_nested_fields ON language.name +| KEEP language.id, language.name, language.name.keyword +; + +language.id:integer | language.name:keyword | language.name.keyword:keyword +1 | English | English +; + ############################################### # Tests with clientips_lookup index ############################################### diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java index e146b517ad1c8..d99e771ebd5f3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java @@ -858,7 +858,7 @@ private static void checkJoin(LogicalPlan plan, Set failures) { for (int i = 0; i < config.leftFields().size(); i++) { Attribute leftField = config.leftFields().get(i); Attribute rightField = config.rightFields().get(i); - if (leftField.dataType() != rightField.dataType()) { + if (leftField.dataType().noText() != rightField.dataType().noText()) { failures.add( fail( leftField, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java index 0bbfc6dd0ce99..a4a7d9f32c0e9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java @@ -80,7 +80,8 @@ protected String getRequiredPrivilege() { private static void validateTypes(DataType inputDataType, MappedFieldType fieldType) { // TODO: consider supporting implicit type conversion as done in ENRICH for some types - if (fieldType.typeName().equals(inputDataType.typeName()) == false) { + String typeName = fieldType.typeName().equals("text") ? DataType.KEYWORD.typeName() : fieldType.typeName(); + if (typeName.equals(inputDataType.noText().typeName()) == false) { throw new EsqlIllegalArgumentException( "LOOKUP JOIN match and input types are incompatible: match[" + fieldType.typeName() + "], input[" + inputDataType + "]" ); From c1744c97bed8e80371ff4a56ce7cdd7fa1f79a7c Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Fri, 3 Jan 2025 17:26:57 +0100 Subject: [PATCH 02/10] Better approach, using KEYWORD subfield --- .../compute/operator/lookup/QueryList.java | 4 +-- .../src/main/resources/lookup-join.csv-spec | 14 ++++++++ .../esql/planner/LocalExecutionPlanner.java | 36 +++++++++++++------ 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java index bfdf6b3bca9cb..0884cd6d6177a 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java @@ -32,7 +32,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Locale; import java.util.function.IntFunction; /** @@ -73,8 +72,7 @@ public static QueryList rawTermQueryList(MappedFieldType field, SearchExecutionC BytesRefBlock bytesRefBlock = (BytesRefBlock) block; BytesRef value = bytesRefBlock.getBytesRef(offset, new BytesRef()); if (field.typeName().equals("text")) { - // Text fields involve case-insensitive contains queries, we need to use lowercase on the term query - return new BytesRef(value.utf8ToString().toLowerCase(Locale.ROOT)); + throw new IllegalArgumentException("Cannot perform LOOKUP JOIN on TEXT fields"); } return value; }; diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index 384576c068d1a..148faa4b80abf 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -521,6 +521,20 @@ language.id:integer | language.name:keyword | language.name.keyword:keyword 1 | English | English ; +joinOnNestedNestedFieldRowImplicitKeywords +required_capability: join_lookup_v10 + +ROW language.name = ["English", "French"] +| MV_EXPAND language.name +| LOOKUP JOIN languages_nested_fields ON language.name +| KEEP language.id, language.name, language.name.keyword, language.code +; + +language.id:integer | language.name:keyword | language.name.keyword:keyword | language.code:keyword +1 | English | English | EN +2 | French | French | FR +; + ############################################### # Tests with clientips_lookup index ############################################### diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java index c40263baa6566..b3ecdb95419c8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java @@ -55,9 +55,11 @@ import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.expression.Expression; 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.NameId; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; +import org.elasticsearch.xpack.esql.core.expression.TypedAttribute; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.util.Holder; @@ -584,28 +586,35 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan throw new IllegalArgumentException("can't plan [" + join + "], found index with mode [" + entry.getValue() + "]"); } String indexName = entry.getKey(); - List matchFields = new ArrayList<>(join.leftFields().size()); - for (Attribute m : join.leftFields()) { - Layout.ChannelAndType t = source.layout.get(m.id()); - if (t == null) { - throw new IllegalArgumentException("can't plan [" + join + "][" + m + "]"); + if (join.leftFields().size() != join.rightFields().size()) { + throw new IllegalArgumentException("can't plan [" + join + "]: mismatching left and right field count"); + } + List matchFields = new ArrayList<>(join.leftFields().size()); + for (int i = 0; i < join.leftFields().size(); i++) { + TypedAttribute left = (TypedAttribute) join.leftFields().get(i); + FieldAttribute right = (FieldAttribute) join.rightFields().get(i); + Layout.ChannelAndType input = source.layout.get(left.id()); + if (input == null) { + throw new IllegalArgumentException("can't plan [" + join + "][" + left + "]"); } - matchFields.add(t); + matchFields.add(new MatchConfig(right, input)); } if (matchFields.size() != 1) { - throw new IllegalArgumentException("can't plan [" + join + "]"); + throw new IllegalArgumentException("can't plan [" + join + "]: multiple join predicates are not supported"); } + // TODO support multiple match fields, and support more than equality predicates + MatchConfig matchConfig = matchFields.getFirst(); return source.with( new LookupFromIndexOperator.Factory( sessionId, parentTask, context.queryPragmas().enrichMaxWorkers(), - matchFields.getFirst().channel(), + matchConfig.channel(), lookupFromIndexService, - matchFields.getFirst().type(), + matchConfig.type(), indexName, - join.leftFields().getFirst().name(), + matchConfig.fieldName(), join.addedFields().stream().map(f -> (NamedExpression) f).toList(), join.source() ), @@ -613,6 +622,13 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan ); } + private record MatchConfig(String fieldName, int channel, DataType type) { + private MatchConfig(FieldAttribute match, Layout.ChannelAndType input) { + // Note, this handles TEXT fields with KEYWORD subfields, and we assume tha has been validated earlier during planning + this(match.exactAttribute().name(), input.channel(), input.type()); + } + } + private ExpressionEvaluator.Factory toEvaluator(Expression exp, Layout layout) { return EvalMapper.toEvaluator(exp, layout); } From 69572e5557d8203ba99cfb8fe2131a21e64c252d Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Fri, 3 Jan 2025 18:30:35 +0100 Subject: [PATCH 03/10] Add new capability to block mixed-cluster tests --- .../qa/testFixtures/src/main/resources/lookup-join.csv-spec | 2 ++ .../elasticsearch/xpack/esql/action/EsqlCapabilities.java | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index 05b5c38ec8002..c489a0e70bb9c 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -512,6 +512,7 @@ language.id:integer | language.name:text | language.name.keyword:keyword joinOnNestedNestedFieldRowImplicitKeyword required_capability: join_lookup_v10 +required_capability: lookup_join_text ROW language.name = "English" | LOOKUP JOIN languages_nested_fields ON language.name @@ -524,6 +525,7 @@ language.id:integer | language.name:keyword | language.name.keyword:keyword joinOnNestedNestedFieldRowImplicitKeywords required_capability: join_lookup_v10 +required_capability: lookup_join_text ROW language.name = ["English", "French"] | MV_EXPAND language.name diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 5c259caa9c940..5ebae4f300afc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -562,6 +562,11 @@ public enum Cap { */ JOIN_LOOKUP_V10(Build.current().isSnapshot()), + /** + * LOOKUP JOIN with TEXT fields on the right (right side of the join) + */ + LOOKUP_JOIN_TEXT(Build.current().isSnapshot()), + /** * Fix for https://github.com/elastic/elasticsearch/issues/117054 */ From 93779f90c805669bbdeae5828a9236a6e769d8a6 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Thu, 16 Jan 2025 18:50:57 +0100 Subject: [PATCH 04/10] Simplify lookup-join validation and remove keyword subfield validation --- .../compute/operator/lookup/QueryList.java | 6 +----- .../xpack/esql/analysis/Verifier.java | 2 +- .../esql/enrich/LookupFromIndexService.java | 16 +--------------- .../esql/planner/LocalExecutionPlanner.java | 2 +- 4 files changed, 4 insertions(+), 22 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java index 0884cd6d6177a..5428863436535 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/lookup/QueryList.java @@ -70,11 +70,7 @@ public static QueryList rawTermQueryList(MappedFieldType field, SearchExecutionC } case BYTES_REF -> offset -> { BytesRefBlock bytesRefBlock = (BytesRefBlock) block; - BytesRef value = bytesRefBlock.getBytesRef(offset, new BytesRef()); - if (field.typeName().equals("text")) { - throw new IllegalArgumentException("Cannot perform LOOKUP JOIN on TEXT fields"); - } - return value; + return bytesRefBlock.getBytesRef(offset, new BytesRef()); }; case DOUBLE -> { DoubleBlock doubleBlock = ((DoubleBlock) block); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java index d99e771ebd5f3..e146b517ad1c8 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java @@ -858,7 +858,7 @@ private static void checkJoin(LogicalPlan plan, Set failures) { for (int i = 0; i < config.leftFields().size(); i++) { Attribute leftField = config.leftFields().get(i); Attribute rightField = config.rightFields().get(i); - if (leftField.dataType().noText() != rightField.dataType().noText()) { + if (leftField.dataType() != rightField.dataType()) { failures.add( fail( leftField, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java index a4a7d9f32c0e9..17cfb81d8e9f0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java @@ -17,13 +17,11 @@ import org.elasticsearch.compute.data.BlockStreamInput; import org.elasticsearch.compute.data.Page; import org.elasticsearch.compute.operator.lookup.QueryList; -import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchService; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.action.EsqlQueryAction; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; import org.elasticsearch.xpack.esql.core.tree.Source; @@ -68,9 +66,7 @@ protected TransportRequest transportRequest(LookupFromIndexService.Request reque @Override protected QueryList queryList(TransportRequest request, SearchExecutionContext context, Block inputBlock, DataType inputDataType) { - MappedFieldType fieldType = context.getFieldType(request.matchField); - validateTypes(request.inputDataType, fieldType); - return termQueryList(fieldType, context, inputBlock, inputDataType); + return termQueryList(context.getFieldType(request.matchField), context, inputBlock, inputDataType); } @Override @@ -78,16 +74,6 @@ protected String getRequiredPrivilege() { return null; } - private static void validateTypes(DataType inputDataType, MappedFieldType fieldType) { - // TODO: consider supporting implicit type conversion as done in ENRICH for some types - String typeName = fieldType.typeName().equals("text") ? DataType.KEYWORD.typeName() : fieldType.typeName(); - if (typeName.equals(inputDataType.noText().typeName()) == false) { - throw new EsqlIllegalArgumentException( - "LOOKUP JOIN match and input types are incompatible: match[" + fieldType.typeName() + "], input[" + inputDataType + "]" - ); - } - } - public static class Request extends AbstractLookupService.Request { private final String matchField; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java index 621fa150a07bd..6bce002a9562f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java @@ -610,7 +610,7 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan private record MatchConfig(String fieldName, int channel, DataType type) { private MatchConfig(FieldAttribute match, Layout.ChannelAndType input) { - // Note, this handles TEXT fields with KEYWORD subfields, and we assume tha has been validated earlier during planning + // Note, this handles TEXT fields with KEYWORD subfields, and we assume that has been validated earlier during planning this(match.exactAttribute().name(), input.channel(), input.type()); } } From 01467423753bc1c25d31a34c7bdd7a829fbf307e Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Thu, 16 Jan 2025 20:00:26 +0100 Subject: [PATCH 05/10] Added yaml tests for failing TEXT lookup and added validation for that --- .../src/main/resources/lookup-join.csv-spec | 2 - .../xpack/esql/plan/logical/join/Join.java | 3 +- .../test/esql/191_lookup_join_text.yml | 133 ++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index d7f40a86567c5..1eebf5c094522 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -529,7 +529,6 @@ language.id:integer | language.name:text | language.name.keyword:keyword ; joinOnNestedNestedFieldRowImplicitKeyword -required_capability: join_lookup_v10 required_capability: lookup_join_text ROW language.name = "English" @@ -542,7 +541,6 @@ language.id:integer | language.name:keyword | language.name.keyword:keyword ; joinOnNestedNestedFieldRowImplicitKeywords -required_capability: join_lookup_v10 required_capability: lookup_join_text ROW language.name = ["English", "French"] diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java index 2b4f93602c6a7..5b5ea04334e36 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java @@ -28,6 +28,7 @@ import java.util.Objects; import static org.elasticsearch.xpack.esql.common.Failure.fail; +import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes; import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT; @@ -216,7 +217,7 @@ public void postAnalysisVerification(Failures failures) { for (int i = 0; i < config.leftFields().size(); i++) { Attribute leftField = config.leftFields().get(i); Attribute rightField = config.rightFields().get(i); - if (leftField.dataType() != rightField.dataType()) { + if (leftField.dataType().noText() != rightField.dataType().noText() || rightField.dataType().equals(TEXT)) { failures.add( fail( leftField, diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml new file mode 100644 index 0000000000000..3e3ca4485c061 --- /dev/null +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml @@ -0,0 +1,133 @@ +--- +setup: + - requires: + test_runner_features: [capabilities, contains] + capabilities: + - method: POST + path: /_query + parameters: [] + capabilities: [lookup_join_text] + reason: "uses LOOKUP JOIN" + - do: + indices.create: + index: test + body: + mappings: + properties: + color: + type: text + fields: + keyword: + type: keyword + description: + type: text + fields: + keyword: + type: keyword + - do: + indices.create: + index: test-lookup + body: + settings: + index: + mode: lookup + number_of_shards: 1 + mappings: + properties: + color: + type: text + fields: + keyword: + type: keyword + description: + type: text + fields: + keyword: + type: keyword + - do: + bulk: + index: "test" + refresh: true + body: + - { "index": { } } + - { "color": "red", "description": "The color Red" } + - { "index": { } } + - { "color": "blue", "description": "The color Blue" } + - { "index": { } } + - { "color": "green", "description": "The color Green" } + - do: + bulk: + index: "test-lookup" + refresh: true + body: + - { "index": { } } + - { "color": "red", "description": "As red as a tomato" } + - { "index": { } } + - { "color": "blue", "description": "As blue as the sky" } + +--- +keyword-keyword: + - do: + esql.query: + body: + query: 'FROM test | SORT color | LOOKUP JOIN `test-lookup` ON color.keyword | LIMIT 3' + + - length: { columns: 4 } + - length: { values: 3 } + - match: {columns.0.name: "color.keyword"} + - match: {columns.0.type: "keyword"} + - match: {columns.1.name: "color"} + - match: {columns.1.type: "text"} + - match: {columns.2.name: "description"} + - match: {columns.2.type: "text"} + - match: {columns.3.name: "description.keyword"} + - match: {columns.3.type: "keyword"} + - match: {values.0: ["blue", "blue", "As blue as the sky", "As blue as the sky"]} + - match: {values.1: ["green", null, null, null]} + - match: {values.2: ["red", "red", "As red as a tomato", "As red as a tomato"]} + +--- +text-keyword: + - do: + esql.query: + body: + query: 'FROM test | SORT color | RENAME color AS x | EVAL color.keyword = x | LOOKUP JOIN `test-lookup` ON color.keyword | LIMIT 3' + + - length: { columns: 5 } + - length: { values: 3 } + - match: {columns.0.name: "x"} + - match: {columns.0.type: "text"} + - match: {columns.1.name: "color.keyword"} + - match: {columns.1.type: "text"} + - match: {columns.2.name: "color"} + - match: {columns.2.type: "text"} + - match: {columns.3.name: "description"} + - match: {columns.3.type: "text"} + - match: {columns.4.name: "description.keyword"} + - match: {columns.4.type: "keyword"} + - match: {values.0: ["blue", "blue", "blue", "As blue as the sky", "As blue as the sky"]} + - match: {values.1: ["green", "green", null, null, null]} + - match: {values.2: ["red", "red", "red", "As red as a tomato", "As red as a tomato"]} + +--- +text-text: + - do: + esql.query: + body: + query: 'FROM test | SORT color | LOOKUP JOIN `test-lookup` ON color | LIMIT 3' + catch: "bad_request" + + - match: { error.type: "verification_exception" } + - contains: { error.reason: "Found 1 problem\nline 1:55: JOIN left field [color] of type [TEXT] is incompatible with right field [color] of type [TEXT]" } + +--- +keyword-text: + - do: + esql.query: + body: + query: 'FROM test | SORT color | EVAL color = color.keyword | LOOKUP JOIN `test-lookup` ON color | LIMIT 3' + catch: "bad_request" + + - match: { error.type: "verification_exception" } + - contains: { error.reason: "Found 1 problem\nline 1:84: JOIN left field [color] of type [KEYWORD] is incompatible with right field [color] of type [TEXT]" } + From e32780890567098039eaef8e1aed4a69c0480072 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Thu, 16 Jan 2025 20:08:00 +0100 Subject: [PATCH 06/10] Fixed failing tests --- .../src/main/resources/lookup-join.csv-spec | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index 1eebf5c094522..13a5221bbc68d 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -528,30 +528,30 @@ language.id:integer | language.name:text | language.name.keyword:keyword 1 | English | English ; -joinOnNestedNestedFieldRowImplicitKeyword +joinOnNestedNestedFieldRowExplicitKeyword required_capability: lookup_join_text -ROW language.name = "English" -| LOOKUP JOIN languages_nested_fields ON language.name +ROW language.name.keyword = "English" +| LOOKUP JOIN languages_nested_fields ON language.name.keyword | KEEP language.id, language.name, language.name.keyword ; -language.id:integer | language.name:keyword | language.name.keyword:keyword -1 | English | English +language.id:integer | language.name:text | language.name.keyword:keyword +1 | English | English ; -joinOnNestedNestedFieldRowImplicitKeywords +joinOnNestedNestedFieldRowExplicitKeywords required_capability: lookup_join_text -ROW language.name = ["English", "French"] -| MV_EXPAND language.name -| LOOKUP JOIN languages_nested_fields ON language.name +ROW language.name.keyword = ["English", "French"] +| MV_EXPAND language.name.keyword +| LOOKUP JOIN languages_nested_fields ON language.name.keyword | KEEP language.id, language.name, language.name.keyword, language.code ; -language.id:integer | language.name:keyword | language.name.keyword:keyword | language.code:keyword -1 | English | English | EN -2 | French | French | FR +language.id:integer | language.name:text | language.name.keyword:keyword | language.code:keyword +1 | English | English | EN +2 | French | French | FR ; ############################################### From 77fc5bf5c6f3091dd64f9655268c00b2acc77c3d Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Fri, 17 Jan 2025 10:54:56 +0100 Subject: [PATCH 07/10] Missing capability check --- .../qa/testFixtures/src/main/resources/lookup-join.csv-spec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec index 13a5221bbc68d..4ffb00b247166 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec @@ -529,6 +529,7 @@ language.id:integer | language.name:text | language.name.keyword:keyword ; joinOnNestedNestedFieldRowExplicitKeyword +required_capability: join_lookup_v11 required_capability: lookup_join_text ROW language.name.keyword = "English" @@ -541,6 +542,7 @@ language.id:integer | language.name:text | language.name.keyword:keyword ; joinOnNestedNestedFieldRowExplicitKeywords +required_capability: join_lookup_v11 required_capability: lookup_join_text ROW language.name.keyword = ["English", "French"] From 34910069e00d8122323d754a186dca9175e6f097 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 20 Jan 2025 21:21:15 +0100 Subject: [PATCH 08/10] Simplify long comment --- .../elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java index 918e08046cbe8..787bf297563ac 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java @@ -611,7 +611,7 @@ private PhysicalOperation planLookupJoin(LookupJoinExec join, LocalExecutionPlan private record MatchConfig(String fieldName, int channel, DataType type) { private MatchConfig(FieldAttribute match, Layout.ChannelAndType input) { - // Note, this handles TEXT fields with KEYWORD subfields, and we assume that has been validated earlier during planning + // Note, this handles TEXT fields with KEYWORD subfields this(match.exactAttribute().name(), input.channel(), input.type()); } } From d5d7cee3f92869ba77f7d98615cbd6c16e70e9bf Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 22 Jan 2025 16:48:42 +0200 Subject: [PATCH 09/10] Use clearer error message when joining with right-side TEXT --- .../elasticsearch/xpack/esql/plan/logical/join/Join.java | 7 ++++++- .../rest-api-spec/test/esql/191_lookup_join_text.yml | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java index 5b5ea04334e36..a541142f952e0 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java @@ -217,7 +217,7 @@ public void postAnalysisVerification(Failures failures) { for (int i = 0; i < config.leftFields().size(); i++) { Attribute leftField = config.leftFields().get(i); Attribute rightField = config.rightFields().get(i); - if (leftField.dataType().noText() != rightField.dataType().noText() || rightField.dataType().equals(TEXT)) { + if (leftField.dataType().noText() != rightField.dataType().noText()) { failures.add( fail( leftField, @@ -229,6 +229,11 @@ public void postAnalysisVerification(Failures failures) { ) ); } + if (rightField.dataType().equals(TEXT)) { + failures.add( + fail(leftField, "JOIN with right field [{}] of type [{}] is not supported", rightField.name(), rightField.dataType()) + ); + } } } } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml index 3e3ca4485c061..2f3dab13423a5 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml @@ -118,7 +118,7 @@ text-text: catch: "bad_request" - match: { error.type: "verification_exception" } - - contains: { error.reason: "Found 1 problem\nline 1:55: JOIN left field [color] of type [TEXT] is incompatible with right field [color] of type [TEXT]" } + - contains: { error.reason: "Found 1 problem\nline 1:55: JOIN with right field [color] of type [TEXT] is not supported" } --- keyword-text: From 724153ce790f1f14d739509acf742c3aefa95f3d Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 22 Jan 2025 17:35:23 +0200 Subject: [PATCH 10/10] Fix failing test --- .../resources/rest-api-spec/test/esql/191_lookup_join_text.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml index 2f3dab13423a5..1b532ab80eeb6 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_text.yml @@ -129,5 +129,5 @@ keyword-text: catch: "bad_request" - match: { error.type: "verification_exception" } - - contains: { error.reason: "Found 1 problem\nline 1:84: JOIN left field [color] of type [KEYWORD] is incompatible with right field [color] of type [TEXT]" } + - contains: { error.reason: "Found 1 problem\nline 1:84: JOIN with right field [color] of type [TEXT] is not supported" }