From 8ffad8659c1740eab91ebd5b06833e8fb1c546ef Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Fri, 4 Jul 2025 20:45:10 +0200 Subject: [PATCH 1/2] ESQL: Prevent search functions work with a non-STANDARD index (#130638) This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index. This should be a temporary fix until we can either push these filters down also on the righthand-side of a JOIN or have these functions execute within the engine. Closes #130561 Closes #129778 --- docs/changelog/130638.yaml | 7 ++ .../xpack/esql/plugin/KnnFunctionIT.java | 51 +++++++++++++++ .../xpack/esql/plugin/MatchFunctionIT.java | 40 ++++++++++-- .../xpack/esql/plugin/MatchOperatorIT.java | 38 +++++------ .../esql/plugin/MatchPhraseFunctionIT.java | 39 +++++------ .../xpack/esql/plugin/QueryStringIT.java | 23 ++++++- .../xpack/esql/plugin/TermIT.java | 65 +++++-------------- .../function/fulltext/FullTextFunction.java | 40 ++++++++++++ .../expression/function/fulltext/Match.java | 15 +---- .../function/fulltext/MatchPhrase.java | 17 +---- .../expression/function/fulltext/Term.java | 16 +---- .../esql/expression/function/vector/Knn.java | 15 ++++- 12 files changed, 219 insertions(+), 147 deletions(-) create mode 100644 docs/changelog/130638.yaml diff --git a/docs/changelog/130638.yaml b/docs/changelog/130638.yaml new file mode 100644 index 0000000000000..c32f0269aab09 --- /dev/null +++ b/docs/changelog/130638.yaml @@ -0,0 +1,7 @@ +pr: 130638 +summary: Prevent search functions work with a non-STANDARD index +area: ES|QL +type: bug +issues: + - 130561 + - 129778 diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index a262943909938..1e38d660c736c 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -8,11 +8,14 @@ package org.elasticsearch.xpack.esql.plugin; import org.elasticsearch.action.index.IndexRequestBuilder; +import org.elasticsearch.client.internal.IndicesAdminClient; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.xpack.esql.EsqlTestUtils; +import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; import org.junit.Before; @@ -25,7 +28,9 @@ import java.util.Locale; import java.util.Map; +import static org.elasticsearch.index.IndexMode.LOOKUP; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.CoreMatchers.containsString; public class KnnFunctionIT extends AbstractEsqlIntegTestCase { @@ -109,6 +114,26 @@ public void testKnnNonPushedDown() { } } + public void testKnnWithLookupJoin() { + float[] queryVector = new float[numDims]; + Arrays.fill(queryVector, 1.0f); + + var query = String.format(Locale.ROOT, """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE KNN(lookup_vector, %s, 5) OR id > 10 + """, Arrays.toString(queryVector)); + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat( + error.getMessage(), + containsString( + "line 3:13: [KNN] function cannot operate on [lookup_vector], supplied by an index [test_lookup] in non-STANDARD " + + "mode [lookup]" + ) + ); + } + @Before public void setup() throws IOException { assumeTrue("Needs KNN support", EsqlCapabilities.Cap.KNN_FUNCTION.isEnabled()); @@ -152,5 +177,31 @@ public void setup() throws IOException { } indexRandom(true, docs); + + createAndPopulateLookupIndex(client, "test_lookup"); + } + + private void createAndPopulateLookupIndex(IndicesAdminClient client, String lookupIndexName) throws IOException { + XContentBuilder mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject("id") + .field("type", "integer") + .endObject() + .startObject("lookup_vector") + .field("type", "dense_vector") + .field("similarity", "l2_norm") + .endObject() + .endObject() + .endObject(); + + Settings.Builder settingsBuilder = Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexSettings.MODE.getKey(), LOOKUP.getName()); + + var createRequest = client.prepareCreate(lookupIndexName).setMapping(mapping).setSettings(settingsBuilder.build()); + assertAcked(createRequest); + } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java index 23958fcd35f30..82124c4c85bb8 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.client.internal.IndicesAdminClient; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase; @@ -17,6 +18,7 @@ import org.junit.Before; import java.util.List; +import java.util.function.Consumer; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList; @@ -27,7 +29,7 @@ public class MatchFunctionIT extends AbstractEsqlIntegTestCase { @Before public void setupIndex() { - createAndPopulateIndex(); + createAndPopulateIndex(this::ensureYellow); } public void testSimpleWhereMatch() { @@ -294,13 +296,30 @@ public void testMatchWithinEval() { assertThat(error.getMessage(), containsString("[MATCH] function is only supported in WHERE and STATS commands")); } - private void createAndPopulateIndex() { + public void testMatchWithLookupJoin() { + var query = """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE id > 0 AND MATCH(lookup_content, "fox") + """; + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat( + error.getMessage(), + containsString( + "line 3:26: [MATCH] function cannot operate on [lookup_content], supplied by an index [test_lookup] " + + "in non-STANDARD mode [lookup]" + ) + ); + } + + static void createAndPopulateIndex(Consumer ensureYellow) { var indexName = "test"; var client = client().admin().indices(); - var CreateRequest = client.prepareCreate(indexName) + var createRequest = client.prepareCreate(indexName) .setSettings(Settings.builder().put("index.number_of_shards", 1)) .setMapping("id", "type=integer", "content", "type=text"); - assertAcked(CreateRequest); + assertAcked(createRequest); client().prepareBulk() .add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox")) .add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog")) @@ -310,6 +329,17 @@ private void createAndPopulateIndex() { .add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog")) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .get(); - ensureYellow(indexName); + + var lookupIndexName = "test_lookup"; + createAndPopulateLookupIndex(client, lookupIndexName); + + ensureYellow.accept(new String[] { indexName, lookupIndexName }); + } + + static void createAndPopulateLookupIndex(IndicesAdminClient client, String lookupIndexName) { + var createRequest = client.prepareCreate(lookupIndexName) + .setSettings(Settings.builder().put("index.number_of_shards", 1).put("index.mode", "lookup")) + .setMapping("id", "type=integer", "lookup_content", "type=text"); + assertAcked(createRequest); } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java index 5a87a4b4302c4..efad0f92ae967 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java @@ -8,9 +8,6 @@ package org.elasticsearch.xpack.esql.plugin; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.index.IndexRequest; -import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.xpack.esql.VerificationException; @@ -21,7 +18,6 @@ import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.esql.action.EsqlQueryRequest.syncEsqlQueryRequest; import static org.hamcrest.CoreMatchers.containsString; @@ -30,7 +26,7 @@ public class MatchOperatorIT extends AbstractEsqlIntegTestCase { @Before public void setupIndex() { - createAndPopulateIndex(); + MatchFunctionIT.createAndPopulateIndex(this::ensureYellow); } public void testSimpleWhereMatch() { @@ -372,22 +368,20 @@ public void testMatchWithNonTextField() { } } - private void createAndPopulateIndex() { - var indexName = "test"; - var client = client().admin().indices(); - var CreateRequest = client.prepareCreate(indexName) - .setSettings(Settings.builder().put("index.number_of_shards", 1)) - .setMapping("id", "type=integer", "content", "type=text"); - assertAcked(CreateRequest); - client().prepareBulk() - .add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox")) - .add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog")) - .add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown")) - .add(new IndexRequest(indexName).id("4").source("id", 4, "content", "The dog is brown but this document is very very long")) - .add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat")) - .add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog")) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .get(); - ensureYellow(indexName); + public void testMatchOperatorWithLookupJoin() { + var query = """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE id > 0 AND lookup_content : "fox" + """; + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat( + error.getMessage(), + containsString( + "line 3:20: [:] operator cannot operate on [lookup_content], supplied by an index [test_lookup] " + + "in non-STANDARD mode [lookup]" + ) + ); } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchPhraseFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchPhraseFunctionIT.java index 44f28e0c9ea93..f269fc7d2bd46 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchPhraseFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchPhraseFunctionIT.java @@ -8,9 +8,6 @@ package org.elasticsearch.xpack.esql.plugin; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.index.IndexRequest; -import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase; import org.hamcrest.Matchers; @@ -19,8 +16,8 @@ import java.util.Collections; import java.util.List; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList; +import static org.elasticsearch.xpack.esql.plugin.MatchFunctionIT.createAndPopulateIndex; import static org.hamcrest.CoreMatchers.containsString; //@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") @@ -28,7 +25,7 @@ public class MatchPhraseFunctionIT extends AbstractEsqlIntegTestCase { @Before public void setupIndex() { - createAndPopulateIndex(); + createAndPopulateIndex(this::ensureYellow); } public void testSimpleWhereMatchPhrase() { @@ -325,22 +322,20 @@ public void testMatchPhraseWithinEval() { assertThat(error.getMessage(), containsString("[MatchPhrase] function is only supported in WHERE and STATS commands")); } - private void createAndPopulateIndex() { - var indexName = "test"; - var client = client().admin().indices(); - var CreateRequest = client.prepareCreate(indexName) - .setSettings(Settings.builder().put("index.number_of_shards", 1)) - .setMapping("id", "type=integer", "content", "type=text"); - assertAcked(CreateRequest); - client().prepareBulk() - .add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox")) - .add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog")) - .add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown")) - .add(new IndexRequest(indexName).id("4").source("id", 4, "content", "The dog is brown but this document is very very long")) - .add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat")) - .add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog")) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .get(); - ensureYellow(indexName); + public void testMatchPhraseWithLookupJoin() { + var query = """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE id > 0 AND MATCH_PHRASE(lookup_content, "fox") + """; + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat( + error.getMessage(), + containsString( + "line 3:33: [MatchPhrase] function cannot operate on [lookup_content], supplied by an index [test_lookup] " + + "in non-STANDARD mode [lookup]" + ) + ); } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/QueryStringIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/QueryStringIT.java index aba0a5f4a5b97..d241580545c31 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/QueryStringIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/QueryStringIT.java @@ -16,15 +16,17 @@ import org.junit.Before; import java.util.List; +import java.util.function.Consumer; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.xpack.esql.plugin.MatchFunctionIT.createAndPopulateLookupIndex; import static org.hamcrest.CoreMatchers.containsString; public class QueryStringIT extends AbstractEsqlIntegTestCase { @Before public void setupIndex() { - createAndPopulateIndex(); + createAndPopulateIndex(this::ensureYellow); } public void testSimpleQueryString() { @@ -91,7 +93,7 @@ public void testInvalidQueryStringLexicalError() { ); } - private void createAndPopulateIndex() { + static void createAndPopulateIndex(Consumer ensureYellow) { var indexName = "test"; var client = client().admin().indices(); var CreateRequest = client.prepareCreate(indexName) @@ -135,7 +137,11 @@ private void createAndPopulateIndex() { ) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .get(); - ensureYellow(indexName); + + var lookupIndexName = "test_lookup"; + createAndPopulateLookupIndex(client, lookupIndexName); + + ensureYellow.accept(new String[] { indexName, lookupIndexName }); } public void testWhereQstrWithScoring() { @@ -228,4 +234,15 @@ AND abs(id) > 0 assertValuesInAnyOrder(resp.values(), List.of(List.of(5, 1.0), List.of(4, 1.0))); } } + + public void testWhereQstrWithLookupJoin() { + var query = """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE id > 0 AND QSTR("lookup_content: fox") + """; + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat(error.getMessage(), containsString("line 3:3: [QSTR] function cannot be used after LOOKUP")); + } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/TermIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/TermIT.java index 9afefca5eed6e..a56dec2ff4883 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/TermIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/TermIT.java @@ -7,9 +7,6 @@ package org.elasticsearch.xpack.esql.plugin; -import org.elasticsearch.action.index.IndexRequest; -import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; @@ -19,14 +16,14 @@ import java.util.List; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.xpack.esql.plugin.QueryStringIT.createAndPopulateIndex; import static org.hamcrest.CoreMatchers.containsString; public class TermIT extends AbstractEsqlIntegTestCase { @Before public void setupIndex() { - createAndPopulateIndex(); + createAndPopulateIndex(this::ensureYellow); } @Override @@ -90,50 +87,20 @@ public void testNotWhereTerm() { } } - private void createAndPopulateIndex() { - var indexName = "test"; - var client = client().admin().indices(); - var CreateRequest = client.prepareCreate(indexName) - .setSettings(Settings.builder().put("index.number_of_shards", 1)) - .setMapping("id", "type=integer", "content", "type=text"); - assertAcked(CreateRequest); - client().prepareBulk() - .add( - new IndexRequest(indexName).id("1") - .source("id", 1, "content", "The quick brown animal swiftly jumps over a lazy dog", "title", "A Swift Fox's Journey") - ) - .add( - new IndexRequest(indexName).id("2") - .source("id", 2, "content", "A speedy brown fox hops effortlessly over a sluggish canine", "title", "The Fox's Leap") - ) - .add( - new IndexRequest(indexName).id("3") - .source("id", 3, "content", "Quick and nimble, the fox vaults over the lazy dog", "title", "Brown Fox in Action") - ) - .add( - new IndexRequest(indexName).id("4") - .source( - "id", - 4, - "content", - "A fox that is quick and brown jumps over a dog that is quite lazy", - "title", - "Speedy Animals" - ) - ) - .add( - new IndexRequest(indexName).id("5") - .source( - "id", - 5, - "content", - "With agility, a quick brown fox bounds over a slow-moving dog", - "title", - "Foxes and Canines" - ) + public void testTermWithLookupJoin() { + var query = """ + FROM test + | LOOKUP JOIN test_lookup ON id + | WHERE id > 0 AND TERM(lookup_content, "fox") + """; + + var error = expectThrows(VerificationException.class, () -> run(query)); + assertThat( + error.getMessage(), + containsString( + "line 3:25: [Term] function cannot operate on [lookup_content], supplied by an index [test_lookup] " + + "in non-STANDARD mode [lookup]" ) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .get(); - ensureYellow(indexName); + ); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java index 5510fbddd27c7..ec29b4b658c76 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java @@ -14,6 +14,7 @@ import org.elasticsearch.compute.lucene.LuceneQueryScoreEvaluator; import org.elasticsearch.compute.operator.EvalOperator; import org.elasticsearch.compute.operator.ScoreOperator; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware; import org.elasticsearch.xpack.esql.capabilities.TranslationAware; @@ -314,6 +315,45 @@ private static FullTextFunction forEachFullTextFunctionParent(Expression conditi return null; } + public static void fieldVerifier(LogicalPlan plan, FullTextFunction function, Expression field, Failures failures) { + var fieldAttribute = fieldAsFieldAttribute(field); + if (fieldAttribute == null) { + plan.forEachExpression(function.getClass(), m -> { + if (function.children().contains(field)) { + failures.add( + fail( + field, + "[{}] {} cannot operate on [{}], which is not a field from an index mapping", + m.functionName(), + m.functionType(), + field.sourceText() + ) + ); + } + }); + } else { + // Traverse the plan to find the EsRelation outputting the field + plan.forEachDown(p -> { + if (p instanceof EsRelation esRelation && esRelation.indexMode() != IndexMode.STANDARD) { + // Check if this EsRelation supplies the field + if (esRelation.outputSet().contains(fieldAttribute)) { + failures.add( + fail( + field, + "[{}] {} cannot operate on [{}], supplied by an index [{}] in non-STANDARD mode [{}]", + function.functionName(), + function.functionType(), + field.sourceText(), + esRelation.indexPattern(), + esRelation.indexMode() + ) + ); + } + } + }); + } + } + @Override public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) { List shardContexts = toEvaluator.shardContexts(); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java index 53aa87f4b861a..e6d99d158aaaf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java @@ -15,7 +15,6 @@ import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware; -import org.elasticsearch.xpack.esql.common.Failure; import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.InvalidArgumentException; import org.elasticsearch.xpack.esql.core.expression.Expression; @@ -393,19 +392,7 @@ public Expression replaceQueryBuilder(QueryBuilder queryBuilder) { public BiConsumer postAnalysisPlanVerification() { return (plan, failures) -> { super.postAnalysisPlanVerification().accept(plan, failures); - plan.forEachExpression(Match.class, m -> { - if (m.fieldAsFieldAttribute() == null) { - failures.add( - Failure.fail( - m.field(), - "[{}] {} cannot operate on [{}], which is not a field from an index mapping", - functionName(), - functionType(), - m.field().sourceText() - ) - ); - } - }); + fieldVerifier(plan, this, field, failures); }; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java index 134096460a3d7..4a99227576611 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware; -import org.elasticsearch.xpack.esql.common.Failure; import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.InvalidArgumentException; import org.elasticsearch.xpack.esql.core.expression.Expression; @@ -60,10 +59,8 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS; import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT; import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; -import static org.elasticsearch.xpack.esql.core.type.DataType.IP; import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; -import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; /** * Full text function that performs a {@link org.elasticsearch.xpack.esql.querydsl.query.MatchPhraseQuery} . @@ -252,19 +249,7 @@ public Expression replaceQueryBuilder(QueryBuilder queryBuilder) { public BiConsumer postAnalysisPlanVerification() { return (plan, failures) -> { super.postAnalysisPlanVerification().accept(plan, failures); - plan.forEachExpression(MatchPhrase.class, mp -> { - if (mp.fieldAsFieldAttribute() == null) { - failures.add( - Failure.fail( - mp.field(), - "[{}] {} cannot operate on [{}], which is not a field from an index mapping", - functionName(), - functionType(), - mp.field().sourceText() - ) - ); - } - }); + FullTextFunction.fieldVerifier(plan, this, field, failures); }; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Term.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Term.java index 94de3d33251d8..76188dc146ee6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Term.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Term.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware; -import org.elasticsearch.xpack.esql.common.Failure; import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; @@ -112,19 +111,7 @@ private TypeResolution resolveField() { public BiConsumer postAnalysisPlanVerification() { return (plan, failures) -> { super.postAnalysisPlanVerification().accept(plan, failures); - plan.forEachExpression(Term.class, t -> { - if (t.field() instanceof FieldAttribute == false) { // TODO: is a conversion possible, similar to Match’s case? - failures.add( - Failure.fail( - t.field(), - "[{}] {} cannot operate on [{}], which is not a field from an index mapping", - t.functionName(), - t.functionType(), - t.field().sourceText() - ) - ); - } - }); + fieldVerifier(plan, this, field, failures); }; } @@ -157,6 +144,7 @@ public Expression field() { return field; } + // TODO: method can be dropped, to allow failure messages contain the capitalized function name, aligned with similar functions/classes @Override public String functionName() { return ENTRY.name; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/vector/Knn.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/vector/Knn.java index ecce0b069693d..df1542a2c292c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/vector/Knn.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/vector/Knn.java @@ -11,6 +11,8 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware; +import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.InvalidArgumentException; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.FoldContext; @@ -31,6 +33,7 @@ import org.elasticsearch.xpack.esql.expression.function.fulltext.FullTextFunction; import org.elasticsearch.xpack.esql.expression.function.fulltext.Match; import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; +import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.planner.TranslatorHandler; import org.elasticsearch.xpack.esql.querydsl.query.KnnQuery; @@ -39,6 +42,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.BiConsumer; import static java.util.Map.entry; import static org.elasticsearch.index.query.AbstractQueryBuilder.BOOST_FIELD; @@ -55,9 +59,8 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.DENSE_VECTOR; import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT; import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; -import static org.elasticsearch.xpack.esql.expression.function.fulltext.Match.getNameFromFieldAttribute; -public class Knn extends FullTextFunction implements OptionalArgument, VectorFunction { +public class Knn extends FullTextFunction implements OptionalArgument, VectorFunction, PostAnalysisPlanVerificationAware { public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Knn", Knn::readFrom); @@ -235,6 +238,14 @@ private Map queryOptions() throws InvalidArgumentException { return options; } + @Override + public BiConsumer postAnalysisPlanVerification() { + return (plan, failures) -> { + super.postAnalysisPlanVerification().accept(plan, failures); + fieldVerifier(plan, this, field, failures); + }; + } + @Override public Expression replaceChildren(List newChildren) { return new Knn( From 68b61eaf6c02a6cac25c8471fcd0891d04883857 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Mon, 7 Jul 2025 11:45:33 +0200 Subject: [PATCH 2/2] adapt test to 9.1 --- .../java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java index 1e38d660c736c..a107e58c0869c 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java @@ -121,7 +121,7 @@ public void testKnnWithLookupJoin() { var query = String.format(Locale.ROOT, """ FROM test | LOOKUP JOIN test_lookup ON id - | WHERE KNN(lookup_vector, %s, 5) OR id > 10 + | WHERE KNN(lookup_vector, %s, {"k": 5}) OR id > 10 """, Arrays.toString(queryVector)); var error = expectThrows(VerificationException.class, () -> run(query));