Skip to content

Commit 0766c5d

Browse files
carlosdelestcarlosdelest
committed
Remove restrictions for disjunctions in full text functions
1 parent a7fdc10 commit 0766c5d

File tree

5 files changed

+235
-19
lines changed

5 files changed

+235
-19
lines changed

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-function.csv-spec‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,62 @@ book_no:keyword | title:text
115115
7140 |The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1)
116116
;
117117

118+
matchWithDisjunction
119+
required_capability: match_function
120+
121+
from books
122+
| where match(author, "Vonnegut") or match(author, "Guinane")
123+
| keep book_no, author;
124+
ignoreOrder:true
125+
126+
book_no:keyword | author:text
127+
2464 | Kurt Vonnegut
128+
6970 | Edith Vonnegut
129+
8956 | Kurt Vonnegut
130+
3950 | Kurt Vonnegut
131+
4382 | Carole Guinane
132+
;
133+
134+
matchWithDisjunctionAndFiltersConjunction
135+
required_capability: match_function
136+
137+
from books
138+
| where (match(author, "Vonnegut") or match(author, "Guinane")) and year > 1997
139+
| keep book_no, author, year;
140+
ignoreOrder:true
141+
142+
book_no:keyword | author:text | year:integer
143+
6970 | Edith Vonnegut | 1998
144+
4382 | Carole Guinane | 2001
145+
;
146+
147+
matchWithDisjunctionAndConjunction
148+
required_capability: match_function
149+
150+
from books
151+
| where (match(author, "Vonnegut") or match(author, "Marquez")) and match(description, "realism")
152+
| keep book_no;
153+
154+
book_no:keyword
155+
4814
156+
;
157+
158+
matchWithDisjunctionIncludingConjunction
159+
required_capability: match_function
160+
161+
from books
162+
| where match(author, "Vonnegut") or (match(author, "Marquez") and match(description, "realism"))
163+
| keep book_no;
164+
ignoreOrder:true
165+
166+
book_no:keyword
167+
2464
168+
6970
169+
4814
170+
8956
171+
3950
172+
;
173+
118174
matchWithFunctionPushedToLucene
119175
required_capability: match_function
120176

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-operator.csv-spec‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,64 @@ book_no:keyword | title:text
102102
7140 |The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1)
103103
;
104104

