Skip to content

Commit 8ffad86

Browse files
bpinteabpintea
committed
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
1 parent 77dd043 commit 8ffad86

File tree

12 files changed

+219
-147
lines changed

12 files changed

+219
-147
lines changed

‎docs/changelog/130638.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 130638
2+
summary: Prevent search functions work with a non-STANDARD index
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 130561
7+
- 129778

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/KnnFunctionIT.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@
88
package org.elasticsearch.xpack.esql.plugin;
99

1010
import org.elasticsearch.action.index.IndexRequestBuilder;
11+
import org.elasticsearch.client.internal.IndicesAdminClient;
1112
import org.elasticsearch.cluster.metadata.IndexMetadata;
1213
import org.elasticsearch.common.settings.Settings;
14+
import org.elasticsearch.index.IndexSettings;
1315
import org.elasticsearch.xcontent.XContentBuilder;
1416
import org.elasticsearch.xcontent.XContentFactory;
1517
import org.elasticsearch.xpack.esql.EsqlTestUtils;
18+
import org.elasticsearch.xpack.esql.VerificationException;
1619
import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase;
1720
import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
1821
import org.junit.Before;
@@ -25,7 +28,9 @@
2528
import java.util.Locale;
2629
import java.util.Map;
2730

31+
import static org.elasticsearch.index.IndexMode.LOOKUP;
2832
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
33+
import static org.hamcrest.CoreMatchers.containsString;
2934

3035
public class KnnFunctionIT extends AbstractEsqlIntegTestCase {
3136

@@ -109,6 +114,26 @@ public void testKnnNonPushedDown() {
109114
}
110115
}
111116

117+
public void testKnnWithLookupJoin() {
118+
float[] queryVector = new float[numDims];
119+
Arrays.fill(queryVector, 1.0f);
120+
121+
var query = String.format(Locale.ROOT, """
122+
FROM test
123+
| LOOKUP JOIN test_lookup ON id
124+
| WHERE KNN(lookup_vector, %s, 5) OR id > 10
125+
""", Arrays.toString(queryVector));
126+
127+
var error = expectThrows(VerificationException.class, () -> run(query));
128+
assertThat(
129+
error.getMessage(),
130+
containsString(
131+
"line 3:13: [KNN] function cannot operate on [lookup_vector], supplied by an index [test_lookup] in non-STANDARD "
132+
+ "mode [lookup]"
133+
)
134+
);
135+
}
136+
112137
@Before
113138
public void setup() throws IOException {
114139
assumeTrue("Needs KNN support", EsqlCapabilities.Cap.KNN_FUNCTION.isEnabled());
@@ -152,5 +177,31 @@ public void setup() throws IOException {
152177
}
153178

154179
indexRandom(true, docs);
180+
181+
createAndPopulateLookupIndex(client, "test_lookup");
182+
}
183+
184+
private void createAndPopulateLookupIndex(IndicesAdminClient client, String lookupIndexName) throws IOException {
185+
XContentBuilder mapping = XContentFactory.jsonBuilder()
186+
.startObject()
187+
.startObject("properties")
188+
.startObject("id")
189+
.field("type", "integer")
190+
.endObject()
191+
.startObject("lookup_vector")
192+
.field("type", "dense_vector")
193+
.field("similarity", "l2_norm")
194+
.endObject()
195+
.endObject()
196+
.endObject();
197+
198+
Settings.Builder settingsBuilder = Settings.builder()
199+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
200+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
201+
.put(IndexSettings.MODE.getKey(), LOOKUP.getName());
202+
203+
var createRequest = client.prepareCreate(lookupIndexName).setMapping(mapping).setSettings(settingsBuilder.build());
204+
assertAcked(createRequest);
205+
155206
}
156207
}

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@
1010
import org.elasticsearch.ElasticsearchException;
1111
import org.elasticsearch.action.index.IndexRequest;
1212
import org.elasticsearch.action.support.WriteRequest;
13+
import org.elasticsearch.client.internal.IndicesAdminClient;
1314
import org.elasticsearch.common.settings.Settings;
1415
import org.elasticsearch.xpack.esql.VerificationException;
1516
import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase;
1617
import org.hamcrest.Matchers;
1718
import org.junit.Before;
1819

1920
import java.util.List;
21+
import java.util.function.Consumer;
2022

