Skip to content

Commit 14f0156

Browse files
carlosdelestcarlosdelest
committed
Update verifier and tests
1 parent cad830c commit 14f0156

File tree

5 files changed

+41
-103
lines changed

5 files changed

+41
-103
lines changed

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.action.index.IndexRequest;
1313
import org.elasticsearch.action.support.WriteRequest;
1414
import org.elasticsearch.common.settings.Settings;
15+
import org.elasticsearch.test.junit.annotations.TestLogging;
1516
import org.elasticsearch.xpack.esql.VerificationException;
1617
import org.elasticsearch.xpack.esql.action.AbstractEsqlIntegTestCase;
1718
import org.elasticsearch.xpack.esql.action.ColumnInfoImpl;
@@ -29,7 +30,7 @@
2930
import static org.hamcrest.CoreMatchers.containsString;
3031
import static org.hamcrest.Matchers.equalTo;
3132

32-
//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug")
33+
@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug")
3334
public class MatchOperatorIT extends AbstractEsqlIntegTestCase {
3435

3536
@Before
@@ -147,7 +148,10 @@ public void testWhereMatchEvalColumn() {
147148
""";
148149

149150
var error = expectThrows(VerificationException.class, () -> run(query));
150-
assertThat(error.getMessage(), containsString("[:] operator requires a mapped index field, found [upper_content]"));
151+
assertThat(
152+
error.getMessage(),
153+
containsString("[:] operator cannot operate on [upper_content], which is not a field from an index mapping")
154+
);
151155
}
152156

153157
public void testWhereMatchOverWrittenColumn() {
@@ -159,7 +163,10 @@ public void testWhereMatchOverWrittenColumn() {
159163
""";
160164

161165
var error = expectThrows(VerificationException.class, () -> run(query));
162-
assertThat(error.getMessage(), containsString("[:] operator requires a mapped index field, found [content]"));
166+
assertThat(
167+
error.getMessage(),
168+
containsString("[:] operator cannot operate on [content], which is not a field from an index mapping")
169+
);
163170
}
164171

165172
public void testWhereMatchAfterStats() {
@@ -195,7 +202,10 @@ public void testWhereMatchWithRow() {
195202
""";
196203

197204
var error = expectThrows(ElasticsearchException.class, () -> run(query));
198-
assertThat(error.getMessage(), containsString("[:] operator requires a mapped index field, found [content]"));
205+
assertThat(
206+
error.getMessage(),
207+
containsString("[:] operator cannot operate on [\"a brown fox\"], which is not a field from an index mapping")
208+
);
199209
}
200210

201211
public void testMatchWithinEval() {
@@ -215,7 +225,7 @@ public void testMatchWithNonTextField() {
215225
""";
216226

217227
var error = expectThrows(VerificationException.class, () -> run(query));
218-
assertThat(error.getMessage(), containsString("[:] operator requires a text or keyword field, but [id] has type [integer]"));
228+
assertThat(error.getMessage(), containsString("first argument of [id:\"fox\"] must be [string], found value [id] type [integer]"));
219229
}
220230

221231
private void createAndPopulateIndex() {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java‎

Lines changed: 10 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.analysis;
99

10-
import org.elasticsearch.common.logging.LoggerMessageFormat;
1110
import org.elasticsearch.xpack.esql.common.Failure;
1211
import org.elasticsearch.xpack.esql.core.capabilities.Unresolvable;
1312
import org.elasticsearch.xpack.esql.core.expression.Alias;
@@ -21,7 +20,6 @@
2120
import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
2221
import org.elasticsearch.xpack.esql.core.expression.function.Function;
2322
import org.elasticsearch.xpack.esql.core.expression.predicate.BinaryOperator;
24-
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MatchQueryPredicate;
2523
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.BinaryLogic;
2624
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Not;
2725
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
@@ -193,7 +191,6 @@ else if (p instanceof Lookup lookup) {
193191
checkBinaryComparison(p, failures);
194192
checkForSortableDataTypes(p, failures);
195193

196-
checkMatchQueryPredicates(p, failures);
197194
checkFullTextQueryFunctions(p, failures);
198195
});
199196
checkRemoteEnrich(plan, failures);
@@ -629,59 +626,6 @@ private static void checkRemoteEnrich(LogicalPlan plan, Set<Failure> failures) {
629626
});
630627
}
631628

