Skip to content

Commit

Permalink
[ES|QL] Stronger typing for ESQL field interface (elastic#189941)
Browse files Browse the repository at this point in the history 
## Summary

The heart of this PR is [this
change](https://github.com/elastic/kibana/pull/189941/files#diff-88513481c44d7b2de70ca2f7826c2b1fb1d8bda5b308aab0f8917a42ac2c24b7R11-R94)
where I clean up and clarify the various data-type-related types
floating around in the engines + [this
change](https://github.com/elastic/kibana/pull/189941/files#diff-f48b526b82119bd591cf781262173d7a0233d236ab26496a4c06f5ea9a441561R21)
where I add strong typing to the ES|QL field interface.

Pretty much everything else is a result of that. For example, strongly
typing the fields and test helpers highlighted a bunch of tests that
were still using Kibana types instead of Elasticsearch types. So, then
those had to be updated.

There's more work to do to extend the strong field typing to the rest of
the engines, but this got big and I decided to do it piece-meal.

Next plans
- Extend typing to subroutines in autocomplete and validation engines
(e.g. the stuff in `factories.ts`)
- Add typing to the [variable
interface](https://github.com/elastic/kibana/pull/189941/files#diff-f48b526b82119bd591cf781262173d7a0233d236ab26496a4c06f5ea9a441561R13)
- Consider merging `time_literal` and `time_duration` types
- "It looks like timespan literals are the way to write a constant
time_duration. and time_durations can only be constants at the moment.
so they aren't the same, but sure are about the same" - Nik
- Consider merging
`packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts`
with
`packages/kbn-esql-validation-autocomplete/src/definitions/types.ts` in
some common place


### 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
  • Loading branch information
@drewdaemon
drewdaemon authored Aug 8, 2024
1 parent c860846 commit edf5c76
Show file tree
Hide file tree
Showing 23 changed files with 811 additions and 1,050 deletions.
11 changes: 6 additions & 5 deletions examples/esql_validation_example/public/app.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {

import type { CoreStart } from '@kbn/core/public';

import { ESQLCallbacks, validateQuery } from '@kbn/esql-validation-autocomplete';
import { ESQLCallbacks, ESQLRealField, validateQuery } from '@kbn/esql-validation-autocomplete';
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import type { StartDependencies } from './plugin';
import { CodeSnippet } from './code_snippet';
Expand Down Expand Up @@ -52,10 +52,11 @@ export const App = (props: { core: CoreStart; plugins: StartDependencies }) => {
['index1', 'anotherIndex', 'dataStream'].map((name) => ({ name, hidden: false }))
: undefined,
getFieldsFor: callbacksEnabled.fields
? async () => [
{ name: 'numberField', type: 'number' },
{ name: 'stringField', type: 'string' },
]
? async () =>
[
{ name: 'doubleField', type: 'double' },
{ name: 'keywordField', type: 'keyword' },
] as ESQLRealField[]
: undefined,
getPolicies: callbacksEnabled.policies
? async () => [
Expand Down
45 changes: 26 additions & 19 deletions packages/kbn-esql-validation-autocomplete/scripts/generate_function_validation_tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ import { getFunctionSignatures } from '../src/definitions/helpers';
import { timeUnits } from '../src/definitions/literals';
import { nonNullable } from '../src/shared/helpers';
import {
SupportedFieldType,
SupportedDataType,
FunctionDefinition,
supportedFieldTypes,
isSupportedFieldType,
dataTypes,
isSupportedDataType,
fieldTypes,
} from '../src/definitions/types';
import { FUNCTION_DESCRIBE_BLOCK_NAME } from '../src/validation/function_describe_block_name';
import { getMaxMinNumberOfParams } from '../src/validation/helpers';
import { ESQL_NUMBER_TYPES, isNumericType, isStringType } from '../src/shared/esql_types';

export const fieldNameFromType = (type: SupportedFieldType) => `${camelCase(type)}Field`;
export const fieldNameFromType = (type: SupportedDataType) => `${camelCase(type)}Field`;

function main() {
const testCasesByFunction: Map<string, Map<string, string[]>> = new Map();
Expand Down Expand Up @@ -301,8 +302,8 @@ function generateWhereCommandTestsForEvalFunction(
// TODO: not sure why there's this constraint...
const supportedFunction = signatures.some(
({ returnType, params }) =>
[...ESQL_NUMBER_TYPES, 'string'].includes(returnType) &&
params.every(({ type }) => [...ESQL_NUMBER_TYPES, 'string'].includes(type))
[...ESQL_NUMBER_TYPES, 'string'].includes(returnType as string) &&
params.every(({ type }) => [...ESQL_NUMBER_TYPES, 'string'].includes(type as string))
);

if (!supportedFunction) {
Expand All @@ -312,7 +313,7 @@ function generateWhereCommandTestsForEvalFunction(
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
[...ESQL_NUMBER_TYPES, 'string'].includes(returnType)
[...ESQL_NUMBER_TYPES, 'string'].includes(returnType as string)
);
for (const { params, returnType, ...restSign } of supportedSignatures) {
const correctMapping = getFieldMapping(params);
Expand Down Expand Up @@ -905,7 +906,7 @@ function generateStatsCommandTestsForGroupingFunction(
fieldReplacedType
// if a param of type time_literal or chrono_literal it will always be a literal
// so no way to test the constantOnly thing
.filter((type) => !['time_literal', 'chrono_literal'].includes(type))
.filter((type) => !['time_literal'].includes(type as string))
.map((type) => `Argument of [${name}] must be a constant, received [${type}Field]`)
);
}
Expand Down Expand Up @@ -965,7 +966,7 @@ function generateSortCommandTestsForAggFunction(

const generateSortCommandTestsForGroupingFunction = generateSortCommandTestsForAggFunction;

const fieldTypesToConstants: Record<SupportedFieldType, string> = {
const fieldTypesToConstants: Record<SupportedDataType, string> = {
text: '"a"',
keyword: '"a"',
double: '5.5',
Expand All @@ -984,14 +985,20 @@ const fieldTypesToConstants: Record<SupportedFieldType, string> = {
geo_shape: 'to_geoshape("POINT (30 10)")',
cartesian_point: 'to_cartesianpoint("POINT (30 10)")',
cartesian_shape: 'to_cartesianshape("POINT (30 10)")',
null: 'NULL',
time_duration: '1 day',
// the following are never supplied
// by the ES function definitions. Just making types happy
time_literal: '1 day',
unsupported: '',
};

const supportedTypesAndFieldNames = supportedFieldTypes.map((type) => ({
const supportedTypesAndFieldNames = fieldTypes.map((type) => ({
name: fieldNameFromType(type),
type,
}));

const supportedTypesAndConstants = supportedFieldTypes.map((type) => ({
const supportedTypesAndConstants = dataTypes.map((type) => ({
name: fieldTypesToConstants[type],
type,
}));
Expand Down Expand Up @@ -1029,7 +1036,7 @@ const toCartesianShapeSignature = evalFunctionDefinitions.find(
const toVersionSignature = evalFunctionDefinitions.find(({ name }) => name === 'to_version')!;

// We don't have full list for long, unsigned_long, etc.
const nestedFunctions: Record<SupportedFieldType, string> = {
const nestedFunctions: Record<SupportedDataType, string> = {
double: prepareNestedFunction(toDoubleSignature),
integer: prepareNestedFunction(toInteger),
text: prepareNestedFunction(toStringSignature),
Expand All @@ -1046,7 +1053,7 @@ const nestedFunctions: Record<SupportedFieldType, string> = {
};

function getFieldName(
typeString: SupportedFieldType,
typeString: SupportedDataType,
{ useNestedFunction, isStats }: { useNestedFunction: boolean; isStats: boolean }
) {
if (useNestedFunction && isStats) {
Expand Down Expand Up @@ -1082,7 +1089,7 @@ function tweakSignatureForRowCommand(signature: string): string {
*/
let ret = signature;
for (const [type, value] of Object.entries(fieldTypesToConstants)) {
ret = ret.replace(new RegExp(fieldNameFromType(type as SupportedFieldType), 'g'), value);
ret = ret.replace(new RegExp(fieldNameFromType(type as SupportedDataType), 'g'), value);
}
return ret;
}
Expand All @@ -1101,8 +1108,8 @@ function getFieldMapping(
};

return params.map(({ name: _name, type, constantOnly, literalOptions, ...rest }) => {
const typeString: string = type;
if (isSupportedFieldType(typeString)) {
const typeString: string = type as string;
if (isSupportedDataType(typeString)) {
if (useLiterals && literalOptions) {
return {
name: `"${literalOptions[0]}"`,
Expand Down Expand Up @@ -1146,7 +1153,7 @@ function generateIncorrectlyTypedParameters(
name: string,
signatures: FunctionDefinition['signatures'],
currentParams: FunctionDefinition['signatures'][number]['params'],
availableFields: Array<{ name: string; type: SupportedFieldType }>
availableFields: Array<{ name: string; type: SupportedDataType }>
) {
const literalValues = {
string: `"a"`,
Expand All @@ -1167,7 +1174,7 @@ function generateIncorrectlyTypedParameters(

if (type !== 'any') {
// try to find an unacceptable field
const unacceptableField: { name: string; type: SupportedFieldType } | undefined =
const unacceptableField: { name: string; type: SupportedDataType } | undefined =
availableFields
// sort to make the test deterministic
.sort((a, b) => a.type.localeCompare(b.type))
Expand All @@ -1187,7 +1194,7 @@ function generateIncorrectlyTypedParameters(
}

// failed to find a bad field... they must all be acceptable
const acceptableField: { name: string; type: SupportedFieldType } | undefined =
const acceptableField: { name: string; type: SupportedDataType } | undefined =
type === 'any'
? availableFields[0]
: availableFields.find(({ type: fieldType }) => fieldType === type);
Expand Down
18 changes: 12 additions & 6 deletions packages/kbn-esql-validation-autocomplete/src/__tests__/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,27 @@
*/

import { camelCase } from 'lodash';
import { supportedFieldTypes } from '../definitions/types';
import { ESQLRealField } from '../validation/types';
import { fieldTypes } from '../definitions/types';

export const fields = [
...supportedFieldTypes.map((type) => ({ name: `${camelCase(type)}Field`, type })),
export const fields: ESQLRealField[] = [
...fieldTypes
.map((type) => ({ name: `${camelCase(type)}Field`, type }))
.filter((f) => f.type !== 'unsupported'),
{ name: 'any#Char$Field', type: 'double' },
{ name: 'kubernetes.something.something', type: 'double' },
{ name: '@timestamp', type: 'date' },
];

export const enrichFields = [
export const enrichFields: ESQLRealField[] = [
{ name: 'otherField', type: 'text' },
{ name: 'yetAnotherField', type: 'double' },
];

// eslint-disable-next-line @typescript-eslint/naming-convention
export const unsupported_field = [{ name: 'unsupported_field', type: 'unsupported' }];
export const unsupported_field: ESQLRealField[] = [
{ name: 'unsupported_field', type: 'unsupported' },
];

export const indexes = [
'a_index',
Expand Down Expand Up @@ -58,7 +63,8 @@ export function getCallbackMocks() {
return unsupported_field;
}
if (/dissect|grok/.test(query)) {
return [{ name: 'firstWord', type: 'text' }];
const field: ESQLRealField = { name: 'firstWord', type: 'text' };
return [field];
}
return fields;
}),
Expand Down
32 changes: 15 additions & 17 deletions ...sql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
import { ESQL_COMMON_NUMERIC_TYPES, ESQL_NUMBER_TYPES } from '../../shared/esql_types';
import { setup, getFunctionSignaturesByReturnType, getFieldNamesByType } from './helpers';

const ESQL_NUMERIC_TYPES = ESQL_NUMBER_TYPES as unknown as string[];

const allAggFunctions = getFunctionSignaturesByReturnType('stats', 'any', {
agg: true,
});
Expand Down Expand Up @@ -84,51 +82,51 @@ describe('autocomplete.suggest', () => {
scalar: true,
}).map((s) => ({ ...s, text: `${s.text},` })),
]);

const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const;
await assertSuggestions('from a | stats round(/', [
...getFunctionSignaturesByReturnType('stats', ESQL_NUMERIC_TYPES, {
...getFunctionSignaturesByReturnType('stats', roundParameterTypes, {
agg: true,
grouping: true,
}),
...getFieldNamesByType(ESQL_NUMERIC_TYPES),
...getFieldNamesByType(roundParameterTypes),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_NUMERIC_TYPES,
roundParameterTypes,
{ scalar: true },
undefined,
['round']
),
]);
await assertSuggestions('from a | stats round(round(/', [
...getFunctionSignaturesByReturnType('stats', ESQL_NUMERIC_TYPES, { agg: true }),
...getFieldNamesByType(ESQL_NUMERIC_TYPES),
...getFunctionSignaturesByReturnType('stats', roundParameterTypes, { agg: true }),
...getFieldNamesByType(roundParameterTypes),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_NUMERIC_TYPES,
ESQL_NUMBER_TYPES,
{ scalar: true },
undefined,
['round']
),
]);
await assertSuggestions('from a | stats avg(round(/', [
...getFieldNamesByType(ESQL_NUMERIC_TYPES),
...getFieldNamesByType(roundParameterTypes),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_NUMERIC_TYPES,
ESQL_NUMBER_TYPES,
{ scalar: true },
undefined,
['round']
),
]);
await assertSuggestions('from a | stats avg(/', [
...getFieldNamesByType(ESQL_NUMERIC_TYPES),
...getFunctionSignaturesByReturnType('eval', ESQL_NUMERIC_TYPES, { scalar: true }),
...getFieldNamesByType(ESQL_NUMBER_TYPES),
...getFunctionSignaturesByReturnType('eval', ESQL_NUMBER_TYPES, { scalar: true }),
]);
await assertSuggestions('from a | stats round(avg(/', [
...getFieldNamesByType(ESQL_NUMERIC_TYPES),
...getFieldNamesByType(ESQL_NUMBER_TYPES),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_NUMERIC_TYPES,
ESQL_NUMBER_TYPES,
{ scalar: true },
undefined,
['round']
Expand All @@ -142,7 +140,7 @@ describe('autocomplete.suggest', () => {
...getFieldNamesByType([...ESQL_COMMON_NUMERIC_TYPES, 'date', 'boolean', 'ip']),
...getFunctionSignaturesByReturnType(
'stats',
[...ESQL_COMMON_NUMERIC_TYPES, 'date', 'boolean', 'ip'],
[...ESQL_COMMON_NUMERIC_TYPES, 'date', 'date_period', 'boolean', 'ip'],
{
scalar: true,
}
Expand All @@ -158,7 +156,7 @@ describe('autocomplete.suggest', () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | stats avg(b/) by stringField', [
...getFieldNamesByType('double'),
...getFieldNamesByType(ESQL_NUMBER_TYPES),
...getFunctionSignaturesByReturnType(
'eval',
['double', 'integer', 'long', 'unsigned_long'],
Expand Down
Loading

0 comments on commit edf5c76

Please sign in to comment.