Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
drewdaemon merged 2 commits into from
May 8, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14a4c42
accept string literals for date args
drewdaemon May 7, 2024
50b63f4
Merge branch 'main' into dates-accept-strings
kibanamachine May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 0 additions & 42 deletions packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
},
],
},
{
Expand Down
12 changes: 8 additions & 4 deletions packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
70 changes: 31 additions & 39 deletions packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": [],
Expand All @@ -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": [],
Expand Down Expand Up @@ -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": [],
Expand Down Expand Up @@ -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": [],
Expand Down
18 changes: 13 additions & 5 deletions packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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, {
Expand Down