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.

Sign up for GitHub

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] client-side validation: make and and or accept null #179707

Merged
merged 3 commits into from
Apr 2, 2024
Merged

[ES|QL] client-side validation: make and and or accept null #179707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ff9d1c
'and' and 'or' accept null
drewdaemon Mar 29, 2024
bed58e7
Merge branch 'main' into 177699/fix-or-null-validation
stratoula Apr 2, 2024
3a52cce
add where test case
drewdaemon Apr 2, 2024
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
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/ast_walker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ function getBooleanValue(ctx: BooleanLiteralContext | BooleanValueContext) {

function getConstant(ctx: ConstantContext | undefined): ESQLAstItem | undefined {
if (ctx instanceof NullLiteralContext) {
return createLiteral('string', ctx.NULL());
return createLiteral('null', ctx.NULL());
}
if (ctx instanceof QualifiedIntegerLiteralContext) {
// despite the generic name, this is a date unit constant:
Expand Down
21 changes: 21 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts
View file
Open in desktop
drewdaemon marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,27 @@ export const builtinFunctions: FunctionDefinition[] = [
],
returnType: 'boolean',
},
{
params: [
{ name: 'left', type: 'null' },
{ name: 'right', type: 'boolean' },
],
returnType: 'boolean',
},
{
params: [
{ name: 'left', type: 'boolean' },
{ name: 'right', type: 'null' },
],
returnType: 'boolean',
},
{
params: [
{ name: 'left', type: 'null' },
{ name: 'right', type: 'null' },
],
returnType: 'boolean',
},
],
})),
{
Expand Down
50 changes: 50 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,46 @@
],
"warning": []
},
{
"query": "row bool_var = true and false",
"error": [],
"warning": []
},
{
"query": "row bool_var = true and null",
"error": [],
"warning": []
},
{
"query": "row bool_var = null and false",
"error": [],
"warning": []
},
{
"query": "row bool_var = null and null",
"error": [],
"warning": []
},
{
"query": "row bool_var = true or false",
"error": [],
"warning": []
},
{
"query": "row bool_var = true or null",
"error": [],
"warning": []
},
{
"query": "row bool_var = null or false",
"error": [],
"warning": []
},
{
"query": "row bool_var = null or null",
"error": [],
"warning": []
},
{
"query": "row var = abs(5)",
"error": [],
Expand Down Expand Up @@ -5118,6 +5158,11 @@
"error": [],
"warning": []
},
{
"query": "from a_index | where stringField == \"a\" or null",
"error": [],
"warning": []
},
{
"query": "from a_index | where avg(numberField)",
"error": [
Expand Down Expand Up @@ -10587,6 +10632,11 @@
"error": [],
"warning": []
},
{
"query": "from a_index | stats count(stringField == \"a\" or null)",
"error": [],
"warning": []
},
{
"query": "from a_index | stats count(`numberField`) | keep `count(``numberField``)` ",
"error": [],
Expand Down
15 changes: 15 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,15 @@ describe('validation logic', () => {
'Argument of [not_in] must be [number[]], found value [(1, 2, 3, "a")] type [(number, number, number, string)]',
]);

// test that "and" and "or" accept null... not sure if this is the best place or not...
for (const op of ['and', 'or']) {
for (const firstParam of ['true', 'null']) {
for (const secondParam of ['false', 'null']) {
testErrorsAndWarnings(`row bool_var = ${firstParam} ${op} ${secondParam}`, []);
}
}
}

function tweakSignatureForRowCommand(signature: string) {
/**
* row has no access to any field, so replace it with literal
Expand Down Expand Up @@ -1098,6 +1107,9 @@ describe('validation logic', () => {
testErrorsAndWarnings(`from a_index | where ${camelCase(field)}Field Is nOt NuLL`, []);
}

// this is a scenario that was failing because "or" didn't accept "null"
testErrorsAndWarnings('from a_index | where stringField == "a" or null', []);

for (const {
name,
alias,
Expand Down Expand Up @@ -1742,6 +1754,9 @@ describe('validation logic', () => {
]);
testErrorsAndWarnings('from a_index | stats count(`numberField`)', []);

// this is a scenario that was failing because "or" didn't accept "null"
testErrorsAndWarnings('from a_index | stats count(stringField == "a" or null)', []);

for (const subCommand of ['keep', 'drop', 'eval']) {
testErrorsAndWarnings(
`from a_index | stats count(\`numberField\`) | ${subCommand} \`count(\`\`numberField\`\`)\` `,
Expand Down