From c198d91483acf15d78eba80875fb9cc0829dfdf0 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Thu, 12 Dec 2024 15:44:03 +0100 Subject: [PATCH] Fixing tests --- x-pack/plugin/build.gradle | 1 + .../main/resources/match-function.csv-spec | 2 + .../main/resources/match-operator.csv-spec | 57 ++++++++++--------- .../xpack/esql/action/EsqlCapabilities.java | 7 ++- .../xpack/esql/analysis/Verifier.java | 6 +- .../xpack/esql/analysis/VerifierTests.java | 20 ++++--- .../test/esql/180_match_operator.yml | 2 +- 7 files changed, 53 insertions(+), 42 deletions(-) diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index fb37fb3575551..aa6e8de4ec27c 100644 --- 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 index 31b5221d493c3..97da74f2fbfac 100644 --- 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 index 8cb49dd41be9e..89bb0c988a43b 100644 --- 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 index 1aee1df3dbafb..a0b3b52c7b496 100644 --- 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 index 1afea2f1f6664..a62fd312376ee 100644 --- 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 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 index 2ffe68e297b20..c82dcdda60900 100644 --- 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 index 663c0dc78acb3..23921614c9a63 100644 --- 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":