Detect a non-SARGable function or conversion on an unaliased table-variable column

[erikdarlingdata]

Problem

Rule 12 (Non-SARGable Predicate) reads a comparison's ScalarString to decide which side a function or an implicit conversion sits on. It calls this a column only when ColumnReferenceRegex finds a dotted name, such as [table].[col].

An aliased table-variable column renders dotted, through the alias, so that case already worked: SELECT v.X FROM @tv AS v WHERE ABS(v.X) = 1 gives abs(@tv.[X] as [v].[X])=(1).

An unaliased table-variable column renders as a bare bracketed name with no dotted qualifier at all: SELECT X FROM @tv WHERE ABS(X) = 1 gives abs([X])=(1). I confirmed this on SQL Server 2016, 2017, 2019, 2022 and 2025. No version renders [@tv].[col], so ColumnReferenceRegex can never match it, and rule 12 never flags the function or the conversion. Rule 11 (Scan With Predicate) then fires the generic residual-predicate warning instead, which does not name the fix.

Change

src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs:

• Added IsTableVariable(PlanNode node), a helper the scan's Object Table="[@tv]" attribute test already used inline in CheckForTableVariables. CheckForTableVariables now calls it too, instead of repeating the check.
• Added IsColumnReference(string text, bool isTableVariableScan). It checks ColumnReferenceRegex first, exactly as before. When that fails and isTableVariableScan is true, it also reads a bare bracketed name as a column. It excludes 3 shapes: a parameter or a variable ([@p1]), an optimizer-generated expression ([Expr1003]), and the name of a function call (followed by (). It reuses BracketedNameRegex, the same regex ReadsAnotherInput already uses to walk names in a ScalarString.
• ConvertImplicitWrapsColumn and IsFunctionOnColumnSide now call IsColumnReference instead of ColumnReferenceRegex directly, each with a new isTableVariableScan parameter that defaults to false.
• DetectNonSargablePattern takes the same flag, also defaulted to false, and passes it to both.
• DetectNonSargablePredicate(node) computes IsTableVariable(node) and passes the real flag. This is the only caller that does not use the default, so every other scan keeps today's behavior exactly.

src/PlanViewer.Core/Services/PlanAnalyzer.cs:

• Added ExpressionColumnRegex (^\[Expr\d+\]$) for the expression-name exclusion above.
• Corrected the comment on ColumnReferenceRegex, which claimed an unaliased table-variable column renders as [@tv].[col]. It does not, on any of the five SQL Server versions I checked. The comment now gives the real aliased and unaliased shapes.
• Corrected the same wrong claim in the doc comment on ConvertImplicitWrapsColumn.

Rule 11 already skips its own warning when rule 12 has already flagged the predicate (GetNonSargableReason(node, cfg) == null in its condition). Once rule 12 recognizes the bare column, "Scan With Predicate" stops firing on these scans. This needed no separate change.

A second commit corrects the history in the ColumnReferenceRegex comment. The first commit said that an aliased table-variable column failed the old pattern. It did not, because the alias part [v].[X] has no @, so the old pattern matched it.

Known and accepted, not fixed here

Rule 12 already reads an outer reference as a column: on a correlated inner scan, u.X = ABS(t.a) renders #u.[X] as [u].[X]=abs(#t.[a] as [t].[a]) and gets flagged, because #t.[a] as [t].[a] is dotted. This change extends that same reading to a bare outer reference from another unaliased table variable. #564 covers both cases, with plans captured for each rendering.

Tests

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

Corrected, using real shapes instead of the fictional [@tv].[col]:

• Rule12f_NonSargable_AliasedTableVariableColumnConversion_IsFlagged
• Rule12f_NonSargable_UnaliasedTableVariableColumnConversion_IsFlaggedWithTableVariableFlag
• Rule12f_NonSargable_UnaliasedTableVariableColumnConversion_NotFlaggedWithoutTableVariableFlag
• Rule12f_NonSargable_UnaliasedTableVariableParameterSideConversion_IsNotFlagged

String-level tests on DetectNonSargablePattern. With the flag on, abs([X])=(1) and CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n] are flagged. Still with the flag on, [X]=abs([@i]), abs([Expr1003])=(1), and [X]=upper('[Y]') are not flagged. With the flag off, abs([X])=(1) is not flagged.

