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.

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

[ES|QL] Create expression type evaluator #195989

Merged
merged 18 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions packages/kbn-esql-ast/src/parser/__tests__/literal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('literal expression', () => {
});
});

it('decimals vs integers', () => {
it('doubles vs integers', () => {
const text = 'ROW a(1.0, 1)';
const { ast } = parse(text);

Expand All @@ -36,7 +36,7 @@ describe('literal expression', () => {
args: [
{
type: 'literal',
literalType: 'decimal',
literalType: 'double',
},
{
type: 'literal',
Expand Down
9 changes: 5 additions & 4 deletions packages/kbn-esql-ast/src/parser/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import type {
FunctionSubtype,
ESQLNumericLiteral,
ESQLOrderExpression,
InlineCastingType,
} from '../types';
import { parseIdentifier, getPosition } from './helpers';
import { Builder, type AstNodeParserFields } from '../builder';
Expand Down Expand Up @@ -72,7 +73,7 @@ export const createCommand = (name: string, ctx: ParserRuleContext) =>

export const createInlineCast = (ctx: InlineCastContext, value: ESQLInlineCast['value']) =>
Builder.expression.inlineCast(
{ castType: ctx.dataType().getText(), value },
{ castType: ctx.dataType().getText().toLowerCase() as InlineCastingType, value },
createParserFields(ctx)
);

Expand Down Expand Up @@ -107,7 +108,7 @@ export function createLiteralString(token: Token): ESQLLiteral {
const text = token.text!;
return {
type: 'literal',
literalType: 'string',
literalType: 'keyword',
text,
name: text,
value: text,
Expand Down Expand Up @@ -149,13 +150,13 @@ export function createLiteral(
location: getPosition(node.symbol),
incomplete: isMissingText(text),
};
if (type === 'decimal' || type === 'integer') {
if (type === 'double' || type === 'integer') {
return {
...partialLiteral,
literalType: type,
value: Number(text),
paramType: 'number',
} as ESQLNumericLiteral<'decimal'> | ESQLNumericLiteral<'integer'>;
} as ESQLNumericLiteral<'double'> | ESQLNumericLiteral<'integer'>;
} else if (type === 'param') {
throw new Error('Should never happen');
}
Expand Down
14 changes: 7 additions & 7 deletions packages/kbn-esql-ast/src/parser/walkers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ function getConstant(ctx: ConstantContext): ESQLAstItem {

// Decimal type covers multiple ES|QL types: long, double, etc.
if (ctx instanceof DecimalLiteralContext) {
return createNumericLiteral(ctx.decimalValue(), 'decimal');
return createNumericLiteral(ctx.decimalValue(), 'double');
}

// Integer type encompasses integer
Expand All @@ -358,7 +358,7 @@ function getConstant(ctx: ConstantContext): ESQLAstItem {
}
if (ctx instanceof StringLiteralContext) {
// String literal covers multiple ES|QL types: text and keyword types
return createLiteral('string', ctx.string_().QUOTED_STRING());
return createLiteral('keyword', ctx.string_().QUOTED_STRING());
}
if (
ctx instanceof NumericArrayLiteralContext ||
Expand All @@ -371,14 +371,14 @@ function getConstant(ctx: ConstantContext): ESQLAstItem {
const isDecimal =
numericValue.decimalValue() !== null && numericValue.decimalValue() !== undefined;
const value = numericValue.decimalValue() || numericValue.integerValue();
values.push(createNumericLiteral(value!, isDecimal ? 'decimal' : 'integer'));
values.push(createNumericLiteral(value!, isDecimal ? 'double' : 'integer'));
}
for (const booleanValue of ctx.getTypedRuleContexts(BooleanValueContext)) {
values.push(getBooleanValue(booleanValue)!);
}
for (const string of ctx.getTypedRuleContexts(StringContext)) {
// String literal covers multiple ES|QL types: text and keyword types
const literal = createLiteral('string', string.QUOTED_STRING());
const literal = createLiteral('keyword', string.QUOTED_STRING());
if (literal) {
values.push(literal);
}
Expand Down Expand Up @@ -534,7 +534,7 @@ function collectRegexExpression(ctx: BooleanExpressionContext): ESQLFunction[] {
const arg = visitValueExpression(regex.valueExpression());
if (arg) {
fn.args.push(arg);
const literal = createLiteral('string', regex._pattern.QUOTED_STRING());
const literal = createLiteral('keyword', regex._pattern.QUOTED_STRING());
if (literal) {
fn.args.push(literal);
}
Expand Down Expand Up @@ -672,7 +672,7 @@ export function visitDissect(ctx: DissectCommandContext) {
return [
visitPrimaryExpression(ctx.primaryExpression()),
...(pattern && textExistsAndIsValid(pattern.getText())
? [createLiteral('string', pattern), ...visitDissectOptions(ctx.commandOptions())]
? [createLiteral('keyword', pattern), ...visitDissectOptions(ctx.commandOptions())]
: []),
].filter(nonNullable);
}
Expand All @@ -682,7 +682,7 @@ export function visitGrok(ctx: GrokCommandContext) {
return [
visitPrimaryExpression(ctx.primaryExpression()),
...(pattern && textExistsAndIsValid(pattern.getText())
? [createLiteral('string', pattern)]
? [createLiteral('keyword', pattern)]
: []),
].filter(nonNullable);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ ROW
// 2
/* 3 */
// 4
/* 5 */ /* 6 */ 1::INTEGER /* 7 */ /* 8 */ // 9`);
/* 5 */ /* 6 */ 1::integer /* 7 */ /* 8 */ // 9`);
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-esql-ast/src/pretty_print/leaf_printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ export const LeafPrinter = {
return '?';
}
}
case 'string': {
case 'keyword': {
return String(node.value);
}
case 'decimal': {
case 'double': {
const isRounded = node.value % 1 === 0;

if (isRounded) {
Expand Down
31 changes: 27 additions & 4 deletions packages/kbn-esql-ast/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,33 @@ export type BinaryExpressionAssignmentOperator = '=';
export type BinaryExpressionComparisonOperator = '==' | '=~' | '!=' | '<' | '<=' | '>' | '>=';
export type BinaryExpressionRegexOperator = 'like' | 'not_like' | 'rlike' | 'not_rlike';

// from https://github.com/elastic/elasticsearch/blob/122e7288200ee03e9087c98dff6cebbc94e774aa/docs/reference/esql/functions/kibana/inline_cast.json
export type InlineCastingType =
| 'bool'
| 'boolean'
| 'cartesian_point'
| 'cartesian_shape'
| 'date_nanos'
| 'date_period'
| 'datetime'
| 'double'
| 'geo_point'
| 'geo_shape'
| 'int'
| 'integer'
| 'ip'
| 'keyword'
| 'long'
| 'string'
| 'text'
| 'time_duration'
| 'unsigned_long'
| 'version';

export interface ESQLInlineCast<ValueType = ESQLAstItem> extends ESQLAstBaseItem {
type: 'inlineCast';
value: ValueType;
castType: string;
castType: InlineCastingType;
}

/**
Expand Down Expand Up @@ -270,7 +293,7 @@ export interface ESQLList extends ESQLAstBaseItem {
values: ESQLLiteral[];
}

export type ESQLNumericLiteralType = 'decimal' | 'integer';
export type ESQLNumericLiteralType = 'double' | 'integer';

export type ESQLLiteral =
| ESQLDecimalLiteral
Expand All @@ -290,7 +313,7 @@ export interface ESQLNumericLiteral<T extends ESQLNumericLiteralType> extends ES
}
// We cast anything as decimal (e.g. 32.12) as generic decimal numeric type here
// @internal
export type ESQLDecimalLiteral = ESQLNumericLiteral<'decimal'>;
export type ESQLDecimalLiteral = ESQLNumericLiteral<'double'>;

// @internal
export type ESQLIntegerLiteral = ESQLNumericLiteral<'integer'>;
Expand All @@ -312,7 +335,7 @@ export interface ESQLNullLiteral extends ESQLAstBaseItem {
// @internal
export interface ESQLStringLiteral extends ESQLAstBaseItem {
type: 'literal';
literalType: 'string';
literalType: 'keyword';
value: string;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/visitor/contexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export class LimitCommandVisitorContext<
if (
arg &&
arg.type === 'literal' &&
(arg.literalType === 'integer' || arg.literalType === 'decimal')
(arg.literalType === 'integer' || arg.literalType === 'double')
) {
return arg;
}
Expand Down
20 changes: 10 additions & 10 deletions packages/kbn-esql-ast/src/walker/walker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ describe('structurally can walk all nodes', () => {
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"foo"',
},
{
Expand Down Expand Up @@ -375,7 +375,7 @@ describe('structurally can walk all nodes', () => {
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"2"',
},
{
Expand All @@ -390,7 +390,7 @@ describe('structurally can walk all nodes', () => {
},
{
type: 'literal',
literalType: 'decimal',
literalType: 'double',
name: '3.14',
},
]);
Expand Down Expand Up @@ -473,7 +473,7 @@ describe('structurally can walk all nodes', () => {
values: [
{
type: 'literal',
literalType: 'decimal',
literalType: 'double',
name: '3.3',
},
],
Expand All @@ -492,7 +492,7 @@ describe('structurally can walk all nodes', () => {
},
{
type: 'literal',
literalType: 'decimal',
literalType: 'double',
name: '3.3',
},
]);
Expand Down Expand Up @@ -600,27 +600,27 @@ describe('structurally can walk all nodes', () => {
expect(literals).toMatchObject([
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"a"',
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"b"',
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"c"',
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"d"',
},
{
type: 'literal',
literalType: 'string',
literalType: 'keyword',
name: '"e"',
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const extraFunctions: FunctionDefinition[] = [
{ name: 'value', type: 'any' },
],
minParams: 2,
returnType: 'any',
returnType: 'unknown',
},
],
examples: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ describe('autocomplete.suggest', () => {
...getFieldNamesByType('integer'),
...getFieldNamesByType('double'),
...getFieldNamesByType('long'),
'`avg(b)`',
...getFunctionSignaturesByReturnType('eval', ['integer', 'double', 'long'], {
scalar: true,
}),
Expand All @@ -284,11 +283,19 @@ describe('autocomplete.suggest', () => {
const { assertSuggestions } = await setup();

await assertSuggestions('from a | stats avg(b) by doubleField % 2 /', [',', '| ']);
await assertSuggestions('from a | stats avg(b) by doubleField % 2 /', [',', '| '], {
triggerCharacter: ' ',
});

await assertSuggestions(
'from a | stats var0 = AVG(doubleField) BY var1 = BUCKET(dateField, 1 day)/',
[',', '| ', '+ $0', '- $0']
);
await assertSuggestions(
'from a | stats var0 = AVG(doubleField) BY var1 = BUCKET(dateField, 1 day) /',
[',', '| ', '+ $0', '- $0'],
{ triggerCharacter: ' ' }
);
});
test('on space within bucket()', async () => {
const { assertSuggestions } = await setup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1780,10 +1780,15 @@ async function getOptionArgsSuggestions(
innerText,
command,
option,
{ type: argDef?.type || 'any' },
{ type: argDef?.type || 'unknown' },
nodeArg,
nodeArgType as string,
references,
{
fields: references.fields,
// you can't use a variable defined
// in the stats command in the by clause
variables: new Map(),
},
getFieldsByType
))
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export const getBuiltinCompatibleFunctionDefinition = (
({ name, supportedCommands, supportedOptions, signatures, ignoreAsSuggestion }) =>
(command === 'where' && commandsToInclude ? commandsToInclude.indexOf(name) > -1 : true) &&
!ignoreAsSuggestion &&
!/not_/.test(name) &&
(!skipAssign || name !== '=') &&
(option ? supportedOptions?.includes(option) : supportedCommands.includes(command)) &&
signatures.some(
Expand All @@ -79,7 +78,10 @@ export const getBuiltinCompatibleFunctionDefinition = (
return compatibleFunctions
.filter((mathDefinition) =>
mathDefinition.signatures.some(
(signature) => returnTypes[0] === 'any' || returnTypes.includes(signature.returnType)
(signature) =>
returnTypes[0] === 'unknown' ||
returnTypes[0] === 'any' ||
returnTypes.includes(signature.returnType)
)
)
.map(getSuggestionBuiltinDefinition);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,6 @@ export function getFunctionsToIgnoreForStats(command: ESQLCommand, argIndex: num
return isFunctionItem(arg) ? getFnContent(arg) : [];
}

/**
* Given a function signature, returns the parameter at the given position.
*
* Takes into account variadic functions (minParams), returning the last
* parameter if the position is greater than the number of parameters.
*
* @param signature
* @param position
* @returns
*/
export function getParamAtPosition(
{ params, minParams }: FunctionDefinition['signatures'][number],
position: number
) {
return params.length > position ? params[position] : minParams ? params[params.length - 1] : null;
}

/**
* Given a function signature, returns the parameter at the given position, even if it's undefined or null
*
Expand Down
Loading