Skip to content
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
36 changes: 33 additions & 3 deletions packages/kbn-monaco/src/esql/lib/ast/definitions/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/

import { i18n } from '@kbn/i18n';
import { isColumnItem } from '../shared/helpers';
import { ESQLColumn, ESQLCommand, ESQLMessage } from '../types';
import {
appendSeparatorOption,
asOption,
Expand Down Expand Up @@ -42,7 +44,7 @@ export const commandDefinitions: CommandDefinition[] = [
options: [metadataOption],
signature: {
multipleParams: true,
params: [{ name: 'index', type: 'source' }],
params: [{ name: 'index', type: 'source', wildcards: true }],
},
},
{
Expand Down Expand Up @@ -122,7 +124,7 @@ export const commandDefinitions: CommandDefinition[] = [
options: [],
signature: {
multipleParams: true,
params: [{ name: 'column', type: 'column' }],
params: [{ name: 'column', type: 'column', wildcards: true }],
},
},
{
Expand All @@ -134,7 +136,35 @@ export const commandDefinitions: CommandDefinition[] = [
options: [],
signature: {
multipleParams: true,
params: [{ name: 'column', type: 'column' }],
params: [{ name: 'column', type: 'column', wildcards: true }],
},
validate: (command: ESQLCommand) => {
const messages: ESQLMessage[] = [];
const wildcardItems = command.args.filter((arg) => isColumnItem(arg) && arg.name === '*');
if (wildcardItems.length) {
messages.push(
...wildcardItems.map((column) => ({
location: (column as ESQLColumn).location,
text: i18n.translate('monaco.esql.validation.dropAllColumnsError', {
defaultMessage: 'Removing all fields is not allowed [*]',
}),
type: 'error' as const,
}))
);
}
const droppingTimestamp = command.args.find(
(arg) => isColumnItem(arg) && arg.name === '@timestamp'
);
if (droppingTimestamp) {
messages.push({
location: (droppingTimestamp as ESQLColumn).location,
text: i18n.translate('monaco.esql.validation.dropTimestampWarning', {
defaultMessage: 'Drop [@timestamp] will remove all time filters to the search results',
}),
type: 'warning',
});
}
return messages;
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export const appendSeparatorOption: CommandOptionsDefinition = {
params: [{ name: 'separator', type: 'string' }],
},
optional: true,
skipCommonValidation: true, // tell the validation engine to use only the validate function here
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any reason we can't just use the presence of the validate method to decide (i.e. a clean override)?

Copy link
Copy Markdown
Contributor Author

@dej611 dej611 Nov 1, 2023

Choose a reason for hiding this comment

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

Initially that was the design, but then I rolled back to this other decision.
There are several reasons for this change:

  • shared validation code is quite a big surface and replicate it every time to just add a tiny little bit of extra ad-hoc validation doesn't make sense
  • therefore I want a way to add only this extra validation bit on top of the common one
  • ideally I would like to have a "positive" flag (something like commonValidation: true) but I've only few cases (one so far) where this extra bit is required and adding a "negative" ("skipCommonValidation") was the less intrusive one

Another design approach could be to rename validate as extraValidation to semantically avoid this flag + callback confusion.

validate: (option: ESQLCommandOption) => {
const messages: ESQLMessage[] = [];
const [firstArg] = option.args;
Expand Down
5 changes: 4 additions & 1 deletion packages/kbn-monaco/src/esql/lib/ast/definitions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { ESQLCommandOption, ESQLMessage, ESQLSingleAstItem } from '../types';
import type { ESQLCommand, ESQLCommandOption, ESQLMessage, ESQLSingleAstItem } from '../types';

export interface FunctionDefinition {
name: string;
Expand Down Expand Up @@ -42,19 +42,22 @@ export interface CommandBaseDefinition {
optional?: boolean;
innerType?: string;
values?: string[];
wildcards?: boolean;
}>;
};
}

export interface CommandOptionsDefinition extends CommandBaseDefinition {
wrapped?: string[];
optional: boolean;
skipCommonValidation?: boolean;
validate?: (option: ESQLCommandOption) => ESQLMessage[];
}

export interface CommandDefinition extends CommandBaseDefinition {
options: CommandOptionsDefinition[];
examples: string[];
validate?: (option: ESQLCommand) => ESQLMessage[];
}

export interface Literals {
Expand Down
65 changes: 53 additions & 12 deletions packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,20 +363,61 @@ export function getDurationItemsWithQuantifier(quantifier: number = 1) {
}));
}

function fuzzySearch(fuzzyName: string, resources: IterableIterator<string>) {
const wildCardPosition = getWildcardPosition(fuzzyName);
if (wildCardPosition !== 'none') {
const matcher = getMatcher(fuzzyName, wildCardPosition);
for (const resourceName of resources) {
if (matcher(resourceName)) {
return true;
}
}
}
}

function getMatcher(name: string, position: 'start' | 'end' | 'middle') {
if (position === 'start') {
const prefix = name.substring(1);
return (resource: string) => resource.endsWith(prefix);
}
if (position === 'end') {
const prefix = name.substring(0, name.length - 1);
return (resource: string) => resource.startsWith(prefix);
}
const [prefix, postFix] = name.split('*');
return (resource: string) => resource.startsWith(prefix) && resource.endsWith(postFix);
}

function getWildcardPosition(name: string) {
if (!hasWildcard(name)) {
return 'none';
}
if (name.startsWith('*')) {
return 'start';
}
if (name.endsWith('*')) {
return 'end';
}
return 'middle';
}

export function hasWildcard(name: string) {
return name.includes('*');
}

export function columnExists(
column: string,
{ fields, variables }: Pick<ReferenceMaps, 'fields' | 'variables'>
) {
if (fields.has(column) || variables.has(column)) {
return true;
}
return Boolean(fuzzySearch(column, fields.keys()) || fuzzySearch(column, variables.keys()));
}

export function sourceExists(index: string, sources: Set<string>) {
if (sources.has(index)) {
return true;
}
// it is a fuzzy match
if (index[index.length - 1] === '*') {
const prefix = index.substring(0, index.length - 1);
for (const sourceName of sources.keys()) {
if (sourceName.includes(prefix)) {
// just to be sure that there's not an exact match here
// i.e. index-* should not match index-
return sourceName.length > prefix.length;
}
}
}
return false;
return Boolean(fuzzySearch(index, sources.keys()));
}
10 changes: 10 additions & 0 deletions packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ function getMessageAndTypeFromId<K extends ErrorTypes>({
},
}),
};
case 'wildcardNotSupportedForCommand':
return {
message: i18n.translate('monaco.esql.validation.wildcardNotSupportedForCommand', {
defaultMessage: 'Using wildcards (*) in {command} is not allowed [{value}]',
values: {
command: out.command,
value: out.value,
},
}),
};
}
return { message: '' };
}
Expand Down
4 changes: 4 additions & 0 deletions packages/kbn-monaco/src/esql/lib/ast/validation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ export interface ValidationErrors {
message: string;
type: { command: string; value: string };
};
wildcardNotSupportedForCommand: {
mesage: string;
type: { command: string; value: string };
};
}

