Add if/else if/else expression to KQL parser

[albertlockett]

part of #1667

Adds parsing support for an if/else if/else expression that gets parsed into the ConditionalDataExpression that was added in #1684

The syntax looks like this.

logs |
if (severity_text == "ERROR") { 
    extend attributes["important"] = "very" | extend attributes["triggers_alarm"] = "true"
} else if (severity_text == "WARN") {
    extend attributes["important"] = "somewhat"
} else if (severity_text == "INFO") {
    extend attributes["important"] = "rarely"
} else {
    extend attributes["important"] = "no"
}

else if and else are optional, so the following expressions are also supported:

logs | 
if (severity_text == "ERROR") { 
    extend attributes["is_error"] = true
} else {
    extend attributes["is_not_error"] = true
}

logs | 
if (severity_text == "ERROR") { 
    extend attributes["triggers_alarm"] = "true"
}

If we're OK with the syntax and how this is implemented, I'll go back and clean up the tests that we're added in #1684 in a followup PR. Those tests are manually building the expression AST, but it might be easier to maintain them if we parsed the query.

[codecov]

Codecov Report

❌ Patch coverage is 88.46154% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.94%. Comparing base (32a6fbb) to head (44e38cb).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1722      +/-   ##
==========================================
- Coverage   84.08%   83.94%   -0.14%     
==========================================
  Files         469      468       -1     
  Lines      137651   135954    -1697     
==========================================
- Hits       115746   114129    -1617     
+ Misses      21371    21291      -80     
  Partials      534      534              

Components	Coverage Δ
otap-dataflow	85.14% <ø> (-0.21%)	⬇️
query_abstraction	80.61% <ø> (ø)
query_engine	90.38% <88.46%> (-0.02%)	⬇️
syslog_cef_receivers	∅ <ø> (∅)
otel-arrow-go	53.50% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[drewrelmas]

@albertlockett One FYI note about the usage of the Conditional expressions for assignments, single statement executions could use already supported iif and case. Ignoring the double assignment, a sample statement:

if (severity_text == "ERROR") { 
    extend attributes["important"] = "very" // ignoring | extend attributes["triggers_alarm"] = "true"
} else if (severity_text == "WARN") {
    extend attributes["important"] = "somewhat"
} else if (severity_text == "INFO") {
    extend attributes["important"] = "rarely"
} else {
    extend attributes["important"] = "no"
}

could also be written as:

logs
| extend attributes["important"] = case(severity_text == "ERROR", "very", severity_text == "WARN", "somewhat", severity_text == "INFO", "rarely", "no"

That said, I think the actual important part of this PR here is actually executing multiple statements in a branch (to get the triggers_alarm attribute) and I recall your intention potentially being routing between nodes - that part cannot be represented easily in pure KQL.

@CodeBlanch what other significant departures from pure KQL do we currently have? Off the top of my head, I recall our map accessing is a bit different.

I wonder if we can have a superset of the grammar in a separate .pest file that inherits the base for these significant divergences from KQL with user config specifying what 'flavor' of grammar to support? See load multiple grammars from the Pest docs. I think I do see the value in driving this direction in supported expression tree, I'm still musing on how mixed it should be in the parser... 🤔

[albertlockett]

Hey @drewrelmas thanks for taking a look!

@albertlockett One FYI note about the usage of the Conditional expressions for assignments, single statement executions could use already supported iif and case. Ignoring the double assignment, a sample statement:

if (severity_text == "ERROR") { 
    extend attributes["important"] = "very" // ignoring | extend attributes["triggers_alarm"] = "true"
} else if (severity_text == "WARN") {
    extend attributes["important"] = "somewhat"
} else if (severity_text == "INFO") {
    extend attributes["important"] = "rarely"
} else {
    extend attributes["important"] = "no"
}

could also be written as:

logs
| extend attributes["important"] = case(severity_text == "ERROR", "very", severity_text == "WARN", "somewhat", severity_text == "INFO", "rarely", "no"

That said, I think the actual important part of this PR here is actually executing multiple statements in a branch (to get the triggers_alarm attribute) and I recall your intention potentially being routing between nodes - that part cannot be represented easily in pure KQL.

Yeah exactly. We'd like to add some additional conditional capabilities beyond just assignment, and routing would be part of that. I can imagine us wanting to do something like:

logs |
if (severity_text == "ERROR") { 
    extend attributes["triggers_alarm"] = "true" | route_to "high_severity_out_port"
} else {
    route_to "low_severity_out_port"
}

@CodeBlanch what other significant departures from pure KQL do we currently have? Off the top of my head, I recall our map accessing is a bit different.

I wonder if we can have a superset of the grammar in a separate .pest file that inherits the base for these significant divergences from KQL with user config specifying what 'flavor' of grammar to support? See load multiple grammars from the Pest docs. I think I do see the value in driving this direction in supported expression tree, I'm still musing on how mixed it should be in the parser... 🤔

Thanks for sharing that! I didn't know about the multiple grammars, but I think a that's exactly what we'll need: a base grammar, and parsers that inherit the base, plus various extensions.

I created an issue to track this work #1728 and I'll spend some time today investigating how to make this work. In terms of getting this PR merged, I'll leave it up to you whether you're comfortable merging as is, or whether it'd be better to implement a solution for #1728 first and rebase these changes on top of that.

I foresee a few other divergences from standard KQL we'd like to support in the columnar query engine. These include operators route_to, fork and possibly drop. From the parser's perspective, these would all be variants of tabular_expression, like if_else_expression.

[lquerel]

LGTM

[albertlockett]

@drewrelmas wrt to:

I wonder if we can have a superset of the grammar in a separate .pest file that inherits the base for these significant divergences from KQL with user config specifying what 'flavor' of grammar to support? See load multiple grammars from the Pest docs. I think I do see the value in driving this direction in supported expression tree, I'm still musing on how mixed it should be in the parser... 🤔

I added a solution for this in #1734 Can you (and maybe @CodeBlanch) take a look and let me know what you think?

The changes there will obviously conflict with the changes in this PR, but I'm open to having them merged in either order.