Skip to content

Commit

Permalink
[ES|QL] client-side validation: make and and or accept null (#1…
Browse files Browse the repository at this point in the history
…79707)

## Summary

Part of #177699

`AND` and `OR` were not accepting `NULL` as an argument when they should
have.

The most basic test case is

`ROW var = TRUE or NULL`.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
  • Loading branch information
drewdaemon and stratoula authored Apr 2, 2024
1 parent d81e03e commit 97e554c
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/ast_walker.ts
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
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
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
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

0 comments on commit 97e554c

Please sign in to comment.