Full text functions can't be used as part of OR conditions

Author: carlosdelest

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

@@ -102,21 +102,6 @@ book_no:keyword | author:text | stars:
 3293 | Danny Faulkner | 2
 ;
 
-matchWithPushableDisjunction
-required_capability: match_function
-
-from books 
-| where match(title, "Return") or book_no == "7140" or year > 2020
-| keep book_no, title;
-ignoreOrder:true
-
-book_no:keyword | title:text
-2714 | Return of the King Being the Third Part of The Lord of the Rings
-7350 | Return of the Shadow 
-7140 | The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1) 
-6818 | Hadji Murad 
-;
-
 matchWithConjunction
 required_capability: match_function
 
@@ -151,7 +136,7 @@ from books
 ignoreOrder:true
 
 book_no:keyword | title:text
-4023 |A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings
+4023 | A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings
 ;
 
 matchWithMultipleWhereClauses
@@ -191,30 +176,12 @@ testMultiValuedFieldWithConjunction
 required_capability: match_function
 
 from employees 
-| where (match(job_positions, "Data Scientist") or match(job_positions, "Support Engineer")) and gender == "F"
+| where match(job_positions, "Data Scientist") and match(job_positions, "Support Engineer")
 | keep emp_no, first_name, last_name;
 ignoreOrder:true
 
 emp_no:integer | first_name:keyword | last_name:keyword 
-10023 | Bojan | Montemayor
-10041 | Uri | Lenart
-10044 | Mingsen | Casley
-10053 | Sanjiv | Zschoche
-10069 | Margareta | Bierman
-;
-
-testFieldWithKeywordComparisonDisjunction
-required_capability: match_function
-
-from employees 
-| where match(last_name, "Kalloufi") or last_name == "Piveteau"
-| keep emp_no, first_name, last_name;
-ignoreOrder:true
-
-emp_no:integer | first_name:keyword | last_name:keyword 
-10008 | Saniya | Kalloufi 
-10010 | Duangkaew | Piveteau 
-10084 | Tuval | Kalloufi 
+10043 | Yishay | Tzvieli 
 ;
 
 testMatchAndQueryStringFunctions

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

@@ -49,21 +49,6 @@ book_no:keyword | title:text
 7350 | Return of the Shadow
 ;
 
-qstrWithPushableDisjunction
-required_capability: qstr_function
-
-from books 
-| where qstr("title:Return") or book_no == "7140" or year > 2020
-| keep book_no, title;
-ignoreOrder:true
-
-book_no:keyword | title:text
-2714 | Return of the King Being the Third Part of The Lord of the Rings
-7350 | Return of the Shadow 
-7140 | The Lord of the Rings Poster Collection: Six Paintings by Alan Lee (No. 1) 
-6818 | Hadji Murad 
-;
-
 qstrWithConjunction
 required_capability: qstr_function
 

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

