Skip to content
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

Jump to bottom

[ES|QL] remove chrono literals #186536

Merged
merged 3 commits into from
Jul 10, 2024
Merged

[ES|QL] remove chrono literals #186536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
113b69e
remove chrono literals
drewdaemon Jun 20, 2024
7deb85c
Merge branch 'main' of github.com:elastic/kibana into remove-chrono-l…
drewdaemon Jul 9, 2024
7b5c359
update tests
drewdaemon Jul 9, 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
36 changes: 34 additions & 2 deletions packages/kbn-esql-validation-autocomplete/scripts/generate_function_definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,39 @@ const dateDiffOptions = [
'ns',
];

const dateExtractOptions = [
'ALIGNED_DAY_OF_WEEK_IN_MONTH',
'ALIGNED_DAY_OF_WEEK_IN_YEAR',
'ALIGNED_WEEK_OF_MONTH',
'ALIGNED_WEEK_OF_YEAR',
'AMPM_OF_DAY',
'CLOCK_HOUR_OF_AMPM',
'CLOCK_HOUR_OF_DAY',
'DAY_OF_MONTH',
'DAY_OF_WEEK',
'DAY_OF_YEAR',
'EPOCH_DAY',
'ERA',
'HOUR_OF_AMPM',
'HOUR_OF_DAY',
'INSTANT_SECONDS',
'MICRO_OF_DAY',
'MICRO_OF_SECOND',
'MILLI_OF_DAY',
'MILLI_OF_SECOND',
'MINUTE_OF_DAY',
'MINUTE_OF_HOUR',
'MONTH_OF_YEAR',
'NANO_OF_DAY',
'NANO_OF_SECOND',
'OFFSET_SECONDS',
'PROLEPTIC_MONTH',
'SECOND_OF_DAY',
'SECOND_OF_MINUTE',
'YEAR',
'YEAR_OF_ERA',
];