2123
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
2224
import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList;
@@ -27,7 +29,7 @@ public class MatchFunctionIT extends AbstractEsqlIntegTestCase {
2729

2830
@Before
2931
public void setupIndex() {
30-
createAndPopulateIndex();
32+
createAndPopulateIndex(this::ensureYellow);
3133
}
3234

3335
public void testSimpleWhereMatch() {
@@ -294,13 +296,30 @@ public void testMatchWithinEval() {
294296
assertThat(error.getMessage(), containsString("[MATCH] function is only supported in WHERE and STATS commands"));
295297
}
296298

297-
private void createAndPopulateIndex() {
299+
public void testMatchWithLookupJoin() {
300+
var query = """
301+
FROM test
302+
| LOOKUP JOIN test_lookup ON id
303+
| WHERE id > 0 AND MATCH(lookup_content, "fox")
304+
""";
305+
306+
var error = expectThrows(VerificationException.class, () -> run(query));
307+
assertThat(
308+
error.getMessage(),
309+
containsString(
310+
"line 3:26: [MATCH] function cannot operate on [lookup_content], supplied by an index [test_lookup] "
311+
+ "in non-STANDARD mode [lookup]"
312+
)
313+
);
314+
}
315+
316+
static void createAndPopulateIndex(Consumer<String[]> ensureYellow) {
298317
var indexName = "test";
299318
var client = client().admin().indices();
300-
var CreateRequest = client.prepareCreate(indexName)
319+
var createRequest = client.prepareCreate(indexName)
301320
.setSettings(Settings.builder().put("index.number_of_shards", 1))
302321
.setMapping("id", "type=integer", "content", "type=text");
303-
assertAcked(CreateRequest);
322+
assertAcked(createRequest);
304323
client().prepareBulk()
305324
.add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox"))
306325
.add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog"))
@@ -310,6 +329,17 @@ private void createAndPopulateIndex() {
310329
.add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog"))
311330
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
312331
.get();
313-
ensureYellow(indexName);
332+
333+
var lookupIndexName = "test_lookup";
334+
createAndPopulateLookupIndex(client, lookupIndexName);
335+
336+
ensureYellow.accept(new String[] { indexName, lookupIndexName });
337+
}
338+
339+
static void createAndPopulateLookupIndex(IndicesAdminClient client, String lookupIndexName) {
340+
var createRequest = client.prepareCreate(lookupIndexName)
341+
.setSettings(Settings.builder().put("index.number_of_shards", 1).put("index.mode", "lookup"))
342+
.setMapping("id", "type=integer", "lookup_content", "type=text");
343+
assertAcked(createRequest);
314344
}
315345
}

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
package org.elasticsearch.xpack.esql.plugin;
99

1010
import org.elasticsearch.ElasticsearchException;
11-
import org.elasticsearch.action.index.IndexRequest;
12-
import org.elasticsearch.action.support.WriteRequest;
13-
import org.elasticsearch.common.settings.Settings;
1411
import org.elasticsearch.index.query.QueryBuilder;
1512
import org.elasticsearch.index.query.QueryBuilders;
1613
import org.elasticsearch.xpack.esql.VerificationException;
@@ -21,7 +18,6 @@
2118

2219
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
2320
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
24-
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
2521
import static org.elasticsearch.xpack.esql.action.EsqlQueryRequest.syncEsqlQueryRequest;
2622
import static org.hamcrest.CoreMatchers.containsString;
2723

@@ -30,7 +26,7 @@ public class MatchOperatorIT extends AbstractEsqlIntegTestCase {
3026

3127
@Before
3228
public void setupIndex() {
33-
createAndPopulateIndex();
29+
MatchFunctionIT.createAndPopulateIndex(this::ensureYellow);
3430
}
3531

3632
public void testSimpleWhereMatch() {
@@ -372,22 +368,20 @@ public void testMatchWithNonTextField() {
372368
}
373369
}
374370

375-
private void createAndPopulateIndex() {
376-
var indexName = "test";
377-
var client = client().admin().indices();
378-
var CreateRequest = client.prepareCreate(indexName)
379-
.setSettings(Settings.builder().put("index.number_of_shards", 1))
380-
.setMapping("id", "type=integer", "content", "type=text");
381-
assertAcked(CreateRequest);
382-
client().prepareBulk()
383-
.add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox"))
384-
.add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog"))
385-
.add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown"))
386-
.add(new IndexRequest(indexName).id("4").source("id", 4, "content", "The dog is brown but this document is very very long"))
387-
.add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat"))
388-
.add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog"))
389-
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
390-
.get();
391-
ensureYellow(indexName);
371+
public void testMatchOperatorWithLookupJoin() {
372+
var query = """
373+
FROM test
374+
| LOOKUP JOIN test_lookup ON id
375+
| WHERE id > 0 AND lookup_content : "fox"
376+
""";
377+
378+
var error = expectThrows(VerificationException.class, () -> run(query));
379+
assertThat(
380+
error.getMessage(),
381+
containsString(
382+
"line 3:20: [:] operator cannot operate on [lookup_content], supplied by an index [test_lookup] "
383+
+ "in non-STANDARD mode [lookup]"
384+
)
385+
);
392386
}
393387
}

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchPhraseFunctionIT.java‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
package org.elasticsearch.xpack.esql.plugin;
99

1010
import org.elasticsearch.ElasticsearchException;
11-
import org.elasticsearch.action.index.IndexRequest;
12-
import org.elasticsearch.action.support.WriteRequest;
13-
import org.elasticsearch.common.settings.Settings;
1411
import org.elasticsearch.xpack.esql.VerificationException;
1512
import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase;
1613
import org.hamcrest.Matchers;
@@ -19,16 +16,16 @@
1916
import java.util.Collections;
2017
import java.util.List;
2118

22-
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
2319
import static org.elasticsearch.xpack.esql.EsqlTestUtils.getValuesList;
20+
import static org.elasticsearch.xpack.esql.plugin.MatchFunctionIT.createAndPopulateIndex;
2421
import static org.hamcrest.CoreMatchers.containsString;
2522

2623
//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug")
2724
public class MatchPhraseFunctionIT extends AbstractEsqlIntegTestCase {
2825

2926
@Before
3027
public void setupIndex() {
31-
createAndPopulateIndex();
28+
createAndPopulateIndex(this::ensureYellow);
3229
}
3330

3431
public void testSimpleWhereMatchPhrase() {
@@ -325,22 +322,20 @@ public void testMatchPhraseWithinEval() {
325322
assertThat(error.getMessage(), containsString("[MatchPhrase] function is only supported in WHERE and STATS commands"));
326323
}
327324

328-
private void createAndPopulateIndex() {
329-
var indexName = "test";
330-
var client = client().admin().indices();
331-
var CreateRequest = client.prepareCreate(indexName)
332-
.setSettings(Settings.builder().put("index.number_of_shards", 1))
333-
.setMapping("id", "type=integer", "content", "type=text");
334-
assertAcked(CreateRequest);
335-
client().prepareBulk()
336-
.add(new IndexRequest(indexName).id("1").source("id", 1, "content", "This is a brown fox"))
337-
.add(new IndexRequest(indexName).id("2").source("id", 2, "content", "This is a brown dog"))
338-
.add(new IndexRequest(indexName).id("3").source("id", 3, "content", "This dog is really brown"))
339-
.add(new IndexRequest(indexName).id("4").source("id", 4, "content", "The dog is brown but this document is very very long"))
340-
.add(new IndexRequest(indexName).id("5").source("id", 5, "content", "There is also a white cat"))
341-
.add(new IndexRequest(indexName).id("6").source("id", 6, "content", "The quick brown fox jumps over the lazy dog"))
342-
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
343-
.get();
344-
ensureYellow(indexName);
325+
public void testMatchPhraseWithLookupJoin() {
326+
var query = """
327+
FROM test
328+
| LOOKUP JOIN test_lookup ON id
329+
| WHERE id > 0 AND MATCH_PHRASE(lookup_content, "fox")
330+
""";
331+
332+
var error = expectThrows(VerificationException.class, () -> run(query));
333+
assertThat(
334+
error.getMessage(),
335+
containsString(
336+
"line 3:33: [MatchPhrase] function cannot operate on [lookup_content], supplied by an index [test_lookup] "
337+
+ "in non-STANDARD mode [lookup]"
338+
)
339+
);
345340
}
346341
}

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/QueryStringIT.java‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@
1616
import org.junit.Before;
1717

1818
import java.util.List;
19+
import java.util.function.Consumer;
1920

2021
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
22+
import static org.elasticsearch.xpack.esql.plugin.MatchFunctionIT.createAndPopulateLookupIndex;
2123
import static org.hamcrest.CoreMatchers.containsString;
2224

2325
public class QueryStringIT extends AbstractEsqlIntegTestCase {
2426

2527
@Before
2628
public void setupIndex() {
27-
createAndPopulateIndex();
29+
createAndPopulateIndex(this::ensureYellow);
2830
}
2931

3032
public void testSimpleQueryString() {
@@ -91,7 +93,7 @@ public void testInvalidQueryStringLexicalError() {
9193
);
9294
}
9395

94-
private void createAndPopulateIndex() {
96+
static void createAndPopulateIndex(Consumer<String[]> ensureYellow) {
9597
var indexName = "test";
9698
var client = client().admin().indices();
9799
var CreateRequest = client.prepareCreate(indexName)
@@ -135,7 +137,11 @@ private void createAndPopulateIndex() {
135137
)
136138
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
137139
.get();
138-
ensureYellow(indexName);
140+
141+
var lookupIndexName = "test_lookup";
142+
createAndPopulateLookupIndex(client, lookupIndexName);
143+
144+
ensureYellow.accept(new String[] { indexName, lookupIndexName });
139145
}
140146

141147
public void testWhereQstrWithScoring() {
@@ -228,4 +234,15 @@ AND abs(id) > 0
228234
assertValuesInAnyOrder(resp.values(), List.of(List.of(5, 1.0), List.of(4, 1.0)));
229235
}
230236
}
237+
238+
public void testWhereQstrWithLookupJoin() {
239+
var query = """
240+
FROM test
241+
| LOOKUP JOIN test_lookup ON id
242+
| WHERE id > 0 AND QSTR("lookup_content: fox")
243+
""";
244+
245+
var error = expectThrows(VerificationException.class, () -> run(query));
246+
assertThat(error.getMessage(), containsString("line 3:3: [QSTR] function cannot be used after LOOKUP"));
247+
}
231248
}

0 commit comments

Comments
 (0)