[ES|QL] Small refactoring (#213439)

## Summary

Just a small refactoring here to make the code a bit more readable. Also
functions inside functions didnt make a lot of sense there
This commit is contained in:
Stratoula Kalafateli 2025-03-10 10:58:40 +01:00 committed by GitHub
parent 1e1fc8011e
commit 5f9f88a80e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 85 additions and 71 deletions

View file

@ -11,22 +11,12 @@ import { i18n } from '@kbn/i18n';
import { import {
type ESQLColumn, type ESQLColumn,
type ESQLCommand, type ESQLCommand,
type ESQLAstItem,
type ESQLMessage, type ESQLMessage,
type ESQLFunction, type ESQLFunction,
isFunctionExpression, isFunctionExpression,
isWhereExpression, isWhereExpression,
isFieldExpression,
Walker,
} from '@kbn/esql-ast'; } from '@kbn/esql-ast';
import { import { isAssignment, isColumnItem, isFunctionItem } from '../shared/helpers';
getFunctionDefinition,
isAssignment,
isColumnItem,
isFunctionItem,
isFunctionOperatorParam,
isLiteralItem,
} from '../shared/helpers';
import { import {
appendSeparatorOption, appendSeparatorOption,
asOption, asOption,
@ -37,7 +27,7 @@ import {
} from './options'; } from './options';
import { ENRICH_MODES } from './settings'; import { ENRICH_MODES } from './settings';
import { type CommandDefinition, FunctionDefinitionTypes } from './types'; import { type CommandDefinition } from './types';
import { suggest as suggestForSort } from '../autocomplete/commands/sort'; import { suggest as suggestForSort } from '../autocomplete/commands/sort';
import { suggest as suggestForKeep } from '../autocomplete/commands/keep'; import { suggest as suggestForKeep } from '../autocomplete/commands/keep';
import { suggest as suggestForDrop } from '../autocomplete/commands/drop'; import { suggest as suggestForDrop } from '../autocomplete/commands/drop';
@ -50,6 +40,7 @@ import { suggest as suggestForShow } from '../autocomplete/commands/show';
import { suggest as suggestForGrok } from '../autocomplete/commands/grok'; import { suggest as suggestForGrok } from '../autocomplete/commands/grok';
import { suggest as suggestForDissect } from '../autocomplete/commands/dissect'; import { suggest as suggestForDissect } from '../autocomplete/commands/dissect';
import { suggest as suggestForEnrich } from '../autocomplete/commands/enrich'; import { suggest as suggestForEnrich } from '../autocomplete/commands/enrich';
import { checkAggExistence, checkFunctionContent } from './commands_helpers';
const statsValidator = (command: ESQLCommand) => { const statsValidator = (command: ESQLCommand) => {
const messages: ESQLMessage[] = []; const messages: ESQLMessage[] = [];
@ -82,45 +73,6 @@ const statsValidator = (command: ESQLCommand) => {
.filter(isFunctionItem); .filter(isFunctionItem);
if (statsArg.length) { if (statsArg.length) {
function isAggFunction(arg: ESQLAstItem): arg is ESQLFunction {
return (
isFunctionItem(arg) && getFunctionDefinition(arg.name)?.type === FunctionDefinitionTypes.AGG
);
}
function isOtherFunction(arg: ESQLAstItem): arg is ESQLFunction {
return (
isFunctionItem(arg) && getFunctionDefinition(arg.name)?.type !== FunctionDefinitionTypes.AGG
);
}
function checkAggExistence(arg: ESQLFunction): boolean {
if (isWhereExpression(arg)) {
return checkAggExistence(arg.args[0] as ESQLFunction);
}
if (isFieldExpression(arg)) {
const agg = arg.args[1];
const firstFunction = Walker.match(agg, { type: 'function' });
if (!firstFunction) {
return false;
}
return checkAggExistence(firstFunction as ESQLFunction);
}
// TODO the grouping function check may not
// hold true for all future cases
if (isAggFunction(arg) || isFunctionOperatorParam(arg)) {
return true;
}
if (isOtherFunction(arg)) {
return (arg as ESQLFunction).args.filter(isFunctionItem).some(checkAggExistence);
}
return false;
}
// first check: is there an agg function somewhere? // first check: is there an agg function somewhere?
const noAggsExpressions = statsArg.filter((arg) => !checkAggExistence(arg)); const noAggsExpressions = statsArg.filter((arg) => !checkAggExistence(arg));
@ -144,26 +96,6 @@ const statsValidator = (command: ESQLCommand) => {
})) }))
); );
} else { } else {
function isConstantOrAggFn(arg: ESQLAstItem): boolean {
return isLiteralItem(arg) || isAggFunction(arg);
}
// now check that:
// * the agg function is at root level
// * or if it's a operators function, then all operands are agg functions or literals
// * or if it's a eval function then all arguments are agg functions or literals
// * or if a named param is used
function checkFunctionContent(arg: ESQLFunction) {
// TODO the grouping function check may not
// hold true for all future cases
if (isAggFunction(arg) || isFunctionOperatorParam(arg)) {
return true;
}
return (arg as ESQLFunction).args.every(
(subArg): boolean =>
isConstantOrAggFn(subArg) ||
(isOtherFunction(subArg) ? checkFunctionContent(subArg) : false)
);
}
// @TODO: improve here the check to get the last instance of the invalidExpression // @TODO: improve here the check to get the last instance of the invalidExpression
// to provide a better location for the error message // to provide a better location for the error message
// i.e. STATS round(round(round( a + sum(b) ))) // i.e. STATS round(round(round( a + sum(b) )))

View file

@ -0,0 +1,82 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/
import {
type ESQLAstItem,
type ESQLFunction,
isWhereExpression,
isFieldExpression,
Walker,
} from '@kbn/esql-ast';
import {
getFunctionDefinition,
isFunctionItem,
isFunctionOperatorParam,
isLiteralItem,
} from '../shared/helpers';
import { FunctionDefinitionTypes } from './types';
function isAggregation(arg: ESQLAstItem): arg is ESQLFunction {
return (
isFunctionItem(arg) && getFunctionDefinition(arg.name)?.type === FunctionDefinitionTypes.AGG
);
}
function isNotAnAggregation(arg: ESQLAstItem): arg is ESQLFunction {
return (
isFunctionItem(arg) && getFunctionDefinition(arg.name)?.type !== FunctionDefinitionTypes.AGG
);
}
// now check that:
// * the agg function is at root level
// * or if it's a operators function, then all operands are agg functions or literals
// * or if it's a eval function then all arguments are agg functions or literals
// * or if a named param is used
export function checkFunctionContent(arg: ESQLFunction) {
// TODO the grouping function check may not
// hold true for all future cases
if (isAggregation(arg) || isFunctionOperatorParam(arg)) {
return true;
}
return (arg as ESQLFunction).args.every(
(subArg): boolean =>
isLiteralItem(subArg) ||
isAggregation(subArg) ||
(isNotAnAggregation(subArg) ? checkFunctionContent(subArg) : false)
);
}
export function checkAggExistence(arg: ESQLFunction): boolean {
if (isWhereExpression(arg)) {
return checkAggExistence(arg.args[0] as ESQLFunction);
}
if (isFieldExpression(arg)) {
const agg = arg.args[1];
const firstFunction = Walker.match(agg, { type: 'function' });
if (!firstFunction) {
return false;
}
return checkAggExistence(firstFunction as ESQLFunction);
}
// TODO the grouping function check may not
// hold true for all future cases
if (isAggregation(arg) || isFunctionOperatorParam(arg)) {
return true;
}
if (isNotAnAggregation(arg)) {
return (arg as ESQLFunction).args.filter(isFunctionItem).some(checkAggExistence);
}
return false;
}