diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts index a371ccad1b5a2..b68e9aa987051 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts @@ -67,7 +67,7 @@ function getMessageAndTypeFromId({ return { message: i18n.translate('monaco.esql.validation.noNestedArgumentSupport', { defaultMessage: - "Aggregate function's parameters must be an attribute or literal; found [{name}] of type [{argType}]", + "Aggregate function's parameters must be an attribute, literal or a non-aggregation function; found [{name}] of type [{argType}]", values: { name: out.name, argType: out.argType }, }), }; diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts index 2495111f3a9db..994c250bfacb9 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts @@ -1170,30 +1170,18 @@ describe('validation logic', () => { 'from a | stats avg(numberField), percentile(numberField, 50) BY ipField', [] ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY numberField % 2', - [] - ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY var0 = numberField % 2', - [] - ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY numberField % 2, numberField + 1', - [] - ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY var0 = numberField % 2, var1 = numberField + 1', - [] - ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY var0 = numberField % 2, numberField + 1', - [] - ); - testErrorsAndWarnings( - 'from a | stats avg(numberField), percentile(numberField, 50) BY var0 = numberField % 2, ipField', - [] - ); + testErrorsAndWarnings('from a | stats count(* + 1) BY ipField', [ + 'SyntaxError: expected {STRING, INTEGER_LITERAL, DECIMAL_LITERAL, FALSE, LP, NOT, NULL, PARAM, TRUE, PLUS, MINUS, OPENING_BRACKET, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER} but found "+"', + ]); + testErrorsAndWarnings('from a | stats count(* + round(numberField)) BY ipField', [ + 'SyntaxError: expected {STRING, INTEGER_LITERAL, DECIMAL_LITERAL, FALSE, LP, NOT, NULL, PARAM, TRUE, PLUS, MINUS, OPENING_BRACKET, UNQUOTED_IDENTIFIER, QUOTED_IDENTIFIER} but found "+"', + ]); + testErrorsAndWarnings('from a | stats count(round(*)) BY ipField', [ + 'Using wildcards (*) in round is not allowed', + ]); + testErrorsAndWarnings('from a | stats count(count(*)) BY ipField', [ + `Aggregate function's parameters must be an attribute, literal or a non-aggregation function; found [count(*)] of type [number]`, + ]); testErrorsAndWarnings('from a | stats numberField + 1', ['STATS does not support function +']); testErrorsAndWarnings('from a | stats numberField + 1 by ipField', [ @@ -1220,7 +1208,8 @@ describe('validation logic', () => { { name, ...defRest, signatures: [{ params: fieldMapping, returnType }] }, { withTypes: false } )[0].declaration - }` + }`, + [] ); testErrorsAndWarnings( `from a | stats ${ @@ -1228,7 +1217,8 @@ describe('validation logic', () => { { name, ...defRest, signatures: [{ params: fieldMapping, returnType }] }, { withTypes: false } )[0].declaration - }` + }`, + [] ); if (alias) { @@ -1242,6 +1232,82 @@ describe('validation logic', () => { } } + // test only numeric functions for now + if (params[0].type === 'number') { + const nestedBuiltin = 'numberField / 2'; + const fieldMappingWithNestedBuiltinFunctions = getFieldMapping(params); + fieldMappingWithNestedBuiltinFunctions[0].name = nestedBuiltin; + + const fnSignatureWithBuiltinString = getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMappingWithNestedBuiltinFunctions, returnType }], + }, + { withTypes: false } + )[0].declaration; + // FROM a | STATS aggFn( numberField / 2 ) + testErrorsAndWarnings(`from a | stats ${fnSignatureWithBuiltinString}`, []); + testErrorsAndWarnings(`from a | stats var0 = ${fnSignatureWithBuiltinString}`, []); + testErrorsAndWarnings( + `from a | stats avg(numberField), ${fnSignatureWithBuiltinString}`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), var0 = ${fnSignatureWithBuiltinString}`, + [] + ); + + const nestedEvalAndBuiltin = 'round(numberField / 2)'; + const fieldMappingWithNestedEvalAndBuiltinFunctions = getFieldMapping(params); + fieldMappingWithNestedBuiltinFunctions[0].name = nestedEvalAndBuiltin; + + const fnSignatureWithEvalAndBuiltinString = getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMappingWithNestedEvalAndBuiltinFunctions, returnType }], + }, + { withTypes: false } + )[0].declaration; + // FROM a | STATS aggFn( round(numberField / 2) ) + testErrorsAndWarnings(`from a | stats ${fnSignatureWithEvalAndBuiltinString}`, []); + testErrorsAndWarnings(`from a | stats var0 = ${fnSignatureWithEvalAndBuiltinString}`, []); + testErrorsAndWarnings( + `from a | stats avg(numberField), ${fnSignatureWithEvalAndBuiltinString}`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), var0 = ${fnSignatureWithEvalAndBuiltinString}`, + [] + ); + // FROM a | STATS aggFn(round(numberField / 2) ) BY round(numberField / 2) + testErrorsAndWarnings( + `from a | stats ${fnSignatureWithEvalAndBuiltinString} by ${nestedEvalAndBuiltin}`, + [] + ); + testErrorsAndWarnings( + `from a | stats var0 = ${fnSignatureWithEvalAndBuiltinString} by var1 = ${nestedEvalAndBuiltin}`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), ${fnSignatureWithEvalAndBuiltinString} by ${nestedEvalAndBuiltin}, ipField`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), var0 = ${fnSignatureWithEvalAndBuiltinString} by var1 = ${nestedEvalAndBuiltin}, ipField`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), ${fnSignatureWithEvalAndBuiltinString} by ${nestedEvalAndBuiltin}, ${nestedBuiltin}`, + [] + ); + testErrorsAndWarnings( + `from a | stats avg(numberField), var0 = ${fnSignatureWithEvalAndBuiltinString} by var1 = ${nestedEvalAndBuiltin}, ${nestedBuiltin}`, + [] + ); + } + // Skip functions that have only arguments of type "any", as it is not possible to pass "the wrong type". // auto_bucket and to_version functions are a bit harder to test exactly a combination of argument and predict the // the right error message @@ -1250,7 +1316,7 @@ describe('validation logic', () => { !['auto_bucket', 'to_version'].includes(name) ) { // now test nested functions - const fieldMappingWithNestedFunctions = getFieldMapping(params, { + const fieldMappingWithNestedAggsFunctions = getFieldMapping(params, { useNestedFunction: true, useLiterals: false, }); @@ -1260,14 +1326,30 @@ describe('validation logic', () => { { name, ...defRest, - signatures: [{ params: fieldMappingWithNestedFunctions, returnType }], + signatures: [{ params: fieldMappingWithNestedAggsFunctions, returnType }], + }, + { withTypes: false } + )[0].declaration + }`, + params.map( + (_) => + `Aggregate function's parameters must be an attribute, literal or a non-aggregation function; found [avg(numberField)] of type [number]` + ) + ); + testErrorsAndWarnings( + `from a | stats ${ + getFunctionSignatures( + { + name, + ...defRest, + signatures: [{ params: fieldMappingWithNestedAggsFunctions, returnType }], }, { withTypes: false } )[0].declaration }`, params.map( (_) => - `Aggregate function's parameters must be an attribute or literal; found [avg(numberField)] of type [number]` + `Aggregate function's parameters must be an attribute, literal or a non-aggregation function; found [avg(numberField)] of type [number]` ) ); // and the message is case of wrong argument type is passed diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts index 4a2415e37805e..e1bde9874b40f 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts @@ -36,6 +36,7 @@ import { hasWildcard, hasCCSSource, isSettingItem, + isAssignment, } from '../shared/helpers'; import { collectVariables } from '../shared/variables'; import type { @@ -222,15 +223,25 @@ function validateFunction( astFunction: ESQLFunction, parentCommand: string, parentOption: string | undefined, - references: ReferenceMaps + references: ReferenceMaps, + isNested?: boolean ): ESQLMessage[] { const messages: ESQLMessage[] = []; if (astFunction.incomplete) { return messages; } + const fnDefinition = getFunctionDefinition(astFunction.name)!; + const supportNestedFunctions = + fnDefinition?.signatures.some(({ params }) => + params.some(({ noNestingFunctions }) => !noNestingFunctions) + ) || true; - const isFnSupported = isSupportedFunction(astFunction.name, parentCommand, parentOption); + const isFnSupported = isSupportedFunction( + astFunction.name, + isNested && !supportNestedFunctions ? 'eval' : parentCommand, + parentOption + ); if (!isFnSupported.supported) { if (isFnSupported.reason === 'unknownFunction') { @@ -244,7 +255,8 @@ function validateFunction( }) ); } - if (isFnSupported.reason === 'unsupportedFunction') { + // for nested functions skip this check and make the nested check fail later on + if (isFnSupported.reason === 'unsupportedFunction' && !isNested) { messages.push( parentOption ? getMessageFromId({ @@ -263,9 +275,10 @@ function validateFunction( }) ); } - return messages; + if (messages.length) { + return messages; + } } - const fnDefinition = getFunctionDefinition(astFunction.name)!; const matchingSignatures = fnDefinition.signatures.filter((def) => { if (def.infiniteParams && astFunction.args.length > 0) { return true; @@ -297,7 +310,15 @@ function validateFunction( const wrappedArray = Array.isArray(arg) ? arg : [arg]; for (const subArg of wrappedArray) { if (isFunctionItem(subArg)) { - messages.push(...validateFunction(subArg, parentCommand, parentOption, references)); + messages.push( + ...validateFunction( + subArg, + parentCommand, + parentOption, + references, + isNested || !isAssignment(astFunction) + ) + ); } } } @@ -322,6 +343,7 @@ function validateFunction( // few lines above return; } + // if the arg is an array of values, check each element if (Array.isArray(outerArg) && isArrayType(argDef.type)) { const extractedType = extractSingleType(argDef.type); const everyArgInListMessages = outerArg