/**
* Enrichments for function definitions
*
Expand All @@ -168,8 +201,7 @@ const functionEnrichments: Record<string, RecursivePartial<FunctionDefinition>>
date_extract: {
signatures: [
{
// override the first param as type chrono_literal
params: [{ type: 'chrono_literal' }],
params: [{ literalOptions: dateExtractOptions }],
},
],
},
Expand Down
10 changes: 3 additions & 7 deletions packages/kbn-esql-validation-autocomplete/scripts/generate_function_validation_tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { statsAggregationFunctionDefinitions } from '../src/definitions/aggs';
import { evalFunctionDefinitions } from '../src/definitions/functions';
import { groupingFunctionDefinitions } from '../src/definitions/grouping';
import { getFunctionSignatures } from '../src/definitions/helpers';
import { timeUnits, chronoLiterals } from '../src/definitions/literals';
import { timeUnits } from '../src/definitions/literals';
import { nonNullable } from '../src/shared/helpers';
import {
SupportedFieldType,
Expand Down Expand Up @@ -1045,14 +1045,10 @@ function getFieldName(
}

const literals = {
chrono_literal: chronoLiterals[0].name,
time_literal: timeUnits[0],
};

function getLiteralType(typeString: 'chrono_literal' | 'time_literal') {
if (typeString === 'chrono_literal') {
return literals[typeString];
}
function getLiteralType(typeString: 'time_literal') {
return `1 ${literals[typeString]}`;
}

Expand Down Expand Up @@ -1116,7 +1112,7 @@ function getFieldMapping(
}
if (/literal$/.test(typeString) && useLiterals) {
return {
name: getLiteralType(typeString as 'chrono_literal' | 'time_literal'),
name: getLiteralType(typeString as 'time_literal'),
type,
...rest,
};
Expand Down
5 changes: 1 addition & 4 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { suggest } from './autocomplete';
import { evalFunctionDefinitions } from '../definitions/functions';
import { builtinFunctions } from '../definitions/builtin';
import { statsAggregationFunctionDefinitions } from '../definitions/aggs';
import { chronoLiterals, timeUnitsToSuggest } from '../definitions/literals';
import { timeUnitsToSuggest } from '../definitions/literals';
import { commandDefinitions } from '../definitions/commands';
import { getUnitDuration, TRIGGER_SUGGESTION_COMMAND } from './factories';
import { camelCase, partition } from 'lodash';
Expand Down Expand Up @@ -207,9 +207,6 @@ function getLiteralsByType(_type: string | string[]) {
// return only singular
return timeUnitsToSuggest.map(({ name }) => `1 ${name}`).filter((s) => !/s$/.test(s));
}
if (type.includes('chrono_literal')) {
return chronoLiterals.map(({ name }) => name);
}
return [];
}

Expand Down
5 changes: 1 addition & 4 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { groupingFunctionDefinitions } from '../definitions/grouping';
import { statsAggregationFunctionDefinitions } from '../definitions/aggs';
import { evalFunctionDefinitions } from '../definitions/functions';
import { getFunctionSignatures, getCommandSignature } from '../definitions/helpers';
import { chronoLiterals, timeUnitsToSuggest } from '../definitions/literals';
import { timeUnitsToSuggest } from '../definitions/literals';
import {
FunctionDefinition,
CommandDefinition,
Expand Down Expand Up @@ -336,9 +336,6 @@ export function getCompatibleLiterals(commandName: string, types: string[], name
if (types.includes('time_literal_unit')) {
suggestions.push(...buildConstantsDefinitions(timeUnitsToSuggest.map(({ name }) => name))); // i.e. year, month, ...
}
if (types.includes('chrono_literal')) {
suggestions.push(...buildConstantsDefinitions(chronoLiterals.map(({ name }) => name))); // i.e. EPOC_DAY, ...
}
if (types.includes('string')) {
if (names) {
const index = types.indexOf('string');
Expand Down
34 changes: 33 additions & 1 deletion 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 @@ -668,8 +668,40 @@ const dateExtractDefinition: FunctionDefinition = {
params: [
{
name: 'datePart',
type: 'chrono_literal',
type: 'string',
optional: false,
literalOptions: [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we repeating them here and not adding them to a common place that can be reused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean define an array and reuse it between the generator script and the file it generates (this file)?

Copy link
Contributor

@stratoula stratoula Jul 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja!! (to change only in one place)

'ALIGNED_DAY_OF_WEEK_IN_MONTH',
'ALIGNED_DAY_OF_WEEK_IN_YEAR',
'ALIGNED_WEEK_OF_MONTH',
'ALIGNED_WEEK_OF_YEAR',
'AMPM_OF_DAY',
'CLOCK_HOUR_OF_AMPM',
'CLOCK_HOUR_OF_DAY',
'DAY_OF_MONTH',
'DAY_OF_WEEK',
'DAY_OF_YEAR',
'EPOCH_DAY',
'ERA',
'HOUR_OF_AMPM',
'HOUR_OF_DAY',
'INSTANT_SECONDS',
'MICRO_OF_DAY',
'MICRO_OF_SECOND',
'MILLI_OF_DAY',
'MILLI_OF_SECOND',
'MINUTE_OF_DAY',
'MINUTE_OF_HOUR',
'MONTH_OF_YEAR',
'NANO_OF_DAY',
'NANO_OF_SECOND',
'OFFSET_SECONDS',
'PROLEPTIC_MONTH',
'SECOND_OF_DAY',
'SECOND_OF_MINUTE',
'YEAR',
'YEAR_OF_ERA',
],
},
{
name: 'date',
Expand Down
5 changes: 1 addition & 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 @@ -24,7 +24,7 @@ import { commandDefinitions } from '../definitions/commands';
import { evalFunctionDefinitions } from '../definitions/functions';
import { groupingFunctionDefinitions } from '../definitions/grouping';
import { getFunctionSignatures } from '../definitions/helpers';
import { timeUnits, chronoLiterals } from '../definitions/literals';
import { timeUnits } from '../definitions/literals';
import {
byOption,
metadataOption,
Expand Down Expand Up @@ -233,9 +233,6 @@ function compareLiteralType(argType: string, item: ESQLLiteral) {
return false;
}

if (argType === 'chrono_literal') {
return chronoLiterals.some(({ name }) => name === item.text);
}
// date-type parameters accept string literals because of ES auto-casting
return ['string', 'date'].includes(argType);
}
Expand Down
12 changes: 9 additions & 3 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 @@ -11006,6 +11006,13 @@
"error": [],
"warning": []
},
{
"query": "from a_index | eval date_extract(\"SOME_RANDOM_STRING\", now())",
"error": [],
"warning": [
"Invalid option [\"SOME_RANDOM_STRING\"] for date_extract. Supported options: [\"ALIGNED_DAY_OF_WEEK_IN_MONTH\", \"ALIGNED_DAY_OF_WEEK_IN_YEAR\", \"ALIGNED_WEEK_OF_MONTH\", \"ALIGNED_WEEK_OF_YEAR\", \"AMPM_OF_DAY\", \"CLOCK_HOUR_OF_AMPM\", \"CLOCK_HOUR_OF_DAY\", \"DAY_OF_MONTH\", \"DAY_OF_WEEK\", \"DAY_OF_YEAR\", \"EPOCH_DAY\", \"ERA\", \"HOUR_OF_AMPM\", \"HOUR_OF_DAY\", \"INSTANT_SECONDS\", \"MICRO_OF_DAY\", \"MICRO_OF_SECOND\", \"MILLI_OF_DAY\", \"MILLI_OF_SECOND\", \"MINUTE_OF_DAY\", \"MINUTE_OF_HOUR\", \"MONTH_OF_YEAR\", \"NANO_OF_DAY\", \"NANO_OF_SECOND\", \"OFFSET_SECONDS\", \"PROLEPTIC_MONTH\", \"SECOND_OF_DAY\", \"SECOND_OF_MINUTE\", \"YEAR\", \"YEAR_OF_ERA\"]."
]
},
{
"query": "from a_index | eval var = date_extract(\"ALIGNED_DAY_OF_WEEK_IN_MONTH\", dateField)",
"error": [],
Expand All @@ -11024,7 +11031,6 @@
{
"query": "from a_index | eval date_extract(stringField, stringField)",
"error": [
"Argument of [date_extract] must be [chrono_literal], found value [stringField] type [string]",
"Argument of [date_extract] must be [date], found value [stringField] type [string]"
],
"warning": []
Expand All @@ -11044,7 +11050,7 @@
{
"query": "row var = date_extract(true, true)",
"error": [
"Argument of [date_extract] must be [chrono_literal], found value [true] type [boolean]",
"Argument of [date_extract] must be [string], found value [true] type [boolean]",
"Argument of [date_extract] must be [date], found value [true] type [boolean]"
],
"warning": []
Expand All @@ -11057,7 +11063,7 @@
{
"query": "from a_index | eval date_extract(booleanField, booleanField)",
"error": [
"Argument of [date_extract] must be [chrono_literal], found value [booleanField] type [boolean]",
"Argument of [date_extract] must be [string], found value [booleanField] type [boolean]",
"Argument of [date_extract] must be [date], found value [booleanField] type [boolean]"
],
"warning": []
Expand Down
22 changes: 12 additions & 10 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 @@ -12,7 +12,7 @@ import { ignoreErrorsMap, validateQuery } from './validation';
import { evalFunctionDefinitions } from '../definitions/functions';
import { getFunctionSignatures } from '../definitions/helpers';
import { FunctionDefinition, SupportedFieldType, supportedFieldTypes } from '../definitions/types';
import { chronoLiterals, timeUnits, timeUnitsToSuggest } from '../definitions/literals';
import { timeUnits, timeUnitsToSuggest } from '../definitions/literals';
import { statsAggregationFunctionDefinitions } from '../definitions/aggs';
import capitalize from 'lodash/capitalize';
import { camelCase } from 'lodash';
Expand Down Expand Up @@ -58,13 +58,9 @@ const nestedFunctions = {
};

const literals = {
chrono_literal: chronoLiterals[0].name,
time_literal: timeUnitsToSuggest[0].name,
};
function getLiteralType(typeString: 'chrono_literal' | 'time_literal') {
if (typeString === 'chrono_literal') {
return literals[typeString];
}
function getLiteralType(typeString: 'time_literal') {
return `1 ${literals[typeString]}`;
}

Expand Down Expand Up @@ -144,7 +140,7 @@ function getFieldMapping(
}
if (/literal$/.test(typeString) && useLiterals) {
return {
name: getLiteralType(typeString as 'chrono_literal' | 'time_literal'),
name: getLiteralType(typeString as 'time_literal'),
type,
...rest,
};
Expand Down Expand Up @@ -2610,6 +2606,13 @@ describe('validation logic', () => {
describe('date_extract', () => {
testErrorsAndWarnings('row var = date_extract("ALIGNED_DAY_OF_WEEK_IN_MONTH", now())', []);
testErrorsAndWarnings('row date_extract("ALIGNED_DAY_OF_WEEK_IN_MONTH", now())', []);
testErrorsAndWarnings(
'from a_index | eval date_extract("SOME_RANDOM_STRING", now())',
[],
[
'Invalid option ["SOME_RANDOM_STRING"] for date_extract. Supported options: ["ALIGNED_DAY_OF_WEEK_IN_MONTH", "ALIGNED_DAY_OF_WEEK_IN_YEAR", "ALIGNED_WEEK_OF_MONTH", "ALIGNED_WEEK_OF_YEAR", "AMPM_OF_DAY", "CLOCK_HOUR_OF_AMPM", "CLOCK_HOUR_OF_DAY", "DAY_OF_MONTH", "DAY_OF_WEEK", "DAY_OF_YEAR", "EPOCH_DAY", "ERA", "HOUR_OF_AMPM", "HOUR_OF_DAY", "INSTANT_SECONDS", "MICRO_OF_DAY", "MICRO_OF_SECOND", "MILLI_OF_DAY", "MILLI_OF_SECOND", "MINUTE_OF_DAY", "MINUTE_OF_HOUR", "MONTH_OF_YEAR", "NANO_OF_DAY", "NANO_OF_SECOND", "OFFSET_SECONDS", "PROLEPTIC_MONTH", "SECOND_OF_DAY", "SECOND_OF_MINUTE", "YEAR", "YEAR_OF_ERA"].',
]
);

testErrorsAndWarnings(
'from a_index | eval var = date_extract("ALIGNED_DAY_OF_WEEK_IN_MONTH", dateField)',
Expand All @@ -2627,7 +2630,6 @@ describe('validation logic', () => {
);

testErrorsAndWarnings('from a_index | eval date_extract(stringField, stringField)', [
'Argument of [date_extract] must be [chrono_literal], found value [stringField] type [string]',
'Argument of [date_extract] must be [date], found value [stringField] type [string]',
]);

Expand All @@ -2642,7 +2644,7 @@ describe('validation logic', () => {
);

testErrorsAndWarnings('row var = date_extract(true, true)', [
'Argument of [date_extract] must be [chrono_literal], found value [true] type [boolean]',
'Argument of [date_extract] must be [string], found value [true] type [boolean]',
'Argument of [date_extract] must be [date], found value [true] type [boolean]',
]);

Expand All @@ -2652,7 +2654,7 @@ describe('validation logic', () => {
);

testErrorsAndWarnings('from a_index | eval date_extract(booleanField, booleanField)', [
'Argument of [date_extract] must be [chrono_literal], found value [booleanField] type [boolean]',
'Argument of [date_extract] must be [string], found value [booleanField] type [boolean]',
'Argument of [date_extract] must be [date], found value [booleanField] type [boolean]',
]);
testErrorsAndWarnings('from a_index | eval date_extract(null, null)', []);
Expand Down