632-
private static void checkMatchQueryPredicates(LogicalPlan plan, Set<Failure> failures) {
633-
if (plan instanceof Filter f) {
634-
Expression condition = f.condition();
635-
checkMatchPredicateArgumentTypes(plan, failures);
636-
checkCommandsBeforeExpression(
637-
plan,
638-
condition,
639-
MatchQueryPredicate.class,
640-
lp -> lp instanceof Limit == false,
641-
e -> "[:] operator",
642-
failures
643-
);
644-
checkNotPresentInDisjunctions(condition, MatchQueryPredicate.class, mqp -> "[:] operator", failures);
645-
} else {
646-
plan.forEachExpression(
647-
MatchQueryPredicate.class,
648-
ftf -> { failures.add(fail(ftf, "[:] operator is only supported in WHERE commands")); }
649-
);
650-
}
651-
}
652-
653-
/**
654-
* Currently any filter condition using MATCH needs to be pushed down to the Lucene query.
655-
* Conditions that use a combination of MATCH and ES|QL functions (e.g. `title MATCH "anna" OR DATE_EXTRACT("year", date) > 2010)
656-
* cannot be pushed down to Lucene.
657-
* Another condition is for MATCH to use index fields that have been mapped as text or keyword.
658-
* We are using canPushToSource at the Verifier level because we want to detect any condition that cannot be pushed down
659-
* early in the execution, rather than fail at the compute engine level.
660-
* In the future we will be able to handle MATCH at the compute and we will no longer need these checks.
661-
*/
662-
private static void checkMatchPredicateArgumentTypes(LogicalPlan plan, Set<Failure> failures) {
663-
if (plan instanceof Filter f) {
664-
Expression condition = f.condition();
665-
666-
condition.forEachDown(MatchQueryPredicate.class, mqp -> {
667-
var field = mqp.field();
668-
if (field instanceof FieldAttribute == false) {
669-
failures.add(fail(mqp, "[:] operator requires a mapped index field, found [" + field.sourceText() + "]"));
670-
}
671-
672-
if (DataType.isString(field.dataType()) == false) {
673-
var message = LoggerMessageFormat.format(
674-
null,
675-
"[:] operator requires a text or keyword field, but [{}] has type [{}]",
676-
field.sourceText(),
677-
field.dataType().esType()
678-
);
679-
failures.add(fail(mqp, message));
680-
}
681-
});
682-
}
683-
}
684-
685629
/**
686630
* Checks a whether a condition contains a disjunction with the specified typeToken. Adds to failure if it does.
687631
*
@@ -721,7 +665,7 @@ private static <E extends Expression> void checkNotPresentInDisjunctions(
721665
) {
722666
parentExpression.forEachDown(typeToken, ftp -> {
723667
failures.add(
724-
fail(or, "Invalid condition [{}]. " + elementName.apply(ftp) + " can't be used as part of an or condition", or.sourceText())
668+
fail(or, "Invalid condition [{}]. {} can't be used as part of an or condition", or.sourceText(), elementName.apply(ftp))
725669
);
726670
});
727671
}
@@ -740,22 +684,27 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Set<Failure> f
740684
condition,
741685
QueryString.class,
742686
lp -> (lp instanceof Filter || lp instanceof OrderBy || lp instanceof EsRelation),
743-
e -> "[" + e.functionName() + "] function",
687+
qsf -> "[" + qsf.functionName() + "] " + qsf.functionType(),
744688
failures
745689
);
746690
checkCommandsBeforeExpression(
747691
plan,
748692
condition,
749693
Match.class,
750694
lp -> (lp instanceof Limit == false),
751-
e -> "[" + e.functionName() + "] function",
695+
m -> "[" + m.functionName() + "] " + m.functionType(),
696+
failures
697+
);
698+
checkNotPresentInDisjunctions(
699+
condition,
700+
FullTextFunction.class,
701+
ftf -> "[" + ftf.functionName() + "] " + ftf.functionType(),
752702
failures
753703
);
754-
checkNotPresentInDisjunctions(condition, FullTextFunction.class, ftf -> "[" + ftf.functionName() + "] function", failures);
755704
checkFullTextFunctionsParents(condition, failures);
756705
} else {
757706
plan.forEachExpression(FullTextFunction.class, ftf -> {
758-
failures.add(fail(ftf, "[{}] function is only supported in WHERE commands", ftf.functionName()));
707+
failures.add(fail(ftf, "[{}] {} is only supported in WHERE commands", ftf.functionName(), ftf.functionType()));
759708
});
760709
}
761710
}

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,12 +1122,7 @@ public void testMatchFilter() throws Exception {
11221122
assumeTrue("Match operator is available just for snapshots", EsqlCapabilities.Cap.MATCH_OPERATOR_COLON.isEnabled());
11231123

11241124
assertEquals(
1125-
"1:63: [:] operator requires a mapped index field, found [name]",
1126-
error("from test | eval name = concat(first_name, last_name) | where name:\"Anna\"")
1127-
);
1128-
1129-
assertEquals(
1130-
"1:19: [:] operator requires a text or keyword field, but [salary] has type [integer]",
1125+
"1:19: first argument of [salary:\"100\"] must be [string], found value [salary] type [integer]",
11311126
error("from test | where salary:\"100\"")
11321127
);
11331128

@@ -1141,11 +1136,6 @@ public void testMatchFilter() throws Exception {
11411136
"1:51: Invalid condition [first_name:\"Anna\" OR new_salary > 100]. " + "[:] operator can't be used as part of an or condition",
11421137
error("from test | eval new_salary = salary + 10 | where first_name:\"Anna\" OR new_salary > 100")
11431138
);
1144-
1145-
assertEquals(
1146-
"1:45: [:] operator requires a mapped index field, found [fn]",
1147-
error("from test | rename first_name as fn | where fn:\"Anna\"")
1148-
);
11491139
}
11501140

11511141
public void testMatchFunctionNotAllowedAfterCommands() throws Exception {

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5590,15 +5590,15 @@ public void testMatchWithNonIndexedColumnCurrentlyUnsupported() {
55905590
from test | eval initial = substring(first_name, 1) | where match(initial, "A")"""));
55915591
assertTrue(e.getMessage().startsWith("Found "));
55925592
assertEquals(
5593-
"1:67: [MATCH] cannot operate on [initial], which is not a field from an index mapping",
5593+
"1:67: [MATCH] function cannot operate on [initial], which is not a field from an index mapping",
55945594
e.getMessage().substring(header.length())
55955595
);
55965596

55975597
e = expectThrows(VerificationException.class, () -> plan("""
55985598
from test | eval text=concat(first_name, last_name) | where match(text, "cat")"""));
55995599
assertTrue(e.getMessage().startsWith("Found "));
56005600
assertEquals(
5601-
"1:67: [MATCH] cannot operate on [text], which is not a field from an index mapping",
5601+
"1:67: [MATCH] function cannot operate on [text], which is not a field from an index mapping",
56025602
e.getMessage().substring(header.length())
56035603
);
56045604
}
@@ -5611,7 +5611,7 @@ public void testMatchFunctionIsNotNullable() {
56115611
VerificationException ve = expectThrows(VerificationException.class, () -> plan(queryText));
56125612
assertThat(
56135613
ve.getMessage(),
5614-
containsString("[MATCH] cannot operate on [text::keyword], which is not a field from an index mapping")
5614+
containsString("[MATCH] function cannot operate on [text::keyword], which is not a field from an index mapping")
56155615
);
56165616
}
56175617

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
2222
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
2323
import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute;
24-
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MatchQueryPredicate;
2524
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Not;
2625
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
2726
import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison;
@@ -30,6 +29,7 @@
3029
import org.elasticsearch.xpack.esql.expression.UnresolvedNamePattern;
3130
import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction;
3231
import org.elasticsearch.xpack.esql.expression.function.aggregate.FilteredExpression;
32+
import org.elasticsearch.xpack.esql.expression.function.fulltext.Match;
3333
import org.elasticsearch.xpack.esql.expression.function.scalar.string.RLike;
3434
import org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLike;
3535
import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Add;
@@ -2294,21 +2294,21 @@ public void testMetricWithGroupKeyAsAgg() {
22942294
public void testMatchOperator() {
22952295
var plan = statement("FROM test | WHERE field:\"value\"");
22962296
var filter = as(plan, Filter.class);
2297-
var match = (MatchQueryPredicate) filter.condition();
2297+
var match = (Match) filter.condition();
22982298
var matchField = (UnresolvedAttribute) match.field();
22992299
assertThat(matchField.name(), equalTo("field"));
2300-
assertThat(match.query(), equalTo("value"));
2300+
assertThat(match.query().fold(), equalTo("value"));
23012301
assertNull(match.boost());
23022302
assertNull(match.fuzziness());
23032303
}
23042304

23052305
public void testMatchOperatorWithBoosting() {
23062306
var plan = statement("FROM test | WHERE field^2.1:\"value\"");
23072307
var filter = as(plan, Filter.class);
2308-
var match = (MatchQueryPredicate) filter.condition();
2308+
var match = (Match) filter.condition();
23092309
var matchField = (UnresolvedAttribute) match.field();
23102310
assertThat(matchField.name(), equalTo("field"));
2311-
assertThat(match.query(), equalTo("value"));
2311+
assertThat(match.query().fold(), equalTo("value"));
23122312
assertThat(match.boost(), equalTo(2.1));
23132313
assertNull(match.fuzziness());
23142314
}
@@ -2324,44 +2324,33 @@ public void testMatchOperatorWithFuzziness() {
23242324
public void checkMatchOperatorWithFuzziness(String fuzzinessValue, Fuzziness fuzinessExpected) {
23252325
var plan = statement("FROM test | WHERE field:\"value\"" + fuzzinessValue);
23262326
var filter = as(plan, Filter.class);
2327-
var match = (MatchQueryPredicate) filter.condition();
2327+
var match = (Match) filter.condition();
23282328
var matchField = (UnresolvedAttribute) match.field();
23292329
assertThat(matchField.name(), equalTo("field"));
2330-
assertThat(match.query(), equalTo("value"));
2330+
assertThat(match.query().fold(), equalTo("value"));
23312331
assertNull(match.boost());
23322332
assertThat(match.fuzziness(), equalTo(fuzinessExpected));
23332333
}
23342334

23352335
public void testMatchOperatorWithBoostingAndFuzziness() {
23362336
var plan = statement("FROM test | WHERE field^1.2:\"value\"~1");
23372337
var filter = as(plan, Filter.class);
2338-
var match = (MatchQueryPredicate) filter.condition();
2338+
var match = (Match) filter.condition();
23392339
var matchField = (UnresolvedAttribute) match.field();
23402340
assertThat(matchField.name(), equalTo("field"));
2341-
assertThat(match.query(), equalTo("value"));
2341+
assertThat(match.query().fold(), equalTo("value"));
23422342
assertThat(match.boost(), equalTo(1.2));
23432343
assertThat(match.fuzziness(), equalTo(Fuzziness.ONE));
23442344
}
23452345

23462346
public void testInvalidMatchOperator() {
2347-
expectError("from test | WHERE field:", "line 1:25: missing QUOTED_STRING at '<EOF>");
2348-
expectError("from test | WHERE field:2.3", "line 1:25: mismatched input '2.3' expecting QUOTED_STRING");
2349-
expectError("from test | WHERE field^a:\"value\"", "line 1:25: mismatched input 'a' expecting {DECIMAL_LITERAL, '+', '-'}");
2350-
expectError(
2351-
"from test | WHERE field:\"value\"^1.2",
2352-
"line 1:32: mismatched input '^' expecting {<EOF>, '|', 'and', 'or', DEV_TILDE}"
2353-
);
2354-
expectError("from test | WHERE field:\"value\"~3", "line 1:33: Invalid fuzziness value: [3]");
2355-
expectError("from test | WHERE field:\"value\"~FUZZY", "line 1:33: extraneous input 'FUZZY' expecting <EOF>");
2356-
expectError(
2357-
"from test | WHERE field:\"value\"~AUTO1,2",
2358-
"line 1:33: mismatched input 'AUTO1' expecting {<EOF>, '|', INTEGER_LITERAL, 'and', AUTO, 'or'}"
2359-
);
2347+
expectError("from test | WHERE field:", "line 1:25: mismatched input '<EOF>' expecting {QUOTED_STRING, ");
2348+
expectError("from test | WHERE field^:\"value\"", "line 1:25: extraneous input ':' expecting {QUOTED_STRING,");
2349+
expectError("from test | WHERE field:\"value\"~AUTO:1,2,3", "line 1:41: mismatched input ',' expecting {<EOF>, '|', 'and', 'or'}");
23602350
expectError("from test | WHERE field:\"value\"~AUTO:a,b", "line 1:38: mismatched input 'a' expecting INTEGER_LITERAL");
23612351
expectError(
23622352
"from test | WHERE field:\"value\"~AUTO:1-5",
23632353
"line 1:39: mismatched input '-' expecting {<EOF>, '|', 'and', ',', 'or'}"
23642354
);
2365-
expectError("from test | WHERE field:\"value\"~AUTO:1", "line 1:33: Invalid fuzziness value: [AUTO:1]");
23662355
}
23672356
}

0 commit comments

Comments
 (0)