@@ -698,26 +698,21 @@ private static void checkCommandsBeforeMatchFunction(LogicalPlan plan, Expressio
 
  private static void checkFullTextFunctionsConditions(Expression condition, Set<Failure> failures) {
  condition.forEachUp(Or.class, or -> {
- Expression left = or.left();
- Expression right = or.right();
- checkDisjunction(failures, or, left, right);
- checkDisjunction(failures, or, right, left);
+ checkFullTextFunctionInDisjunction(failures, or, or.left());
+ checkFullTextFunctionInDisjunction(failures, or, or.right());
  });
  }
 
- private static void checkDisjunction(Set<Failure> failures, Or or, Expression left, Expression right) {
+ private static void checkFullTextFunctionInDisjunction(Set<Failure> failures, Or or, Expression left) {
  left.forEachDown(FullTextFunction.class, ftf -> {
- if (canPushToSource(right, x -> false) == false) {
- failures.add(
- fail(
- or,
- "Invalid condition [{}]. Function {} can't be used as part of an or condition that includes [{}]",
- or.sourceText(),
- ftf.functionName(),
- right.sourceText()
- )
- );
- }
+ failures.add(
+ fail(
+ or,
+ "Invalid condition [{}]. Function {} can't be used as part of an or condition",
+ or.sourceText(),
+ ftf.functionName()
+ )
+ );
  });
  }
 

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

@@ -1196,24 +1196,23 @@ public void testQueryStringFunctionArgNotNullOrConstant() throws Exception {
  // Other value types are tested in QueryStringFunctionTests
  }
 
- public void testQueryStringWithDisjunctionsThatCannotBePushedDown() {
+ public void testQueryStringWithDisjunctions() {
  assumeTrue("skipping because QSTR is not enabled", EsqlCapabilities.Cap.QSTR_FUNCTION.isEnabled());
 
- checkWithDisjunctionsThatCannotBePushedDown("QSTR", "qstr(\"first_name: Anna\")");
+ checkWithDisjunctions("QSTR", "qstr(\"first_name: Anna\")");
  }
 
- public void testMatchWithDisjunctionsThatCannotBePushedDown() {
+ public void testMatchWithDisjunctions() {
  assumeTrue("skipping because MATCH is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
 
- checkWithDisjunctionsThatCannotBePushedDown("MATCH", "match(first_name, \"Anna\")");
+ checkWithDisjunctions("MATCH", "match(first_name, \"Anna\")");
  }
 
- private void checkWithDisjunctionsThatCannotBePushedDown(String functionName, String functionInvocation) {
+ private void checkWithDisjunctions(String functionName, String functionInvocation) {
  assertEquals(
  LoggerMessageFormat.format(
  null,
- "1:19: Invalid condition [{} or length(first_name) > 12]. "
- + "Function {} can't be used as part of an or condition that includes [length(first_name) > 12]",
+ "1:19: Invalid condition [{} or length(first_name) > 12]. " + "Function {} can't be used as part of an or condition",
  functionInvocation,
  functionName
  ),
@@ -1223,7 +1222,7 @@ private void checkWithDisjunctionsThatCannotBePushedDown(String functionName, St
  LoggerMessageFormat.format(
  null,
  "1:19: Invalid condition [({} and first_name is not null) or (length(first_name) > 12 and first_name is null)]. "
- + "Function {} can't be used as part of an or condition that includes [length(first_name) > 12 and first_name is null]",
+ + "Function {} can't be used as part of an or condition",
  functionInvocation,
  functionName
  ),
@@ -1233,6 +1232,16 @@ private void checkWithDisjunctionsThatCannotBePushedDown(String functionName, St
  + " and first_name is not null) or (length(first_name) > 12 and first_name is null)"
  )
  );
+ assertEquals(
+ LoggerMessageFormat.format(
+ null,
+ "1:19: Invalid condition [({} and first_name is not null) or first_name is null]. "
+ + "Function {} can't be used as part of an or condition",
+ functionInvocation,
+ functionName
+ ),
+ error("from test | where (" + functionInvocation + " and first_name is not null) or first_name is null")
+ );
  }
 
  public void testQueryStringFunctionWithNonBooleanFunctions() {

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

@@ -11,7 +11,6 @@
 
 import org.apache.lucene.search.IndexSearcher;
 import org.elasticsearch.Build;
-import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.IndexMode;
@@ -437,40 +436,6 @@ public void testQueryStringFunctionConjunctionWhereOperands() {
  assertThat(query.query().toString(), is(expected.toString()));
  }
 
- /**
- * Expecting
- * LimitExec[1000[INTEGER]]
- * \_ExchangeExec[[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gender{f}#5, job{f}#10, job.raw{f}#11, languages{f}#6, last_n
- * ame{f}#7, long_noidx{f}#12, salary{f}#8],false]
- * \_ProjectExec[[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gender{f}#5, job{f}#10, job.raw{f}#11, languages{f}#6, last_n
- * ame{f}#7, long_noidx{f}#12, salary{f}#8]]
- * \_FieldExtractExec[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gen]
- * \_EsQueryExec[test], indexMode[standard], query[{"bool":{"should":[{"query_string":{"query":"last_name: Smith","fields":[]}},
- * {"esql_single_value":{"field":"emp_no","next":{"range":{"emp_no":{"gt":10010,"boost":1.0}}},"source":"emp_no > 10010@2:37"}}],
- * "boost":1.0}}][_doc{f}#13], limit[1000], sort[] estimatedRowSize[324]
- */
- public void testQueryStringFunctionDisjunctionWhereClauses() {
- assumeTrue("skipping because QSTR_FUNCTION is not enabled", EsqlCapabilities.Cap.QSTR_FUNCTION.isEnabled());
- String queryText = """
- from test
- | where qstr("last_name: Smith") or emp_no > 10010
- """;
- var plan = plannerOptimizer.plan(queryText, IS_SV_STATS);
-
- var limit = as(plan, LimitExec.class);
- var exchange = as(limit.child(), ExchangeExec.class);
- var project = as(exchange.child(), ProjectExec.class);
- var field = as(project.child(), FieldExtractExec.class);
- var query = as(field.child(), EsQueryExec.class);
- assertThat(query.limit().fold(), is(1000));
-
- Source filterSource = new Source(2, 36, "emp_no > 10000");
- var range = wrapWithSingleQuery(queryText, QueryBuilders.rangeQuery("emp_no").gt(10010), "emp_no", filterSource);
- var queryString = QueryBuilders.queryStringQuery("last_name: Smith");
- var expected = QueryBuilders.boolQuery().should(queryString).should(range);
- assertThat(query.query().toString(), is(expected.toString()));
- }
-
  /**
  * Expecting
  * LimitExec[1000[INTEGER]]
@@ -637,40 +602,6 @@ public void testMatchFunctionConjunctionWhereOperands() {
  assertThat(query.query().toString(), is(expected.toString()));
  }
 
- /**
- * Expecting
- * LimitExec[1000[INTEGER]]
- * \_ExchangeExec[[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gender{f}#5, job{f}#10, job.raw{f}#11, languages{f}#6, last_n
- * ame{f}#7, long_noidx{f}#12, salary{f}#8],false]
- * \_ProjectExec[[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gender{f}#5, job{f}#10, job.raw{f}#11, languages{f}#6, last_n
- * ame{f}#7, long_noidx{f}#12, salary{f}#8]]
- * \_FieldExtractExec[_meta_field{f}#9, emp_no{f}#3, first_name{f}#4, gen]
- * \_EsQueryExec[test], indexMode[standard], query[{"bool":{"should":[{"match":{"last_name":{"query":"Smith"}}},
- * {"esql_single_value":{"field":"emp_no","next":{"range":{"emp_no":{"gt":10010,"boost":1.0}}},"source":"emp_no > 10010@2:38"}}],
- * "boost":1.0}}][_doc{f}#14], limit[1000], sort[] estimatedRowSize[324]
- */
- public void testMatchFunctionDisjunctionWhereClauses() {
- assumeTrue("skipping because MATCH function is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
- String queryText = """
- from test
- | where match(last_name, "Smith") or emp_no > 10010
- """;
- var plan = plannerOptimizer.plan(queryText, IS_SV_STATS);
-
- var limit = as(plan, LimitExec.class);
- var exchange = as(limit.child(), ExchangeExec.class);
- var project = as(exchange.child(), ProjectExec.class);
- var field = as(project.child(), FieldExtractExec.class);
- var query = as(field.child(), EsQueryExec.class);
- assertThat(query.limit().fold(), is(1000));
-
- Source filterSource = new Source(2, 37, "emp_no > 10000");
- var range = wrapWithSingleQuery(queryText, QueryBuilders.rangeQuery("emp_no").gt(10010), "emp_no", filterSource);
- var queryString = QueryBuilders.matchQuery("last_name", "Smith");
- var expected = QueryBuilders.boolQuery().should(queryString).should(range);
- assertThat(query.query().toString(), is(expected.toString()));
- }
-
  /**
  * Expecting
  * LimitExec[1000[INTEGER]]
@@ -742,73 +673,6 @@ public void testMatchFunctionMultipleWhereClauses() {
  assertThat(query.query().toString(), is(expected.toString()));
  }
 
- public void testQueryStringWithDisjunctionsThatCannotBePushedDown() {
- assumeTrue("skipping because QSTR is not enabled", EsqlCapabilities.Cap.QSTR_FUNCTION.isEnabled());
-
- checkWithDisjunctionsThatCannotBePushedDown("QSTR", "qstr(\"first_name: Anna\")");
- }
-
- public void testMatchWithDisjunctionsThatCannotBePushedDown() {
- assumeTrue("skipping because MATCH function is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
-
- checkWithDisjunctionsThatCannotBePushedDown("MATCH", "match(first_name, \"Anna\")");
- }
-
- private void checkWithDisjunctionsThatCannotBePushedDown(String functionName, String functionInvocation) {
- VerificationException ve = expectThrows(
- VerificationException.class,
- () -> plannerOptimizer.plan("from test | where " + functionInvocation + " or length(first_name) > 12", IS_SV_STATS)
- );
- assertThat(
- ve.getMessage(),
- containsString(
- LoggerMessageFormat.format(
- null,
- "1:19: Invalid condition [{} or length(first_name) > 12]. "
- + "Function {} can't be used as part of an or condition that includes [length(first_name) > 12]",
- functionInvocation,
- functionName
- )
- )
- );
- ve = expectThrows(
- VerificationException.class,
- () -> plannerOptimizer.plan(
- "from test | where (" + functionInvocation + " and emp_no < 1000) or (length(first_name) > 12 and emp_no > 1000)",
- IS_SV_STATS
- )
- );
- assertThat(
- ve.getMessage(),
- containsString(
- LoggerMessageFormat.format(
- null,
- "1:19: Invalid condition [({} and emp_no < 1000) or (length(first_name) > 12 and emp_no > 1000)]. "
- + "Function {} can't be used as part of an or condition that includes [length(first_name) > 12 and emp_no > 1000]",
- functionInvocation,
- functionName
- )
- )
- );
- }
-
- public void testMatchFunctionDisjunctionNonPushableClauses() {
- assumeTrue("skipping because MATCH function is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
- String queryText = """
- from test
- | where match(first_name, "Peter") or length(last_name) > 10
- """;
-
- VerificationException ve = expectThrows(VerificationException.class, () -> plannerOptimizer.plan(queryText, IS_SV_STATS));
- assertThat(
- ve.getMessage(),
- containsString(
- "Invalid condition [match(first_name, \"Peter\") or length(last_name) > 10]. "
- + "Function MATCH can't be used as part of an or condition that includes [length(last_name) > 10]"
- )
- );
- }
-
  public void testMatchFunctionIsNotNullable() {
  assumeTrue("skipping because MATCH function is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
 
@@ -823,40 +687,6 @@ public void testMatchFunctionIsNotNullable() {
  );
  }
 
- /**
- * Expected:
- * LimitExec[1000[INTEGER]]
- * \_ExchangeExec[[_meta_field{f}#10, emp_no{f}#4, first_name{f}#5, gender{f}#6, job{f}#11, job.raw{f}#12, languages{f}#7, last_
- * name{f}#8, long_noidx{f}#13, salary{f}#9],false]
- * \_ProjectExec[[_meta_field{f}#10, emp_no{f}#4, first_name{f}#5, gender{f}#6, job{f}#11, job.raw{f}#12, languages{f}#7, last_
- * name{f}#8, long_noidx{f}#13, salary{f}#9]]
- * \_FieldExtractExec[_meta_field{f}#10, emp_no{f}#4, first_name{f}
- * \_EsQueryExec[test], indexMode[standard],
- * query[{"bool":{"should":[{"match":{"first_name":{"query":"Peter"}}},{"esql_single_value":{"field":"last_name",
- * "next":{"term":{"last_name":{"value":"Griffin"}}},"source":"last_name == \"Griffin\"@2:39"}}],"boost":1.0}}]
- */
- public void testMatchFunctionDisjunctionPushableClauses() {
- assumeTrue("skipping because MATCH function is not enabled", EsqlCapabilities.Cap.MATCH_FUNCTION.isEnabled());
- String queryText = """
- from test
- | where match(first_name, "Peter") or last_name == "Griffin"
- """;
- var plan = plannerOptimizer.plan(queryText, IS_SV_STATS);
-
- var limit = as(plan, LimitExec.class);
- var exchange = as(limit.child(), ExchangeExec.class);
- var project = as(exchange.child(), ProjectExec.class);
- var field = as(project.child(), FieldExtractExec.class);
- var query = as(field.child(), EsQueryExec.class);
- assertThat(query.limit().fold(), is(1000));
-
- var queryStringLeft = QueryBuilders.matchQuery("first_name", "Peter");
- Source termSource = new Source(2, 38, "last_name == \"Griffin\"");
- var queryStringRight = wrapWithSingleQuery(queryText, QueryBuilders.termQuery("last_name", "Griffin"), "last_name", termSource);
- var expected = QueryBuilders.boolQuery().should(queryStringLeft).should(queryStringRight);
- assertThat(query.query().toString(), is(expected.toString()));
- }
-
  /**
  * Expecting
  * LimitExec[1000[INTEGER]]