diff --git a/docs/changelog/120372.yaml b/docs/changelog/120372.yaml new file mode 100644 index 0000000000000..1db505b8b06e2 --- /dev/null +++ b/docs/changelog/120372.yaml @@ -0,0 +1,6 @@ +pr: 120372 +summary: Ask for correct field names if only LOOKUP/ENRICH fields are kept +area: ES|QL +type: bug +issues: + - 120272 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec index 25b114b5d1daf..30bbee8232c1b 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec @@ -643,3 +643,19 @@ FROM airports abbrev:k | city_name:k | city_location:geo_point | country:k | location:geo_point | name:text | region:text | boundary_wkt_length:i IDR | Indore | POINT(75.8472 22.7167) | India | POINT(75.8092915005895 22.727749187571) | Devi Ahilyabai Holkar Int'l | Indore City | 231 ; + +enrichDroppingOriginalIndexFields +required_capability: enrich_load +required_capability: fix_field_names_enrich_lookup_no_valid_fields + +from employees +| limit 3 +| eval x = 2 +| enrich languages_policy on x +| keep language_name; + +language_name:keyword +French +French +French +; 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 c8203042a23de..71704538752d6 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 @@ -242,6 +242,29 @@ emp_no:integer 10001 ; +dropAllOriginalIndexFields +required_capability: join_lookup_v11 +required_capability: fix_field_names_enrich_lookup_no_valid_fields + +FROM employees +| LIMIT 2 +| EVAL language_code = 1 +| LOOKUP JOIN languages_lookup_non_unique_key ON language_code +| SORT language_name, country +| KEEP language_name, country +; + +language_name:keyword | country:text +English | Canada +English | Canada +English | United States of America +English | United States of America +English | null +English | null +null | United Kingdom +null | United Kingdom +; + ############################################### # Filtering tests with languages_lookup index ############################################### 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 fb5433d7662af..a92ced8fee0cc 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 @@ -735,6 +735,13 @@ public enum Cap { */ SEMANTIC_TEXT_FIELD_CAPS, + /** + * Fix for https://github.com/elastic/elasticsearch/issues/120272. + * When there's an ENRICH/LOOKUP, and all fields from the original indices are discarded, + * field-caps wasn't asking for any existing field. + */ + FIX_FIELD_NAMES_ENRICH_LOOKUP_NO_VALID_FIELDS, + /** * Support named argument for function in map format. */ diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index b10f766babb36..1dbcb96d99edd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -543,6 +543,10 @@ private static void resolveFieldNames(LogicalPlan parsed, EnrichResolution enric } } + /** + * Get the field names from the parsed plan combined with the ENRICH match fields from the ENRICH policy + * and the JOIN ON keys. + */ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicyMatchFields, PreAnalysisResult result) { if (false == parsed.anyMatch(plan -> plan instanceof Aggregate || plan instanceof Project)) { // no explicit columns selection, for example "from employees" @@ -635,17 +639,16 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicy // remove valid metadata attributes because they will be filtered out by the IndexResolver anyway // otherwise, in some edge cases, we will fail to ask for "*" (all fields) instead references.removeIf(a -> a instanceof MetadataAttribute || MetadataAttribute.isSupported(a.name())); - Set fieldNames = references.names(); - if (fieldNames.isEmpty() && enrichPolicyMatchFields.isEmpty()) { - // there cannot be an empty list of fields, we'll ask the simplest and lightest one instead: _index - return result.withFieldNames(IndexResolver.INDEX_METADATA_FIELD); - } else { - fieldNames.addAll(subfields(fieldNames)); - fieldNames.addAll(enrichPolicyMatchFields); - fieldNames.addAll(subfields(enrichPolicyMatchFields)); - return result.withFieldNames(fieldNames); - } + Set fieldNames = references.names(); + fieldNames.addAll(subfields(fieldNames)); + fieldNames.addAll(enrichPolicyMatchFields); + fieldNames.addAll(subfields(enrichPolicyMatchFields)); + // We add the "_index" field because it's light, and there must always be some field matching the indices. + // Originally, this was used only when no fields were found, but ENRICH/LOOKUP fields can't currently + // be distinguished from fields of the original index, so we always add it as a safety measure. + fieldNames.addAll(IndexResolver.INDEX_METADATA_FIELD); + return result.withFieldNames(fieldNames); } private static boolean matchByName(Attribute attr, String other, boolean skipIfPattern) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/IndexResolverFieldNamesTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/IndexResolverFieldNamesTests.java index b1c9030db7a43..94ad54e5ef509 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/IndexResolverFieldNamesTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/IndexResolverFieldNamesTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.esql.session; import org.elasticsearch.Build; +import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; import org.elasticsearch.xpack.esql.parser.EsqlParser; @@ -1321,15 +1322,14 @@ public void testEnrichOnDefaultFieldWithKeep() { from employees | enrich languages_policy | keep emp_no""", Set.of("language_name")); - assertThat(fieldNames, equalTo(Set.of("emp_no", "emp_no.*", "language_name", "language_name.*"))); + assertThat(fieldNames, equalTo(expectedFieldNames(Set.of("emp_no", "emp_no.*", "language_name", "language_name.*")))); } public void testDissectOverwriteName() { - Set fieldNames = fieldNames(""" + assertFieldNames(""" from employees | dissect first_name "%{first_name} %{more}" - | keep emp_no, first_name, more""", Set.of()); - assertThat(fieldNames, equalTo(Set.of("emp_no", "emp_no.*", "first_name", "first_name.*"))); + | keep emp_no, first_name, more""", Set.of("emp_no", "emp_no.*", "first_name", "first_name.*")); } public void testEnrichOnDefaultField() { @@ -1346,20 +1346,17 @@ public void testMetrics() { assertThat(e.getMessage(), containsString("line 1:1: mismatched input 'METRICS' expecting {")); return; } - Set fieldNames = fieldNames(query, Set.of()); - assertThat( - fieldNames, - equalTo( - Set.of( - "@timestamp", - "@timestamp.*", - "network.total_bytes_in", - "network.total_bytes_in.*", - "network.total_cost", - "network.total_cost.*", - "cluster", - "cluster.*" - ) + assertFieldNames( + query, + Set.of( + "@timestamp", + "@timestamp.*", + "network.total_bytes_in", + "network.total_bytes_in.*", + "network.total_cost", + "network.total_cost.*", + "cluster", + "cluster.*" ) ); } @@ -1575,6 +1572,15 @@ public void testMultiLookupJoinSameIndexKeepAfter() { ); } + public void testLookupJoinWithEvalKey() { + assumeTrue("LOOKUP JOIN available as snapshot only", EsqlCapabilities.Cap.JOIN_LOOKUP_V11.isEnabled()); + assertFieldNames(""" + FROM some_index + | EVAL key = 5 + | LOOKUP JOIN some_lookup ON key + | KEEP test""", Set.of("test", "test.*", "key", "key.*")); + } + private Set fieldNames(String query, Set enrichPolicyMatchFields) { var preAnalysisResult = new EsqlSession.PreAnalysisResult(null); return EsqlSession.fieldNames(parser.createStatement(query), enrichPolicyMatchFields, preAnalysisResult).fieldNames(); @@ -1582,12 +1588,20 @@ private Set fieldNames(String query, Set enrichPolicyMatchFields private void assertFieldNames(String query, Set expected) { Set fieldNames = fieldNames(query, Collections.emptySet()); - assertThat(fieldNames, equalTo(expected)); + assertThat(fieldNames, equalTo(expectedFieldNames(expected))); } private void assertFieldNames(String query, Set expected, Set wildCardIndices) { var preAnalysisResult = EsqlSession.fieldNames(parser.createStatement(query), Set.of(), new EsqlSession.PreAnalysisResult(null)); - assertThat("Query-wide field names", preAnalysisResult.fieldNames(), equalTo(expected)); + assertThat("Query-wide field names", preAnalysisResult.fieldNames(), equalTo(expectedFieldNames(expected))); assertThat("Lookup Indices that expect wildcard lookups", preAnalysisResult.wildcardJoinIndices(), equalTo(wildCardIndices)); } + + private Set expectedFieldNames(Set expected) { + if (expected.size() == 1 && expected.contains("*")) { + return expected; + } + + return Sets.union(expected, IndexResolver.INDEX_METADATA_FIELD); + } }