Fixture tests, using the 3 new .sqlplan files:

• table_variable_unaliased_function_plan.sqlplan and table_variable_implicit_conversion_plan.sqlplan now get "Non-SARGable Predicate" and no "Scan With Predicate".
• table_variable_aliased_function_plan.sqlplan's warnings do not change.

All 96 tests in PlanAnalyzerTests pass (85 before this change, 11 added).

Mutation results

Four mutations, each restored before the next, each rebuilt with --no-incremental:

• The table-variable gate removed (IsColumnReference always reads a bare name as a column): failed Rule12f_NonSargable_UnaliasedTableVariableColumnConversion_NotFlaggedWithoutTableVariableFlag and Rule12h_NonSargable_BareColumnFunction_NotFlaggedWithoutTableVariableFlag.
• [Expr...] not excluded: failed Rule12h_NonSargable_FunctionOnExpressionColumn_NotFlaggedEvenWithTableVariableFlag.
• [@...] not excluded: failed Rule12f_NonSargable_UnaliasedTableVariableParameterSideConversion_IsNotFlagged and Rule12h_NonSargable_FunctionOnParameterSide_NotFlaggedEvenWithTableVariableFlag.
• The flag not passed from DetectNonSargablePattern into ConvertImplicitWrapsColumn: failed Rule12h_NonSargable_BareColumnImplicitConversion_IsFlaggedWithTableVariableFlag and Rule12h_NonSargable_ImplicitConversionPlan_GetsNonSargableNotScanWithPredicate.

Baseline diff

Regenerated both baselines from empty.

WarningBaseline.txt gained exactly 3 new sections, one per new fixture, and changed nothing else. The 2 function-plan fixtures each show a new "Non-SARGable Predicate" line and no "Scan With Predicate" line.

ComparisonBaseline.txt gained a self-comparison section for each of the 3 new fixtures (each comparing the fixture to itself). This characterization test also pairs each fixture with its alphabetical neighbor. Inserting 3 new file names shifted a few neighbor pairings. For example, spill_plan.sqlplan vs table_variable_plan.sqlplan became spill_plan.sqlplan vs table_variable_aliased_function_plan.sqlplan. This is mechanical churn from the new file names, not a change to any existing fixture's own analysis. No existing fixture's self-comparison changed.

Full suite

I merged both fix/562-expensive-operator-floor and this branch into a throwaway branch off origin/dev. I 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 #561

🤖 Generated with Claude Code

https://claude.ai/code/session_017ZVrq8tpA2DBPEFEqahFK6

[claude]

Reviewed the diff (PlanAnalyzer.Detection.cs, PlanAnalyzer.cs, and the new test fixtures/baselines). This fixes a real gap: an unaliased table-variable column renders as a bare [col] in a ScalarString with no dotted qualifier, so ColumnReferenceRegex could never tell it apart from a parameter/expression, and non-SARGable warnings silently vanished for SELECT X FROM @tv WHERE ABS(X) = 1-shaped predicates.

Things that check out:

• IsTableVariable(node) is a straight extraction of the pre-existing ObjectName.StartsWith("@") check used by CheckForTableVariables — no behavior change there, just dedup.
• isTableVariableScan is threaded through the single call path (DetectNonSargablePredicate → DetectNonSargablePattern → ConvertImplicitWrapsColumn/IsFunctionOnColumnSide → IsColumnReference), and every new parameter defaults to false, so no other caller's behavior shifts.
• IsColumnReference's heuristic (bare name → column, unless it's [@…], [ExprNNN], a function-call name, or inside a string literal) is exercised by good targeted unit tests plus two new plan fixtures (aliased vs. unaliased ABS, and an unaliased CONVERT_IMPLICIT case), and the aliased case is asserted unchanged.
• ExtractBalancedArguments's output for CONVERT_IMPLICIT(nvarchar(20),[S],0) never puts a bracket around the type portion, so the new bare-name matching in IsColumnReference can't misfire on the target-type argument.
• No T-SQL generation, versioned files, or SSMS/Blazor linked-file concerns are touched by this PR.

No correctness, security, or convention issues found. Nit-level only: the "bare name = column" heuristic would misread a literally bracket-quoted column named like [@weird] as a parameter, but that mirrors the existing [@p]-is-never-a-column assumption used everywhere else in this file, so it's not a regression introduced here.