Fixing tests

carlosdelest · Dec 12, 2024 · c198d91 · c198d91
1 parent 68e88ef
commit c198d91
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 42 deletions.
diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle
@@ -94,5 +94,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task ->
   task.skipTest("privileges/11_builtin/Test get builtin privileges" ,"unnecessary to test compatibility")
   task.skipTest("esql/61_enrich_ip/Invalid IP strings", "We switched from exceptions to null+warnings for ENRICH runtime errors")
   task.skipTest("esql/180_match_operator/match with non text field", "Match operator can now be used on non-text fields")
+  task.skipTest("esql/180_match_operator/match with functions", "Error message changed")
 })
 
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-function.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-function.csv-spec
@@ -146,6 +146,7 @@ book_no:keyword | author:text       | year:integer
 
 matchWithDisjunctionAndConjunction
 required_capability: match_function
+required_capability: full_text_functions_disjunctions
 
 from books
 | where (match(author, "Vonnegut") or match(author, "Marquez")) and match(description, "realism")
@@ -157,6 +158,7 @@ book_no:keyword
 
 matchWithDisjunctionIncludingConjunction
 required_capability: match_function
+required_capability: full_text_functions_disjunctions
 
 from books
 | where match(author, "Vonnegut") or (match(author, "Marquez") and match(description, "realism"))

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-operator.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-operator.csv-spec
@@ -104,7 +104,7 @@ book_no:keyword | title:text
 
 
 matchWithDisjunction
-required_capability: match_function
+required_capability: match_operator
 
 from books
 | where author : "Vonnegut" or author : "Guinane" 
@@ -120,7 +120,7 @@ book_no:keyword | author:text
 ;
 
 matchWithDisjunctionAndFiltersConjunction
-required_capability: match_function
+required_capability: match_operator
 
 from books
 | where (author : "Vonnegut" or author : "Guinane") and year > 1997
@@ -133,7 +133,8 @@ book_no:keyword | author:text       | year:integer
 ;
 
 matchWithDisjunctionAndConjunction
-required_capability: match_function
+required_capability: match_operator
+required_capability: full_text_functions_disjunctions
 
 from books
 | where (author : "Vonnegut" or author : "Marquez") and description : "realism"
@@ -144,7 +145,8 @@ book_no:keyword
 ;
 
 matchWithDisjunctionIncludingConjunction
-required_capability: match_function
+required_capability: match_operator
+required_capability: full_text_functions_disjunctions
 
 from books
 | where author : "Vonnegut" or (author : "Marquez" and description : "realism")
@@ -159,7 +161,6 @@ book_no:keyword
 3950
 ;
 
-
 matchWithFunctionPushedToLucene
 required_capability: match_operator_colon
 
@@ -277,7 +278,7 @@ count(*): long  | author.keyword:keyword
 ;
 
 testMatchBooleanField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -293,7 +294,7 @@ Amabile             | true                  | 2.09
 ;
 
 testMatchIntegerField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -305,7 +306,7 @@ emp_no:integer | first_name:keyword
 ;
 
 testMatchDoubleField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -317,7 +318,7 @@ emp_no:integer | salary_change:double
 ;  
 
 testMatchLongField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from date_nanos
@@ -329,7 +330,7 @@ num:long
 ;
 
 testMatchUnsignedLongField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from ul_logs
@@ -341,7 +342,7 @@ bytes_out:unsigned_long
 ;
 
 testMatchIpFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from sample_data
@@ -353,7 +354,7 @@ client_ip:ip   | message:keyword
 ;
 
 testMatchDateFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from date_nanos
@@ -365,7 +366,7 @@ millis:date
 ;
 
 testMatchDateNanosFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from date_nanos
@@ -377,7 +378,7 @@ nanos:date_nanos
 ;
 
 testMatchBooleanFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -393,7 +394,7 @@ Amabile             | true                  | 2.09
 ;
 
 testMatchIntegerFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -405,7 +406,7 @@ emp_no:integer | first_name:keyword
 ;
 
 testMatchDoubleFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -417,7 +418,7 @@ emp_no:integer | salary_change:double
 ;
 
 testMatchLongFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from date_nanos
@@ -429,7 +430,7 @@ num:long
 ;
 
 testMatchUnsignedLongFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from ul_logs
@@ -441,7 +442,7 @@ bytes_out:unsigned_long
 ;
 
 testMatchVersionFieldAsString
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from apps 
@@ -453,7 +454,7 @@ bbbbb        | 2.1
 ;
 
 testMatchIntegerAsDouble
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees 
@@ -466,7 +467,7 @@ emp_no:integer | first_name:keyword
 ;
 
 testMatchDoubleAsIntegerField
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees
@@ -481,7 +482,7 @@ emp_no:integer | height:double
 ;
 
 testMatchMultipleFieldTypes
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible 
@@ -498,7 +499,7 @@ emp_as_int:integer | name_as_kw:keyword
 
 
 testMatchMultipleFieldTypesKeywordText
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible 
@@ -513,7 +514,7 @@ Kazuhito
 ;
 
 testMatchMultipleFieldTypesDoubleFloat
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible
@@ -532,7 +533,7 @@ emp_no:integer | height_dbl:double
 ;
 
 testMatchMultipleFieldTypesBooleanKeyword
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible
@@ -549,7 +550,7 @@ true
 ;
 
 testMatchMultipleFieldTypesLongUnsignedLong
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible  
@@ -564,7 +565,7 @@ avg_worked_seconds_ul:unsigned_long
 ;
 
 testMatchMultipleFieldTypesDateNanosDate
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible
@@ -579,7 +580,7 @@ hire_date_nanos:date_nanos
 ;
 
 testMatchWithWrongFieldValue
