[query-engine] Add special handling for invalid equality expressions in KQL comparisons (#1400)

## Changes

* Add a rule in KQL pest to allow `where [scalar] = [scalar]` so that a
more intentional error message can be generated hinting that `==` is the
most likely solution

Author: CodeBlanch

rust/experimental/query_engine/kql-parser/src/kql.pest

@@ -20,6 +20,7 @@ positive_infinity_token = { "+inf" }
 negative_infinity_token = { "-inf" }
 
 // Logical Tokens
+invalid_equals_token = @{ "=" ~ !("="|"~") }
 equals_token = @{ "==" }
 equals_insensitive_token = @{ "=~" }
 not_equals_token = @{ "!=" }
@@ -250,7 +251,7 @@ scalar_arithmetic_binary_expression = _{
     | (plus_token|minus_token) ~ scalar_unary_expression
 }
 scalar_logical_binary_expression = _{
-    (equals_token|equals_insensitive_token|not_equals_token|not_equals_insensitive_token|greater_than_token|greater_than_or_equal_to_token|less_than_token|less_than_or_equal_to_token) ~ scalar_unary_expression
+    (equals_token|equals_insensitive_token|not_equals_token|not_equals_insensitive_token|greater_than_token|greater_than_or_equal_to_token|less_than_token|less_than_or_equal_to_token|invalid_equals_token) ~ scalar_unary_expression
     | matches_regex_token ~ scalar_unary_expression
     | (not_contains_cs_token|not_contains_token|not_has_cs_token|not_has_token|contains_cs_token|contains_token|has_cs_token|has_token) ~ scalar_unary_expression
     | (not_in_insensitive_token|not_in_token|in_insensitive_token|in_token) ~ scalar_list_expression

rust/experimental/query_engine/kql-parser/src/logical_expressions.rs

@@ -53,6 +53,7 @@ mod tests {
             &[
                 "1 == 1",
                 "1 =~ 1",
+                "1 = 1", // Note: Only valid for pest parsing, should fail when translated to expressions
                 "(1) != true",
                 "1 !~ true",
                 "(1==1) > false",
@@ -78,7 +79,7 @@ mod tests {
 
     #[test]
     fn test_parse_comparison_expression() {
-        let run_test = |input: &str, expected: LogicalExpression| {
+        let run_test_success = |input: &str, expected: LogicalExpression| {
             println!("Testing: {input}");
 
             let state = ParserState::new_with_options(
@@ -107,7 +108,21 @@ mod tests {
             assert_eq!(expected, expression);
         };
 
-        run_test(
+        let run_test_failure = |input: &str, expected: &str| {
+            let state = ParserState::new(input);
+
+            let mut result = KqlPestParser::parse(Rule::logical_expression, input).unwrap();
+
+            let error = parse_logical_expression(result.next().unwrap(), &state).unwrap_err();
+
+            if let ParserError::SyntaxError(_, msg) = error {
+                assert_eq!(expected, msg);
+            } else {
+                panic!("Expected SyntaxError");
+            }
+        };
+
+        run_test_success(
             "variable == 'hello world'",
             LogicalExpression::EqualTo(EqualToLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -123,7 +138,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "variable =~ 'hello world'",
             LogicalExpression::EqualTo(EqualToLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -139,7 +154,12 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_failure(
+            "variable = 'hello world'",
+            "Unexpected assignment operator. Did you mean to use '==' instead?",
+        );
+
+        run_test_success(
             "variable !~ 'hello world'",
             LogicalExpression::Not(NotLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -158,7 +178,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "variable != true",
             LogicalExpression::Not(NotLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -177,7 +197,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "1 > source_path",
             LogicalExpression::GreaterThan(GreaterThanLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -196,7 +216,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "(1) >= resource.key",
             LogicalExpression::GreaterThanOrEqualTo(GreaterThanOrEqualToLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -216,7 +236,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "0 < (variable)",
             LogicalExpression::Not(NotLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -236,7 +256,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "(0) <= variable",
             LogicalExpression::Not(NotLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -255,7 +275,7 @@ mod tests {
         );
 
         // Note: This whole expression folds to a constant value.
-        run_test(
+        run_test_success(
             "'hello world' matches regex '^hello world$'",
             LogicalExpression::Scalar(ScalarExpression::Static(StaticScalarExpression::Boolean(
                 BooleanScalarExpression::new(QueryLocation::new_fake(), true),
@@ -264,7 +284,7 @@ mod tests {
 
         // Note: The string regex pattern gets folded into a compiled regex
         // expression.
-        run_test(
+        run_test_success(
             "Name matches regex '^hello world$'",
             LogicalExpression::Matches(MatchesLogicalExpression::new(
                 QueryLocation::new_fake(),
@@ -286,7 +306,7 @@ mod tests {
             )),
         );
 
-        run_test(
+        run_test_success(
             "Name matches regex const_regex",
             LogicalExpression::Matches(MatchesLogicalExpression::new(
                 QueryLocation::new_fake(),

rust/experimental/query_engine/kql-parser/src/scalar_expression.rs

@@ -29,7 +29,8 @@ static PRATT_PARSER: LazyLock<PrattParser<Rule>> = LazyLock::new(|| {
         .op(Op::infix(equals_token, Left)
             | Op::infix(equals_insensitive_token, Left)
             | Op::infix(not_equals_token, Left)
-            | Op::infix(not_equals_insensitive_token, Left))
+            | Op::infix(not_equals_insensitive_token, Left)
+            | Op::infix(invalid_equals_token, Left))
         // <= >= < >
         .op(Op::infix(less_than_or_equal_to_token, Left)
             | Op::infix(greater_than_or_equal_to_token, Left)
@@ -127,6 +128,13 @@ pub(crate) fn parse_scalar_expression(
                     .into(),
                 ),
 
+                Rule::invalid_equals_token => {
+                    return Err(ParserError::SyntaxError(
+                        location,
+                        "Unexpected assignment operator. Did you mean to use '==' instead?".into(),
+                    ));
+                }
+
                 Rule::greater_than_token => ScalarExpression::Logical(
                     LogicalExpression::GreaterThan(GreaterThanLogicalExpression::new(
                         location, lhs, rhs,