[ES|QL] accept string constants for date args (#182856)

## Summary ## String constants in place of dates As of elastic/elasticsearch#106932 string constants can always be used in place of dates. - `| eval date_extract("DAY_OF_WEEK", "2024-05-07T13:20:40.554Z")` - `| STATS AVG(salary) BY bucket = BUCKET(hire_date, 20, "1985-01-01T00:00:00Z", "1986-01-01T00:00:00Z") | SORT bucket` - `| eval date_diff("day", "2021-01-01", "2022-01-01")` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 9ddc3c6)
elastic · May 8, 2024 · bd1a767 · bd1a767
1 parent 3bf3407
commit bd1a767
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 90 deletions.
diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts
@@ -606,48 +606,6 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [
         returnType: 'number',
         examples: [],
       },
-      {
-        params: [
-          {
-            name: 'unit',
-            type: 'string',
-            literalOptions: dateDiffOptions,
-            literalSuggestions: dateDiffSuggestions,
-          },
-          { name: 'startTimestamp', type: 'string', constantOnly: true },
-          { name: 'endTimestamp', type: 'date' },
-        ],
-        returnType: 'number',
-        examples: [],
-      },
-      {
-        params: [
-          {
-            name: 'unit',
-            type: 'string',
-            literalOptions: dateDiffOptions,
-            literalSuggestions: dateDiffSuggestions,
-          },
-          { name: 'startTimestamp', type: 'date' },
-          { name: 'endTimestamp', type: 'string', constantOnly: true },
-        ],
-        returnType: 'number',
-        examples: [],
-      },
-      {
-        params: [
-          {
-            name: 'unit',
-            type: 'string',
-            literalOptions: dateDiffOptions,
-            literalSuggestions: dateDiffSuggestions,
-          },
-          { name: 'startTimestamp', type: 'string', constantOnly: true },
-          { name: 'endTimestamp', type: 'string', constantOnly: true },
-        ],
-        returnType: 'number',
-        examples: [],
-      },
     ],
   },
   {

diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts
@@ -214,14 +214,18 @@ export function getCommandOption(optionName: CommandOptionsDefinition['name']) {
   );
 }
 
