Skip to content

Commit 3dc7bfd

Browse files
carlosdelestelasticsearchmachine
authored andcommitted
ES|QL - Full text functions accept null as field parameter (#137914)
(cherry picked from commit 85076fe) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MultiMatch.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/vector/Knn.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/KnnTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java
1 parent c2a0be3 commit 3dc7bfd

File tree

19 files changed

+799
-330
lines changed

19 files changed

+799
-330
lines changed

docs/changelog/137430.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 137430
2+
summary: ES|QL - Full text functions accept null as field parameter
3+
area: "ES|QL"
4+
type: bug
5+
issues:
6+
- 136608

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,73 @@ public void testRLikeHandlingOfEmptyLanguagePattern() throws IOException {
15291529
assertThat(answer.get("values"), equalTo(List.of(List.of("#"), List.of("foo#bar"))));
15301530
}
15311531

1532+
public void testMatchFunctionAcrossMultipleIndicesWithMissingField() throws IOException {
1533+
int numberOfIndicesWithField = randomIntBetween(11, 20);
1534+
int numberOfIndicesWithoutField = randomIntBetween(1, 10);
1535+
int totalIndices = numberOfIndicesWithField + numberOfIndicesWithoutField;
1536+
1537+
int indexNum = 0;
1538+
for (int i = 0; i < numberOfIndicesWithField; i++) {
1539+
String indexName = testIndexName() + indexNum++;
1540+
// Create index with the text field
1541+
createIndex(indexName, Settings.EMPTY, """
1542+
{
1543+
"properties": {
1544+
"message": {
1545+
"type": "text"
1546+
}
1547+
}
1548+
}
1549+
""");
1550+
Request doc = new Request("POST", indexName + "/_doc?refresh=true");
1551+
doc.setJsonEntity("""
1552+
{
1553+
"message": "elasticsearch"
1554+
}
1555+
""");
1556+
client().performRequest(doc);
1557+
}
1558+
for (int i = 0; i < numberOfIndicesWithoutField; i++) {
1559+
String indexName = testIndexName() + indexNum++;
1560+
// Create index without the text field
1561+
createIndex(indexName, Settings.EMPTY, """
1562+
{
1563+
"properties": {
1564+
"other_field": {
1565+
"type": "keyword"
1566+
}
1567+
}
1568+
}
1569+
""");
1570+
1571+
// Index a document in each index
1572+
Request doc = new Request("POST", indexName + "/_doc?refresh=true");
1573+
doc.setJsonEntity("""
1574+
{
1575+
"other_field": "elasticsearch"
1576+
}
1577+
""");
1578+
client().performRequest(doc);
1579+
}
1580+
1581+
// Query using MATCH function across all indices
1582+
String query = "FROM " + testIndexName() + "* | WHERE MATCH(message, \"elasticsearch\")";
1583+
Map<String, Object> result = runEsql(requestObjectBuilder().query(query));
1584+
1585+
// Verify the number of results equals the number of indices that have the field
1586+
var values = as(result.get("values"), ArrayList.class);
1587+
assertEquals(
1588+
"Expected " + numberOfIndicesWithField + " results from indices with the 'message' field",
1589+
numberOfIndicesWithField,
1590+
values.size()
1591+
);
1592+
1593+
// Clean up - delete all created indices
1594+
for (int i = 0; i < totalIndices; i++) {
1595+
assertTrue(deleteIndex(testIndexName() + i).isAcknowledged());
1596+
}
1597+
}
1598+
15321599
protected static Request prepareRequestWithOptions(RequestObjectBuilder requestObject, Mode mode) throws IOException {
15331600
requestObject.build();
15341601
Request request = prepareRequest(mode);

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,6 +1288,7 @@ public enum Cap {
12881288
*/
12891289
FIX_REPLACE_ALIASING_EVAL_WITH_PROJECT_SHADOWING,
12901290

1291+
FULL_TEXT_FUNCTIONS_ACCEPT_NULL_FIELD,
12911292
// Last capability should still have a comma for fewer merge conflicts when adding new ones :)
12921293
// This comment prevents the semicolon from being on the previous capability when Spotless formats the file.
12931294
;
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.function;
9+
10+
import org.apache.lucene.util.BytesRef;
11+
import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
12+
import org.elasticsearch.xpack.esql.core.expression.EntryExpression;
13+
import org.elasticsearch.xpack.esql.core.expression.Expression;
14+
import org.elasticsearch.xpack.esql.core.expression.Literal;
15+
import org.elasticsearch.xpack.esql.core.expression.MapExpression;
16+
import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
17+
import org.elasticsearch.xpack.esql.core.tree.Source;
18+
import org.elasticsearch.xpack.esql.core.type.DataType;
19+
import org.elasticsearch.xpack.esql.core.type.DataTypeConverter;
20+
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
import java.util.function.BiConsumer;
24+
import java.util.function.Consumer;
25+
26+
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
27+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isFoldable;
28+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isMapExpression;
29+
import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNotNull;
30+
31+
public class Options {
32+
33+
public static Expression.TypeResolution resolve(
34+
Expression options,
35+
Source source,
36+
TypeResolutions.ParamOrdinal paramOrdinal,
37+
Map<String, DataType> allowedOptions
38+
) {
39+
return resolve(
40+
options,
41+
source,
42+
paramOrdinal,
43+
null,
44+
(opts, optsMap) -> populateMap(opts, optsMap, source, paramOrdinal, allowedOptions)
45+
);
46+
}
47+
48+
public static Expression.TypeResolution resolve(
49+
Expression options,
50+
Source source,
51+
TypeResolutions.ParamOrdinal paramOrdinal,
52+
Map<String, DataType> allowedOptions,
53+
Consumer<Map<String, Object>> verifyOptions
54+
) {
55+
return resolve(
56+
options,
57+
source,
58+
paramOrdinal,
59+
verifyOptions,
60+
(opts, optsMap) -> populateMap(opts, optsMap, source, paramOrdinal, allowedOptions)
61+
);
62+
}
63+
64+
private static Expression.TypeResolution resolve(
65+
Expression options,
66+
Source source,
67+
TypeResolutions.ParamOrdinal paramOrdinal,
68+
Consumer<Map<String, Object>> verifyOptions,
69+
BiConsumer<MapExpression, Map<String, Object>> populateMap
70+
) {
71+
if (options != null) {
72+
Expression.TypeResolution resolution = isNotNull(options, source.text(), paramOrdinal);
73+
if (resolution.unresolved()) {
74+
return resolution;
75+
}
76+
// MapExpression does not have a DataType associated with it
77+
resolution = isMapExpression(options, source.text(), paramOrdinal);
78+
if (resolution.unresolved()) {
79+
return resolution;
80+
}
81+
try {
82+
Map<String, Object> optionsMap = new HashMap<>();
83+
populateMap.accept((MapExpression) options, optionsMap);
84+
if (verifyOptions != null) {
85+
verifyOptions.accept(optionsMap);
86+
}
87+
} catch (InvalidArgumentException e) {
88+
return new Expression.TypeResolution(e.getMessage());
89+
}
90+
}
91+
return Expression.TypeResolution.TYPE_RESOLVED;
92+
}
93+
94+
public static void populateMap(
95+
final MapExpression options,
96+
final Map<String, Object> optionsMap,
97+
final Source source,
98+
final TypeResolutions.ParamOrdinal paramOrdinal,
99+
final Map<String, DataType> allowedOptions
100+
) throws InvalidArgumentException {
101+
for (EntryExpression entry : options.entryExpressions()) {
102+
Expression optionExpr = entry.key();
103+
Expression valueExpr = entry.value();
104+
105+
Expression.TypeResolution optionNameResolution = isFoldable(optionExpr, source.text(), paramOrdinal);
106+
if (optionNameResolution.unresolved()) {
107+
throw new InvalidArgumentException(optionNameResolution.message());
108+
}
109+
110+
Object optionExprLiteral = ((Literal) optionExpr).value();
111+
String optionName = optionExprLiteral instanceof BytesRef br ? br.utf8ToString() : optionExprLiteral.toString();
112+
DataType dataType = allowedOptions.get(optionName);
113+
114+
// valueExpr could be a MapExpression, but for now functions only accept literal values in options
115+
if ((valueExpr instanceof Literal) == false) {
116+
throw new InvalidArgumentException(
117+
format(null, "Invalid option [{}] in [{}], expected a [{}] value", optionName, source.text(), dataType)
118+
);
119+
}
120+
121+
Object valueExprLiteral = ((Literal) valueExpr).value();
122+
String optionValue = valueExprLiteral instanceof BytesRef br ? br.utf8ToString() : valueExprLiteral.toString();
123+
// validate the optionExpr is supported
124+
if (dataType == null) {
125+
throw new InvalidArgumentException(
126+
format(null, "Invalid option [{}] in [{}], expected one of {}", optionName, source.text(), allowedOptions.keySet())
127+
);
128+
}
129+
try {
130+
optionsMap.put(optionName, DataTypeConverter.convert(optionValue, dataType));
131+
} catch (InvalidArgumentException e) {
132+
throw new InvalidArgumentException(
133+
format(null, "Invalid option [{}] in [{}], {}", optionName, source.text(), e.getMessage())
134+
);
135+
}
136+
}
137+
}
138+
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
2323
import org.elasticsearch.xpack.esql.core.expression.EntryExpression;
2424
import org.elasticsearch.xpack.esql.core.expression.Expression;
25+
import org.elasticsearch.xpack.esql.core.expression.Expressions;
2526
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
2627
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
2728
import org.elasticsearch.xpack.esql.core.expression.Literal;
@@ -315,7 +316,11 @@ private static FullTextFunction forEachFullTextFunctionParent(Expression conditi
315316
return null;
316317
}
317318

318-
public static void fieldVerifier(LogicalPlan plan, FullTextFunction function, Expression field, Failures failures) {
319+
protected void fieldVerifier(LogicalPlan plan, FullTextFunction function, Expression field, Failures failures) {
320+
// Accept null as a field
321+
if (Expressions.isGuaranteedNull(field)) {
322+
return;
323+
}
319324
var fieldAttribute = fieldAsFieldAttribute(field);
320325
if (fieldAttribute == null) {
321326
plan.forEachExpression(function.getClass(), m -> {
@@ -438,7 +443,7 @@ protected Map<String, Object> resolvedOptions() throws InvalidArgumentException
438443

439444
// TODO: this should likely be replaced by calls to FieldAttribute#fieldName; the MultiTypeEsField case looks
440445
// wrong if `fieldAttribute` is a subfield, e.g. `parent.child` - multiTypeEsField#getName will just return `child`.
441-
public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
446+
protected String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
442447
String fieldName = fieldAttribute.name();
443448
if (fieldAttribute.field() instanceof MultiTypeEsField multiTypeEsField) {
444449
// If we have multiple field types, we allow the query to be done, but getting the underlying field name
@@ -447,7 +452,7 @@ public static String getNameFromFieldAttribute(FieldAttribute fieldAttribute) {
447452
return fieldName;
448453
}
449454

450-
public static FieldAttribute fieldAsFieldAttribute(Expression field) {
455+
protected FieldAttribute fieldAsFieldAttribute(Expression field) {
451456
Expression fieldExpression = field;
452457
// Field may be converted to other data type (field_name :: data_type), so we need to check the original field
453458
if (fieldExpression instanceof AbstractConvertFunction convertFunction) {

0 commit comments

Comments
 (0)