-required_capability: match_function
+required_capability: match_operator
 required_capability: match_additional_types
 
 from employees,employees_incompatible

diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
@@ -565,7 +565,12 @@ public enum Cap {
         /**
          * Additional types for match function and operator
          */
-        MATCH_ADDITIONAL_TYPES;
+        MATCH_ADDITIONAL_TYPES,
+
+        /**
+         * Full text functions can be used in disjunctions
+         */
+        FULL_TEXT_FUNCTIONS_DISJUNCTIONS;
 
         private final boolean enabled;
 

diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java
@@ -784,10 +784,10 @@ private static void checkFullTextSearchDisjunctions(
             boolean right = checkFullTextSearchInDisjunctions(or.right());
             if (left ^ right) {
                 Holder<String> elementName = new Holder<>();
-                if (right) {
-                    or.left().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
-                } else {
+                if (left) {
                     or.right().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
+                } else {
+                    or.left().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
                 }
                 failures.add(
                     fail(

diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@@ -1168,12 +1168,14 @@ public void testMatchInsideEval() throws Exception {
     public void testMatchFilter() throws Exception {
         assertEquals(
             "1:19: Invalid condition [first_name:\"Anna\" or starts_with(first_name, \"Anne\")]. "
-                + "[:] operator can't be used as part of an or condition",
+                + "[:] operator can be used as part of an OR condition, " +
+                "but only if other full text functions are used as part of the condition",
             error("from test | where first_name:\"Anna\" or starts_with(first_name, \"Anne\")")
         );
 
         assertEquals(
-            "1:51: Invalid condition [first_name:\"Anna\" OR new_salary > 100]. " + "[:] operator can't be used as part of an or condition",
+            "1:51: Invalid condition [first_name:\"Anna\" OR new_salary > 100]. [:] operator can be used as part of an OR " +
+                "condition, but only if other full text functions are used as part of the condition",
             error("from test | eval new_salary = salary + 10 | where first_name:\"Anna\" OR new_salary > 100")
         );
     }
@@ -1417,7 +1419,7 @@ private void checkWithDisjunctions(String functionName, String functionInvocatio
                 "1:19: Invalid condition [{} or length(first_name) > 12]. "
                     + "[{}] "
                     + functionType
-                    + " can't be used as part of an or condition",
+                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
                 functionInvocation,
                 functionName
             ),
@@ -1426,30 +1428,30 @@ private void checkWithDisjunctions(String functionName, String functionInvocatio
         assertEquals(
             LoggerMessageFormat.format(
                 null,
-                "1:19: Invalid condition [({} and first_name is not null) or (length(first_name) > 12 and first_name is null)]. "
+                "1:20: Invalid condition [{} or first_name is not null]. "
                     + "[{}] "
                     + functionType
-                    + " can't be used as part of an or condition",
+                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
                 functionInvocation,
                 functionName
             ),
             error(
                 "from test | where ("
                     + functionInvocation
-                    + " and first_name is not null) or (length(first_name) > 12 and first_name is null)"
+                    + " or first_name is not null) or (length(first_name) > 12 and match(last_name, \"Smith\"))"
             )
         );
         assertEquals(
             LoggerMessageFormat.format(
                 null,
-                "1:19: Invalid condition [({} and first_name is not null) or first_name is null]. "
+                "1:19: Invalid condition [{} or (last_name is not null and first_name is null)]. "
                     + "[{}] "
                     + functionType
-                    + " can't be used as part of an or condition",
+                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
                 functionInvocation,
                 functionName
             ),
-            error("from test | where (" + functionInvocation + " and first_name is not null) or first_name is null")
+            error("from test | where " + functionInvocation + " or (last_name is not null and first_name is null)")
         );
     }
 

diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/180_match_operator.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/180_match_operator.yml
@@ -181,7 +181,7 @@ setup:
 
   - match: { status: 400 }
   - match: { error.type: verification_exception }
-  - match: { error.reason: "Found 1 problem\nline 1:19: Invalid condition [content:\"fox\" OR to_upper(content) == \"FOX\"]. [:] operator can't be used as part of an or condition" }
+  - match: { error.reason: "Found 1 problem\nline 1:19: Invalid condition [content:\"fox\" OR to_upper(content) == \"FOX\"]. [:] operator can be used as part of an OR condition, but only if other full text functions are used as part of the condition" }
 
 ---
 "match within eval":