Require 1,000ms statement elapsed before rule 35 fires

[erikdarlingdata]

Problem

Rule 35 (Expensive Operator) warns when one operator takes at least 20% of a statement's elapsed time. In a statement that finishes in under a second, one or two operators always take most of the (tiny) elapsed time. There is almost nothing else to divide the time among, so the 20% share points at nothing (#562).

Change

Rule35_ExpensiveOperator in src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs now requires the statement's elapsed time to be at least 1,000ms before it runs at all. The floor is a named constant, Rule35MinStatementElapsedMs, placed just above the rule. Rule 19 uses the same floor to fire on compile CPU (stmt.CompileCPUMs >= 1000). Rule 4 uses it to call UDF time Critical (stmt.QueryUdfElapsedTimeMs >= 1000). A different value changes no fixture: every fixture with this warning ran for 3ms or less, or for 1,000ms or more.

Tests

Added to tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs:

• Rule35_ExpensiveOperator_NotFiredWhenStatementUnderOneSecond: multi_index_update_plan.sqlplan (statement elapsed 1ms) gets no "Expensive Operator" warning.
• Rule35_ExpensiveOperator_FiredWhenStatementAtOneSecond: parallel_row_over_batch_plan.sqlplan (statement elapsed exactly 1,000ms) still gets its Critical "Hash Match" warning. This pins the floor as >=, not >.
• Rule35_ExpensiveOperator_NotFiredJustUnderOneSecondFloor and Rule35_ExpensiveOperator_FiredAtOneSecondFloorExactly: the same plan, with the statement's elapsed time forced to 999ms, then 1,000ms. This checks both sides of the line on one plan.

The last two tests needed a way to override a loaded plan's elapsed time before analysis. I added PlanTestHelper.LoadAndAnalyzeWithElapsedTimeMs(planFileName, elapsedTimeMs) next to the existing LoadAndAnalyzeWithConfig helper.

All 89 tests in PlanAnalyzerTests pass.

Mutation results

Two mutations, each restored before the next:

• Floor removed (back to stmt.QueryTimeStats.ElapsedTimeMs > 0): failed Rule35_ExpensiveOperator_NotFiredWhenStatementUnderOneSecond and Rule35_ExpensiveOperator_NotFiredJustUnderOneSecondFloor, as expected.
• > in place of >=: failed Rule35_ExpensiveOperator_FiredWhenStatementAtOneSecond and Rule35_ExpensiveOperator_FiredAtOneSecondFloorExactly, as expected.

Each mutation was built with --no-incremental after restoring the file, to rule out a stale DLL.

Baseline diff

Regenerated WarningBaseline.txt from empty. The diff removes exactly 5 lines, all "Expensive Operator", across the 3 fixtures the brief predicted:

• multi_index_update_plan.sqlplan: 1 line (Critical, Clustered Index Update, 1ms, 100%).
• join_or_expression_plan.sqlplan: 2 lines (Nested Loops and Sort, 1ms, 33.3% each).
• join_or_mixed_parameter_plan.sqlplan: 2 lines (Nested Loops and Sort, 1ms, 33.3% each).

No other fixture changed.

ComparisonBaseline.txt was also regenerated: deleted, then the characterization test run twice, because it fails on purpose on the recording run. It came back byte-identical in content to the committed version, so it is not part of this diff.

Full suite

I merged this branch and fix/561-table-variable-bare-columns into a throwaway branch off origin/dev, and ran the full PlanViewer.Core.Tests project once there, including the headless UI tests. Totals: 758 total, 757 passed, 0 failed, 1 skipped, in 1m 11s. The 1 skip is a pre-existing, unrelated Windows/WAM contract pin (EntraInteractiveAuthTests).

Closes #562

🤖 Generated with Claude Code

https://claude.ai/code/session_017ZVrq8tpA2DBPEFEqahFK6

[claude]

Reviewed. Small, correctly-scoped fix — the >= threshold is pinned by tests on both sides (999ms/1000ms), the baseline diff matches exactly what the PR description predicts, and parallel_row_over_batch_plan.sqlplan's ElapsedTime="1000" confirms the boundary fixture is genuine rather than coincidental. No correctness, security, or convention issues found.