[Performance] EWMA SQL Query Plan Section

[fhibf]

Description

Describe the changes in this PR.

Related issues

Addresses AB#170732

Testing

Describe how this change was tested.

FHIR Team Checklist

• Update the title of the PR to be succinct and less than 65 characters
• Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
• Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
• Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
• Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
• When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
• CI is green before merge
• Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

The best way to fix this issue is to replace the generic catch (Exception exception) clause with one or more specific catch clauses that match the expected exception types thrown by the operations in the try block. Examine the methods used here:

• sqlSearchOptions.Expression.GetHashedUniqueExpressionIdentifier()
• _queryPlanSelector.GetRecommendedQueryPlanCacheSetting(hash)

Assuming these methods are unlikely to throw anything but programming-related or argument exceptions, the most precise approach is to catch exceptions such as ArgumentException, InvalidOperationException, and potentially NullReferenceException or any custom exception the application expects from those functions. In case the expected exceptions are not documented, it's safest (in absence of other project code) to catch ArgumentException and InvalidOperationException.

Only lines 276-280 should be updated. No imports or additional dependencies are required for these exception types.

---

[LTA-Thinking]

When the times comes the new files need to be moved to a folder with a proper name.

To fix the issue, replace the two separate operations: first checking presence of the key with scores.ContainsKey(metric) (line 34) and then accessing its value with scores[metric] (line 35), with a single call to scores.TryGetValue(metric, out var value). This allows both the test for key presence and for the value to be handled efficiently and more idiomatically. Specifically, within the test, use Assert.True(scores.TryGetValue(metric, out var value)); instead of Assert.True(scores.ContainsKey(metric)); and then check Assert.Null(value); instead of reading scores[metric] again. No new methods or imports are required, only a code change within the relevant lines of EwmaTests.cs.

---

To ensure that a NullReferenceException does not occur, the assignment resulting from the reflection call must be guarded by a null check. Specifically, after GetField().GetValue(ewma), the result should be checked before dereferencing. If the result is null, the test should fail, indicating the presence of an unexpected issue (e.g., field name/type mismatch, signature change, etc.). The appropriate fix is to add a check before using scores. If scores is null, an informative assertion should fail the test immediately, helping developers diagnose reflection issues. This check should be applied in every block where similar reflection usage occurs—namely, in all three tests (Ewma_Constructor_InitializesScoresToNull, Ewma_Update_UpdatesScoreCorrectly, and any other present if applicable).

---

To fix the problem, we need to ensure that the scores variable is not null before dereferencing it; this means adding a check after the reflection/casting code. The single best fix is to add an assertion that scores != null before accessing scores["A"]. This way, if the reflection fails, the test fails with a clear message rather than throwing a NullReferenceException. Apply this change only to the region where scores is accessed without null checks (lines 54-55 in Ewma_Update_UpdatesScoreCorrectly). No new imports or definitions are required; just an extra Assert.NotNull(scores); before the offending dereference.

[fhibf]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[LTA-Thinking]

What does this return if this is reached before any scores are set? For example if GetBestMetric is called enough times so that _interactions is greater than _minInteractionsForDecision, but Update hasn't been called yet.