export type ErrorTypes = keyof ValidationErrors;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ function getCallbackMocks() {
name: `listField`,
type: `list`,
},
{ name: '@timestamp', type: 'date' },
]
: [
{ name: 'otherField', type: 'string' },
Expand Down Expand Up @@ -224,7 +225,11 @@ describe('validation logic', () => {
'SyntaxError: expected {<EOF>, PIPE, COMMA, OPENING_BRACKET} but found "(metadata"',
]);
testErrorsAndWarnings(`from ind*, other*`, []);
testErrorsAndWarnings(`from index*`, ['Unknown index [index*]']);
testErrorsAndWarnings(`from index*`, []);
testErrorsAndWarnings(`from *ex`, []);
testErrorsAndWarnings(`from in*ex`, []);
testErrorsAndWarnings(`from ind*ex`, []);
testErrorsAndWarnings(`from indexes*`, ['Unknown index [indexes*]']);
});

describe('row', () => {
Expand Down Expand Up @@ -470,6 +475,14 @@ describe('validation logic', () => {
testErrorsAndWarnings('from index | project missingField, numberField, dateField', [
'Unknown column [missingField]',
]);
testErrorsAndWarnings('from index | keep s*', []);
testErrorsAndWarnings('from index | keep *Field', []);
testErrorsAndWarnings('from index | keep s*Field', []);
testErrorsAndWarnings('from index | keep string*Field', []);
testErrorsAndWarnings('from index | keep s*, n*', []);
testErrorsAndWarnings('from index | keep m*', ['Unknown column [m*]']);
testErrorsAndWarnings('from index | keep *m', ['Unknown column [*m]']);
testErrorsAndWarnings('from index | keep d*m', ['Unknown column [d*m]']);
});

describe('drop', () => {
Expand All @@ -489,6 +502,28 @@ describe('validation logic', () => {
testErrorsAndWarnings('from index | project missingField, numberField, dateField', [
'Unknown column [missingField]',
]);
testErrorsAndWarnings('from index | drop s*', []);
testErrorsAndWarnings('from index | drop *Field', []);
testErrorsAndWarnings('from index | drop s*Field', []);
testErrorsAndWarnings('from index | drop string*Field', []);
testErrorsAndWarnings('from index | drop s*, n*', []);
testErrorsAndWarnings('from index | drop m*', ['Unknown column [m*]']);
testErrorsAndWarnings('from index | drop *m', ['Unknown column [*m]']);
testErrorsAndWarnings('from index | drop d*m', ['Unknown column [d*m]']);
testErrorsAndWarnings('from index | drop *', ['Removing all fields is not allowed [*]']);
testErrorsAndWarnings('from index | drop stringField, *', [
'Removing all fields is not allowed [*]',
]);
testErrorsAndWarnings(
'from index | drop @timestamp',
[],
['Drop [@timestamp] will remove all time filters to the search results']
);
testErrorsAndWarnings(
'from index | drop stringField, @timestamp',
[],
['Drop [@timestamp] will remove all time filters to the search results']
);
});

describe('mv_expand', () => {
Expand Down Expand Up @@ -535,6 +570,10 @@ describe('validation logic', () => {
testErrorsAndWarnings('from a | eval numberField + 1 | rename `numberField + 1` as ', [
"SyntaxError: missing {SRC_UNQUOTED_IDENTIFIER, SRC_QUOTED_IDENTIFIER} at '<EOF>'",
]);
testErrorsAndWarnings('from a | rename s* as strings', [
'Using wildcards (*) in rename is not allowed [s*]',
'Unknown column [strings]',
]);
});

describe('dissect', () => {
Expand Down Expand Up @@ -576,6 +615,9 @@ describe('validation logic', () => {
testErrorsAndWarnings('from a | dissect stringField "%{a}" append_separator = true', [
'Invalid value for dissect append_separator: expected a string, but was [true]',
]);
// testErrorsAndWarnings('from a | dissect s* "%{a}"', [
// 'Using wildcards (*) in dissect is not allowed [s*]',
// ]);
});

describe('grok', () => {
Expand All @@ -596,6 +638,9 @@ describe('validation logic', () => {
testErrorsAndWarnings('from a | grok numberField "%{a}"', [
'Grok only supports string type values, found [numberField] of type number',
]);
// testErrorsAndWarnings('from a | grok s* "%{a}"', [
// 'Using wildcards (*) in grok is not allowed [s*]',
// ]);
});

describe('where', () => {
Expand Down Expand Up @@ -1185,6 +1230,9 @@ describe('validation logic', () => {
testErrorsAndWarnings(`from a | enrich policy with otherField`, []);
testErrorsAndWarnings(`from a | enrich policy | eval otherField`, []);
testErrorsAndWarnings(`from a | enrich policy with var0 = otherField | eval var0`, []);
testErrorsAndWarnings('from a | enrich my-pol*', [
'Using wildcards (*) in enrich is not allowed [my-pol*]',
]);
});

describe('shadowing', () => {
Expand Down
Loading