From 97e554cd3b6dda8c4effd3261c7b1f7cea5788f0 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 2 Apr 2024 19:15:57 +0200 Subject: [PATCH] [ES|QL] client-side validation: make `and` and `or` accept `null` (#179707) ## Summary Part of https://github.com/elastic/kibana/issues/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 --- packages/kbn-esql-ast/src/ast_walker.ts | 2 +- .../src/definitions/builtin.ts | 21 ++++++++ .../esql_validation_meta_tests.json | 50 +++++++++++++++++++ .../src/validation/validation.test.ts | 15 ++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/packages/kbn-esql-ast/src/ast_walker.ts b/packages/kbn-esql-ast/src/ast_walker.ts index 05393d7b07c161..740986ec3edd62 100644 --- a/packages/kbn-esql-ast/src/ast_walker.ts +++ b/packages/kbn-esql-ast/src/ast_walker.ts @@ -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: diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts index 668a2cd20407b9..23da9e1684cf4f 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/builtin.ts @@ -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', + }, ], })), { diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json index 0db7e695f8a627..d6919bb97551f6 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json +++ b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json @@ -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": [], @@ -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": [ @@ -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": [], diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts index 1d934731b9725f..498d5f264a8f6c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -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 @@ -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, @@ -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\`\`)\` `,