From bd1a767d5be13bd00bed4291c00e643b8c18f3c1 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 8 May 2024 07:53:47 -0600 Subject: [PATCH] [ES|QL] accept string constants for date args (#182856) ## Summary ## String constants in place of dates As of https://github.com/elastic/elasticsearch/pull/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 <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 9ddc3c6aa836e9e5d3bdd30512f7de55d62a16d2) --- .../src/definitions/functions.ts | 42 ----------- .../src/shared/helpers.ts | 12 ++-- .../esql_validation_meta_tests.json | 70 ++++++++----------- .../src/validation/validation.test.ts | 18 +++-- 4 files changed, 52 insertions(+), 90 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index bfa718353f6b2..205b2d158fd05 100644 --- 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 index 868e8f3f9212a..fb1d3d648142b 100644 --- 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 index c9b89c0163111..660fff0bb7d8f 100644 --- 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 index 578d3e9d79b6a..1d7d080b6c5c0 100644 --- 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, {