Read only the function's own comparison in the non-SARGable check - #557
Conversation
IsFunctionOnColumnSide split the whole predicate at its first comparison operator. In a compound predicate that put every later comparison, column and all, on the function's side, so [t].[A]=CONVERT_IMPLICIT(int,[@1],0) AND [t].[B]=CONVERT(tinyint,[@2],0) reported a conversion of a parameter as a function on a column. The same mistake fired on the everyday date range [d]>=dateadd(day,(-7),getdate()) AND [d]<getdate(). The side check now reads only the comparison that holds the function: the text between the nearest AND/OR on either side, split at every depth so parenthesized OR groups come apart too, and never inside a string literal or a bracketed name. Two more false positives from the same family, fixed with it: - ISNULL/COALESCE was flagged wherever it appeared. It now gets the same side check, so ISNULL(@p, 0) on the parameter side is quiet while col = ISNULL(@p, col) is still caught, and a parameter-side ISNULL no longer steals the message from a real function on a column. - LIKE was not recognized as a comparison, so [col] like upper([@p]) fell to the assume-the-worst default and was flagged. DetectNonSargablePattern is split out as an internal seam so predicate shapes can be tested as raw strings, the way #436 tested ConvertImplicitWrapsColumn. The reporter's plan is a new fixture. Both golden masters change only by that fixture: the warning digest gains its section (a true Scan With Predicate, no Non-SARGable), and the comparison digest gains its self and next pairings plus the one predecessor pairing that now points at it. Fixes #556 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E9G1hWs6Nk4h4U2gn7TSy5
|
Reviewed the diff (PlanAnalyzer.Detection.cs, PlanAnalyzer.cs, and the new tests/fixture). The fix is sound. Traced through the new No new warnings, no T-SQL involved, nothing here touches versioned files or anything that needs a PlanViewer.Web linked-include. Test coverage matches the fix precisely — the raw-string tests exercise each regex boundary the PR description calls out, and the new fixture/baseline-digest diffs are the expected mechanical fallout of adding one plan alphabetically. Nothing to flag. |
1 participant
Summary
Fixes #556. Rule 12 (Non-SARGable Predicate) reported a function on a column when the function wrapped a parameter. This happened when the predicate had more than one comparison joined by AND or OR.
The side check split the whole predicate at its first comparison operator. In a compound predicate, that put the next comparison and its column on the same side as the function. The check now reads only the comparison that contains the function. It splits the predicate at every AND and OR, also inside parentheses. It does not split inside a string literal or a bracketed name.
The same mistake also flagged a common date range:
[d]>=dateadd(day,(-7),getdate()) AND [d]<getdate(). This shape is probably the most frequent trigger in real plans.Also fixed
Two related false positives in the same rule:
col = ISNULL(@p, 0)is no longer flagged.col = ISNULL(@p, col)is still flagged, because the column is inside the function.[col] like upper([@p])fell back to the worst case and was flagged. LIKE is now a comparison operator for this check.Tests
DetectNonSargablePatternis now internal so that these shapes can be tested without a plan file. [FEATURE] Distinguish between severity of Warnings #436 did the same forConvertImplicitWrapsColumn.planview analyzeon the reporter's plan now reports one warning, Scan With Predicate.🤖 Generated with Claude Code
https://claude.ai/code/session_01E9G1hWs6Nk4h4U2gn7TSy5