105+
106+
matchWithDisjunction
107+
required_capability: match_function
108+
109+
from books
110+
| where author : "Vonnegut" or author : "Guinane"
111+
| keep book_no, author;
112+
ignoreOrder:true
113+
114+
book_no:keyword | author:text
115+
2464 | Kurt Vonnegut
116+
6970 | Edith Vonnegut
117+
8956 | Kurt Vonnegut
118+
3950 | Kurt Vonnegut
119+
4382 | Carole Guinane
120+
;
121+
122+
matchWithDisjunctionAndFiltersConjunction
123+
required_capability: match_function
124+
125+
from books
126+
| where (author : "Vonnegut" or author : "Guinane") and year > 1997
127+
| keep book_no, author, year;
128+
ignoreOrder:true
129+
130+
book_no:keyword | author:text | year:integer
131+
6970 | Edith Vonnegut | 1998
132+
4382 | Carole Guinane | 2001
133+
;
134+
135+
matchWithDisjunctionAndConjunction
136+
required_capability: match_function
137+
138+
from books
139+
| where (author : "Vonnegut" or author : "Marquez") and description : "realism"
140+
| keep book_no;
141+
142+
book_no:keyword
143+
4814
144+
;
145+
146+
matchWithDisjunctionIncludingConjunction
147+
required_capability: match_function
148+
149+
from books
150+
| where author : "Vonnegut") or (author : "Marquez" and description : "realism")
151+
| keep book_no;
152+
ignoreOrder:true
153+
154+
book_no:keyword
155+
2464
156+
6970
157+
4814
158+
8956
159+
3950
160+
;
161+
162+
105163
matchWithFunctionPushedToLucene
106164
required_capability: match_operator_colon
107165

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

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
2929
import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison;
3030
import org.elasticsearch.xpack.esql.core.type.DataType;
31+
import org.elasticsearch.xpack.esql.core.util.Holder;
3132
import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
3233
import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
3334
import org.elasticsearch.xpack.esql.expression.function.aggregate.FilteredExpression;
@@ -773,35 +774,53 @@ private static void checkRemoteEnrich(LogicalPlan plan, Set<Failure> failures) {
773774
* @param typeNameProvider provider for the type name to add in the failure message
774775
* @param failures failures collection to add to
775776
*/
776-
private static void checkNotPresentInDisjunctions(
777+
private static void checkFullTextSearchDisjunctions(
777778
Expression condition,
778779
java.util.function.Function<FullTextFunction, String> typeNameProvider,
779780
Set<Failure> failures
780781
) {
781782
condition.forEachUp(Or.class, or -> {
782-
checkNotPresentInDisjunctions(or.left(), or, typeNameProvider, failures);
783-
checkNotPresentInDisjunctions(or.right(), or, typeNameProvider, failures);
783+
boolean left = checkFullTextSearchInDisjunctions(or.left());
784+
boolean right = checkFullTextSearchInDisjunctions(or.right());
785+
if (left ^ right) {
786+
Holder<String> elementName = new Holder<>();
787+
if (right) {
788+
or.left().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
789+
} else {
790+
or.right().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
791+
}
792+
failures.add(
793+
fail(
794+
or,
795+
"Invalid condition [{}]. {} can be used as part of an OR condition, "
796+
+ "but only if other full text functions are used as part of the condition",
797+
or.sourceText(),
798+
elementName.get()
799+
)
800+
);
801+
}
784802
});
785803
}
786804

787805
/**
788-
* Checks whether a condition contains a disjunction with the specified typeToken. Adds to failure if it does.
806+
* Checks whether an expression contains just full text functions or negations (NOT) and combinations (AND, OR) of full text functions
789807
*
790-
* @param parentExpression parent expression to add to the failure message
791-
* @param or disjunction that is being checked
792-
* @param failures failures collection to add to
808+
* @param expression parent expression to add to the failure message
809+
* @return true if all children are full text functions or negations of full text functions, false otherwise
793810
*/
794-
private static void checkNotPresentInDisjunctions(
795-
Expression parentExpression,
796-
Or or,
797-
java.util.function.Function<FullTextFunction, String> elementName,
798-
Set<Failure> failures
799-
) {
800-
parentExpression.forEachDown(FullTextFunction.class, ftp -> {
801-
failures.add(
802-
fail(or, "Invalid condition [{}]. {} can't be used as part of an or condition", or.sourceText(), elementName.apply(ftp))
803-
);
804-
});
811+
private static boolean checkFullTextSearchInDisjunctions(Expression expression) {
812+
if (expression instanceof FullTextFunction) {
813+
return false;
814+
} else if (expression instanceof Not || expression instanceof BinaryLogic) {
815+
for (Expression child : expression.children()) {
816+
if (checkFullTextSearchInDisjunctions(child)) {
817+
return true;
818+
}
819+
}
820+
return false;
821+
}
822+
823+
return true;
805824
}
806825

807826
/**
@@ -871,7 +890,7 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Set<Failure> f
871890
m -> "[" + m.functionName() + "] " + m.functionType(),
872891
failures
873892
);
874-
checkNotPresentInDisjunctions(condition, ftf -> "[" + ftf.functionName() + "] " + ftf.functionType(), failures);
893+
checkFullTextSearchDisjunctions(condition, ftf -> "[" + ftf.functionName() + "] " + ftf.functionType(), failures);
875894
checkFullTextFunctionsParents(condition, failures);
876895
} else {
877896
plan.forEachExpression(FullTextFunction.class, ftf -> {

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,6 +1453,47 @@ private void checkWithDisjunctions(String functionName, String functionInvocatio
14531453
);
14541454
}
14551455

1456+
public void testFullTextFunctionsDisjunctions() {
1457+
checkWithFullTextFunctionsDisjunctions("MATCH", "match(last_name, \"Smith\")", "function");
1458+
checkWithFullTextFunctionsDisjunctions(":", "last_name : \"Smith\"", "operator");
1459+
checkWithFullTextFunctionsDisjunctions("QSTR", "qstr(\"last_name: Smith\")", "function");
1460+
1461+
assumeTrue("KQL function capability not available", EsqlCapabilities.Cap.KQL_FUNCTION.isEnabled());
1462+
checkWithFullTextFunctionsDisjunctions("KQL", "kql(\"last_name: Smith\")", "function");
1463+
}
1464+
1465+
private void checkWithFullTextFunctionsDisjunctions(String functionName, String functionInvocation, String functionType) {
1466+
passes("from test | where " + functionInvocation + " or match(first_name, \"Anna\")");
1467+
passes("from test | where " + functionInvocation + " or not match(first_name, \"Anna\")");
1468+
passes("from test | where (" + functionInvocation + " or match(first_name, \"Anna\")) and length(first_name) > 10");
1469+
passes("from test | where (" + functionInvocation + " or match(first_name, \"Anna\")) and match(last_name, \"Smith\")");
1470+
passes("from test | where " + functionInvocation + " or (match(first_name, \"Anna\") and match(last_name, \"Smith\"))");
1471+
1472+
assertEquals(
1473+
LoggerMessageFormat.format(
1474+
null,
1475+
"1:19: Invalid condition [{} or length(first_name) > 10]. [{}] {} can be used as part of an OR condition, "
1476+
+ "but only if other full text functions are used as part of the condition",
1477+
functionInvocation,
1478+
functionName,
1479+
functionType
1480+
),
1481+
error("from test | where " + functionInvocation + " or length(first_name) > 10")
1482+
);
1483+
assertEquals(
1484+
LoggerMessageFormat.format(
1485+
null,
1486+
"1:19: Invalid condition [{} or (match(last_name, \"Anneke\") and length(first_name) > 10)]."
1487+
+ " [{}] {} can be used as part of an OR condition, "
1488+
+ "but only if other full text functions are used as part of the condition",
1489+
functionInvocation,
1490+
functionName,
1491+
functionType
1492+
),
1493+
error("from test | where " + functionInvocation + " or (match(last_name, \"Anneke\") and length(first_name) > 10)")
1494+
);
1495+
}
1496+
14561497
public void testQueryStringFunctionWithNonBooleanFunctions() {
14571498
checkFullTextFunctionsWithNonBooleanFunctions("QSTR", "qstr(\"first_name: Anna\")", "function");
14581499
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.index.query.MatchQueryBuilder;
2222
import org.elasticsearch.index.query.QueryBuilder;
2323
import org.elasticsearch.index.query.QueryBuilders;
24+
import org.elasticsearch.index.query.QueryStringQueryBuilder;
2425
import org.elasticsearch.index.query.RangeQueryBuilder;
2526
import org.elasticsearch.index.query.SearchExecutionContext;
2627
import org.elasticsearch.license.XPackLicenseState;
@@ -57,6 +58,7 @@
5758
import org.elasticsearch.xpack.esql.plan.physical.EvalExec;
5859
import org.elasticsearch.xpack.esql.plan.physical.ExchangeExec;
5960
import org.elasticsearch.xpack.esql.plan.physical.FieldExtractExec;
61+
import org.elasticsearch.xpack.esql.plan.physical.FilterExec;
6062
import org.elasticsearch.xpack.esql.plan.physical.LimitExec;
6163
import org.elasticsearch.xpack.esql.plan.physical.LocalSourceExec;
6264
import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan;
@@ -1543,6 +1545,46 @@ public void testMultipleMatchFilterPushdown() {
15431545
assertThat(actualLuceneQuery.toString(), is(expectedLuceneQuery.toString()));
15441546
}
15451547

1548+
public void testFullTextFunctionsDisjunctionPushdown() {
1549+
String query = """
1550+
from test
1551+
| where (match(first_name, "Anna") or qstr("first_name: Anneke")) and last_name: "Smith"
1552+
| sort emp_no
1553+
""";
1554+
var plan = plannerOptimizer.plan(query);
1555+
var topNExec = as(plan, TopNExec.class);
1556+
var exchange = as(topNExec.child(), ExchangeExec.class);
1557+
var project = as(exchange.child(), ProjectExec.class);
1558+
var fieldExtract = as(project.child(), FieldExtractExec.class);
1559+
var actualLuceneQuery = as(fieldExtract.child(), EsQueryExec.class).query();
1560+
var expectedLuceneQuery = new BoolQueryBuilder().must(
1561+
new BoolQueryBuilder().should(new MatchQueryBuilder("first_name", "Anna").lenient(true))
1562+
.should(new QueryStringQueryBuilder("first_name: Anneke"))
1563+
).must(new MatchQueryBuilder("last_name", "Smith").lenient(true));
1564+
assertThat(actualLuceneQuery.toString(), is(expectedLuceneQuery.toString()));
1565+
}
1566+
1567+
public void testFullTextFunctionsDisjunctionWithFiltersPushdown() {
1568+
String query = """
1569+
from test
1570+
| where (first_name:"Anna" or first_name:"Anneke") and last_name:"first_name) > 5
1571+
| sort emp_no
1572+
""";
1573+
var plan = plannerOptimizer.plan(query);
1574+
var topNExec = as(plan, TopNExec.class);
1575+
var exchange = as(topNExec.child(), ExchangeExec.class);
1576+
var project = as(exchange.child(), ProjectExec.class);
1577+
var fieldExtract = as(project.child(), FieldExtractExec.class);
1578+
var secondTopNExec = as(fieldExtract.child(), TopNExec.class);
1579+
var secondFieldExtract = as(secondTopNExec.child(), FieldExtractExec.class);
1580+
var filterExec = as(secondFieldExtract.child(), FilterExec.class);
1581+
var thirdFilterExtract = as(filterExec.child(), FieldExtractExec.class);
1582+
var actualLuceneQuery = as(thirdFilterExtract.child(), EsQueryExec.class).query();
1583+
var expectedLuceneQuery = new BoolQueryBuilder().should(new MatchQueryBuilder("first_name", "Anna").lenient(true))
1584+
.should(new MatchQueryBuilder("first_name", "Anneke").lenient(true));
1585+
assertThat(actualLuceneQuery.toString(), is(expectedLuceneQuery.toString()));
1586+
}
1587+
15461588
/**
15471589
* Expecting
15481590
* LimitExec[1000[INTEGER]]

0 commit comments

Comments
 (0)