-function compareLiteralType(argTypes: string, item: ESQLLiteral) {
+function compareLiteralType(argType: string, item: ESQLLiteral) {
   if (item.literalType !== 'string') {
-    return argTypes === item.literalType;
+    if (argType === item.literalType) {
+      return true;
+    }
+    return false;
   }
-  if (argTypes === 'chrono_literal') {
+  if (argType === 'chrono_literal') {
     return chronoLiterals.some(({ name }) => name === item.text);
   }
-  return argTypes === item.literalType;
+  // date-type parameters accept string literals because of ES auto-casting
+  return ['string', 'date'].includes(argType);
 }
 
 export function getColumnHit(

diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json
@@ -952,19 +952,6 @@
       "error": [],
       "warning": []
     },
-    {
-      "query": "row var = date_extract(\"ALIGNED_DAY_OF_WEEK_IN_MONTH\", to_datetime(\"a\"))",
-      "error": [],
-      "warning": []
-    },
-    {
-      "query": "row var = date_extract(\"a\", \"a\")",
-      "error": [
-        "Argument of [date_extract] must be [chrono_literal], found value [\"a\"] type [string]",
-        "Argument of [date_extract] must be [date], found value [\"a\"] type [string]"
-      ],
-      "warning": []
-    },
     {
       "query": "row var = date_format(now(), \"a\")",
       "error": [],
@@ -975,19 +962,6 @@
       "error": [],
       "warning": []
     },
-    {
-      "query": "row var = date_format(to_datetime(\"a\"), to_string(\"a\"))",
-      "error": [],
-      "warning": []
-    },
-    {
-      "query": "row var = date_format(\"a\", 5)",
-      "error": [
-        "Argument of [date_format] must be [date], found value [\"a\"] type [string]",
-        "Argument of [date_format] must be [string], found value [5] type [number]"
-      ],
-      "warning": []
-    },
     {
       "query": "row var = date_parse(\"a\", \"a\")",
       "error": [],
@@ -1021,19 +995,6 @@
       "error": [],
       "warning": []
     },
-    {
-      "query": "row var = date_trunc(1 year, to_datetime(\"a\"))",
-      "error": [],
-      "warning": []
-    },
-    {
-      "query": "row var = date_trunc(\"a\", \"a\")",
-      "error": [
-        "Argument of [date_trunc] must be [time_literal], found value [\"a\"] type [string]",
-        "Argument of [date_trunc] must be [date], found value [\"a\"] type [string]"
-      ],
-      "warning": []
-    },
     {
       "query": "row var = e()",
       "error": [],
@@ -9121,6 +9082,37 @@
       ],
       "warning": []
     },
+    {
+      "query": "from a_index | eval var = date_diff(\"year\", dateField, dateField)",
+      "error": [],
+      "warning": []
+    },
+    {
+      "query": "from a_index | eval date_diff(\"year\", dateField, dateField)",
+      "error": [],
+      "warning": []
+    },
+    {
+      "query": "from a_index | eval var = date_diff(\"year\", to_datetime(stringField), to_datetime(stringField))",
+      "error": [],
+      "warning": []
+    },
+    {
+      "query": "from a_index | eval date_diff(numberField, stringField, stringField)",
+      "error": [
+        "Argument of [date_diff] must be [string], found value [numberField] type [number]",
+        "Argument of [date_diff] must be [date], found value [stringField] type [string]",
+        "Argument of [date_diff] must be [date], found value [stringField] type [string]"
+      ],
+      "warning": []
+    },
+    {
+      "query": "from a_index | eval date_diff(\"year\", dateField, dateField, extraArg)",
+      "error": [
+        "Error: [date_diff] function expects exactly 3 arguments, got 4."
+      ],
+      "warning": []
+    },
     {
       "query": "from a_index | eval var = date_extract(\"ALIGNED_DAY_OF_WEEK_IN_MONTH\", dateField)",
       "error": [],

diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts
@@ -622,7 +622,17 @@ describe('validation logic', () => {
           // the right error message
           if (
             params.every(({ type }) => type !== 'any') &&
-            !['to_version', 'mv_sort', 'date_diff'].includes(name)
+            ![
+              'to_version',
+              'mv_sort',
+              // skip the date functions because the row tests always throw in
+              // a string literal and expect it to be invalid for the date functions
+              // but it's always valid because ES will parse it as a date
+              'date_diff',
+              'date_extract',
+              'date_format',
+              'date_trunc',
+            ].includes(name)
           ) {
             // now test nested functions
             const fieldMappingWithNestedFunctions = getFieldMapping(params, {
@@ -1289,7 +1299,6 @@ describe('validation logic', () => {
         );
       });
       for (const { name, signatures, ...rest } of numericOrStringFunctions) {
-        if (name === 'date_diff') continue; // date_diff is hard to test
         const supportedSignatures = signatures.filter(({ returnType }) =>
           // TODO — not sure why the tests have this limitation... seems like any type
           // that can be part of a boolean expression should be allowed in a where clause
@@ -1489,7 +1498,6 @@ describe('validation logic', () => {
       }
 
       for (const { name, alias, signatures, ...defRest } of evalFunctionsDefinitions) {
-        if (name === 'date_diff') continue; // date_diff is hard to test
         for (const { params, ...signRest } of signatures) {
           const fieldMapping = getFieldMapping(params);
           testErrorsAndWarnings(
@@ -1561,7 +1569,7 @@ describe('validation logic', () => {
           // the right error message
           if (
             params.every(({ type }) => type !== 'any') &&
-            !['to_version', 'mv_sort', 'date_diff'].includes(name)
+            !['to_version', 'mv_sort'].includes(name)
           ) {
             // now test nested functions
             const fieldMappingWithNestedFunctions = getFieldMapping(params, {
@@ -2251,7 +2259,7 @@ describe('validation logic', () => {
           // the right error message
           if (
             params.every(({ type }) => type !== 'any') &&
-            !['to_version', 'mv_sort', 'date_diff'].includes(name)
+            !['to_version', 'mv_sort'].includes(name)
           ) {
             // now test nested functions
             const fieldMappingWithNestedAggsFunctions = getFieldMapping(params, {