diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5c3a52b0..d2b044f6 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -3,21 +3,13 @@ name: CI
on:
push:
branches: [main]
+ # No paths-ignore here, deliberately. A required status check has to REPORT on every
+ # PR, and a workflow skipped by a path filter reports nothing at all -- GitHub then
+ # shows the check as forever pending and the PR can never merge. A docs-only PR would
+ # deadlock. The filtering moved into the job below, where the steps are skipped but the
+ # job still finishes and reports success. Same shape PerformanceMonitor's build.yml uses.
pull_request:
branches: [main, dev]
- paths-ignore:
- - '**.md'
- - 'LICENSE'
- - '.gitattributes'
- - '.gitignore'
- - 'CITATION.cff'
- - 'llms.txt'
- - '.github/ISSUE_TEMPLATE/**'
- - 'docs/**'
- - 'screenshots/**'
- - 'server/**'
- - 'src/PlanViewer.Ssms/**'
- - 'src/PlanViewer.Ssms.Installer/**'
permissions:
contents: read
@@ -29,7 +21,31 @@ jobs:
steps:
- uses: actions/checkout@v7
+ # What used to be the workflow's paths-ignore list. Anything NOT matched here is
+ # code, and only then is there anything to build.
+ - name: Classify changed paths
+ uses: dorny/paths-filter@v4
+ id: filter
+ with:
+ # Positive list, not negations. paths-filter ORs the patterns in a filter, so a
+ # stack of '!' patterns matches whenever a file fails ANY one of them, which for
+ # a docs-only change is always true. Listing what IS code keeps the OR honest.
+ # PlanViewer.Ssms and PlanViewer.Ssms.Installer stay out: they are not in the
+ # solution and ci.yml never built them.
+ filters: |
+ code:
+ - 'src/PlanViewer.App/**'
+ - 'src/PlanViewer.Cli/**'
+ - 'src/PlanViewer.Core/**'
+ - 'src/PlanViewer.Web/**'
+ - 'src/Directory.Build.props'
+ - 'tests/**'
+ - 'PlanViewer.sln'
+ - 'global.json'
+ - '.github/workflows/ci.yml'
+
- name: Setup .NET 10.0
+ if: steps.filter.outputs.code == 'true'
uses: actions/setup-dotnet@v6
with:
dotnet-version: 10.0.x
@@ -37,13 +53,17 @@ jobs:
cache-dependency-path: '**/*.csproj'
- name: Install WASM workload
+ if: steps.filter.outputs.code == 'true'
run: dotnet workload install wasm-tools
- name: Restore solution
+ if: steps.filter.outputs.code == 'true'
run: dotnet restore PlanViewer.sln
- name: Build solution
+ if: steps.filter.outputs.code == 'true'
run: dotnet build PlanViewer.sln -c Release --no-restore
- name: Run tests
+ if: steps.filter.outputs.code == 'true'
run: dotnet test tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj -c Release --no-build --verbosity normal -- --hangdump --hangdump-timeout 5m --hangdump-type none
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 92ff73f7..30d517fe 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -130,7 +130,7 @@ jobs:
path: publish/win-x64/
- name: Sign Windows build
- uses: signpath/github-action-submit-signing-request@v2
+ uses: signpath/github-action-submit-signing-request@v3
with:
api-token: '${{ secrets.SIGNPATH_API_TOKEN }}'
organization-id: '7969f8b6-d946-4a74-9bac-a55856d8b8e0'
diff --git a/CITATION.cff b/CITATION.cff
index 6e18b56e..72cc0fe1 100644
--- a/CITATION.cff
+++ b/CITATION.cff
@@ -9,8 +9,8 @@ authors:
website: "https://erikdarling.com"
repository-code: "https://github.com/erikdarlingdata/PerformanceStudio"
license: MIT
-version: "1.26.0"
-date-released: "2026-09-16"
+version: "1.27.0"
+date-released: "2026-09-24"
keywords:
- sql-server
- execution-plan
diff --git a/README.md b/README.md
index d93af5a0..847bbeb1 100644
--- a/README.md
+++ b/README.md
@@ -38,19 +38,19 @@ Navigate stored procedures and batches with multiple statements. Click any state
### Operator Tooltip and Properties
-Hover over any operator for a detailed tooltip with costs, rows, I/O, timing, parallelism, and warnings. Click to open the full properties panel with per-thread timing, predicates, and more.
+Hover any operator for a grouped tooltip with costs, rows, timing, and parallelism. Click for the full properties panel, which has a filter box and folds per-thread stats into one expander per section instead of hundreds of rows.
### Advice for Humans
-One-click text report with server context, warnings, wait stats, and expensive operators — ready to read or share.
+Severity-scored cards for each statement: the warnings that fired, wait stats, memory grant, and missing indexes. Copy to clipboard gives you the plain-text version to paste into a ticket.
### Plan Comparison
-Side-by-side comparison of two plans showing cost, runtime, I/O, memory, and wait stat differences.
+A real metric diff. Each statement is scored as regressed or improved, every metric carries its own delta chip, and the direction that counts as better is declared per metric rather than guessed from the sign.
diff --git a/screenshots/Actual Execution Plan With Warning Tool Tip.png b/screenshots/Actual Execution Plan With Warning Tool Tip.png
index ba4b4596..65ccc48e 100644
Binary files a/screenshots/Actual Execution Plan With Warning Tool Tip.png and b/screenshots/Actual Execution Plan With Warning Tool Tip.png differ
diff --git a/screenshots/Actual Execution Plan.png b/screenshots/Actual Execution Plan.png
index 21d01f30..fe7dd620 100644
Binary files a/screenshots/Actual Execution Plan.png and b/screenshots/Actual Execution Plan.png differ
diff --git a/screenshots/Advice For Humans.png b/screenshots/Advice For Humans.png
index 4fefa7ce..bb9937c9 100644
Binary files a/screenshots/Advice For Humans.png and b/screenshots/Advice For Humans.png differ
diff --git a/screenshots/Navigate Stored Procedure Statements and Plans.png b/screenshots/Navigate Stored Procedure Statements and Plans.png
index effab0cd..b7661215 100644
Binary files a/screenshots/Navigate Stored Procedure Statements and Plans.png and b/screenshots/Navigate Stored Procedure Statements and Plans.png differ
diff --git a/screenshots/Operator Properties.png b/screenshots/Operator Properties.png
index 523aedf1..9ce33e57 100644
Binary files a/screenshots/Operator Properties.png and b/screenshots/Operator Properties.png differ
diff --git a/screenshots/Plan Comparison.png b/screenshots/Plan Comparison.png
index 777abcc9..08f6686c 100644
Binary files a/screenshots/Plan Comparison.png and b/screenshots/Plan Comparison.png differ
diff --git a/screenshots/Query Editor.png b/screenshots/Query Editor.png
index 7f2beffa..18e863b6 100644
Binary files a/screenshots/Query Editor.png and b/screenshots/Query Editor.png differ
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
index 224e9118..f07613cb 100644
--- a/src/Directory.Build.props
+++ b/src/Directory.Build.props
@@ -15,7 +15,7 @@
Tests and server/ projects are outside src/ and are unaffected.
-->
- 1.26.0
+ 1.27.0Erik DarlingDarling Data LLCPerformance Studio
diff --git a/src/PlanViewer.App/App.axaml b/src/PlanViewer.App/App.axaml
index 03f3a9c2..d6c41328 100644
--- a/src/PlanViewer.App/App.axaml
+++ b/src/PlanViewer.App/App.axaml
@@ -43,11 +43,11 @@
-
+
diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs b/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs
index e772bb50..5237aacb 100644
--- a/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs
+++ b/src/PlanViewer.App/Controls/PlanViewerControl.Schema.cs
@@ -109,10 +109,24 @@ private void ShowSchemaResult(string title, string content)
Padding = new Thickness(4)
};
- // SQL syntax highlighting
- var registryOptions = new TextMateSharp.Grammars.RegistryOptions(TextMateSharp.Grammars.ThemeName.DarkPlus);
- var tm = editor.InstallTextMate(registryOptions);
- tm.SetGrammar(registryOptions.GetScopeByLanguageId("sql"));
+ /* SQL syntax highlighting, on the query editor's own lifecycle: installed on attach,
+ disposed on detach (#546). An installation owns a tokenization model with its own
+ thread, and a thread roots itself against GC — so installing once and never disposing
+ leaked a live thread for every schema tab ever closed. Detach also fires on plain tab
+ switches, which is why attach re-installs. */
+ TextMate.Installation? tm = null;
+ editor.AttachedToVisualTree += (_, _) =>
+ {
+ if (tm != null) return;
+ var registryOptions = new TextMateSharp.Grammars.RegistryOptions(TextMateSharp.Grammars.ThemeName.DarkPlus);
+ tm = editor.InstallTextMate(registryOptions);
+ tm.SetGrammar(registryOptions.GetScopeByLanguageId("sql"));
+ };
+ editor.DetachedFromVisualTree += (_, _) =>
+ {
+ tm?.Dispose();
+ tm = null;
+ };
// Context menu
var copyItem = new MenuItem { Header = "Copy" };
diff --git a/src/PlanViewer.App/Controls/PlanViewerControl.axaml b/src/PlanViewer.App/Controls/PlanViewerControl.axaml
index 8df78fab..f7b1b85c 100644
--- a/src/PlanViewer.App/Controls/PlanViewerControl.axaml
+++ b/src/PlanViewer.App/Controls/PlanViewerControl.axaml
@@ -525,7 +525,7 @@
Background="{DynamicResource BackgroundBrush}"
BorderBrush="{DynamicResource BorderBrush}" BorderThickness="0,0,0,1">
- /// Update the connection UI to reflect an active connection (used when connection is inherited).
+ /// Update the connection UI to reflect an active connection (used when connection is
+ /// inherited). Label and button only — the session-hosted viewers that call this hide the
+ /// whole connection toolbar, so there is no picker to feed. A standalone tab with a live
+ /// toolbar wants instead.
///
public void SetConnectionStatus(string serverName, string? database)
{
@@ -340,6 +342,27 @@ public void SetConnectionStatus(string serverName, string? database)
_planSelectedDatabase = database;
}
+ ///
+ /// Takes over a connection the ConnectionDialog just validated, for a standalone tab whose
+ /// toolbar is visible: paints the status AND fills, enables and pre-selects the database
+ /// picker, with set so changing the picker actually switches
+ /// . Status alone left a green label over a disabled, empty
+ /// picker — the same lie the connect handlers used to tell (#540 follow-up).
+ ///
+ /// Call first: the picker's SelectionChanged
+ /// rebuilds the connection string through the credential service.
+ ///
+ public void AdoptConnection(ServerConnection connection, string? database,
+ IReadOnlyList databases)
+ {
+ _planConnection = connection;
+ SetConnectionStatus(connection.ServerName, database);
+
+ PlanDatabaseBox.ItemsSource = databases;
+ PlanDatabaseBox.IsEnabled = true;
+ SelectPlanDatabase();
+ }
+
// Events for MainWindow to wire up advice/repro actions
public event EventHandler? HumanAdviceRequested;
public event EventHandler? RobotAdviceRequested;
@@ -591,39 +614,34 @@ private async void PlanConnect_Click(object? sender, RoutedEventArgs e)
PlanServerLabel.Foreground = FindBrushResource("SuccessBrush");
PlanConnectButton.Content = AppIcons.MakeContent(AppIcons.Connect, "Reconnect");
- // Populate database dropdown
- try
- {
- var connStr = _planConnection.GetConnectionString(_planCredentialService, "master");
- await using var conn = new SqlConnection(connStr);
- await conn.OpenAsync();
-
- var databases = new List();
- using var cmd = new SqlCommand(
- "SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; SELECT name FROM sys.databases WHERE state_desc = 'ONLINE' ORDER BY name", conn);
- using var reader = await cmd.ExecuteReaderAsync();
- while (await reader.ReadAsync())
- databases.Add(reader.GetString(0));
+ /* The dialog only closes with true after it opened this connection and enumerated
+ these databases — through the database the user named, which is the one some
+ logins (Azure SQL DB, JIT access) can open when master is off limits. Asking
+ again here through a second, hardcoded-master connection was a wasted round trip
+ whose swallowed failure left a green toolbar over a dead database picker. Same
+ hand-over QuerySessionControl's connect block takes. */
+ PlanDatabaseBox.ItemsSource = dialog.ResultDatabases;
+ PlanDatabaseBox.IsEnabled = true;
+ SelectPlanDatabase();
+ }
- PlanDatabaseBox.ItemsSource = databases;
- PlanDatabaseBox.IsEnabled = true;
+ ///
+ /// Points the picker at when the list holds it. The
+ /// selection this raises recomputes the same ConnectionString the caller already set, which
+ /// is idempotent on purpose — the handler is the one place the string is derived.
+ ///
+ private void SelectPlanDatabase()
+ {
+ if (_planSelectedDatabase == null) return;
- if (_planSelectedDatabase != null)
+ for (int i = 0; i < PlanDatabaseBox.Items.Count; i++)
+ {
+ if (PlanDatabaseBox.Items[i]?.ToString() == _planSelectedDatabase)
{
- for (int i = 0; i < PlanDatabaseBox.Items.Count; i++)
- {
- if (PlanDatabaseBox.Items[i]?.ToString() == _planSelectedDatabase)
- {
- PlanDatabaseBox.SelectedIndex = i;
- break;
- }
- }
+ PlanDatabaseBox.SelectedIndex = i;
+ break;
}
}
- catch
- {
- PlanDatabaseBox.IsEnabled = false;
- }
}
private void PlanDatabase_SelectionChanged(object? sender, SelectionChangedEventArgs e)
diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs
index 5d379d1a..4512eba6 100644
--- a/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs
+++ b/src/PlanViewer.App/Controls/QuerySessionControl.Connection.cs
@@ -57,7 +57,22 @@ the rest of it has to live. */
ServerLabel.Foreground = Token("SuccessBrush", Brushes.LimeGreen);
ConnectButton.Content = Helpers.AppIcons.MakeContent(Helpers.AppIcons.Connect, "Reconnect");
- await PopulateDatabases();
+ /* Connecting is the one way a fresh session stops being empty without a keystroke
+ or a document, so neither of the overlay's other triggers will fire — without
+ this, the "Get started" panel keeps covering the editor and offering "Connect to
+ a server" on a session that just did (#540). Before the awaits below, so the
+ editor appears the moment the dialog closes rather than after three round trips
+ to the server. */
+ RefreshEmptyState();
+
+ /* The dialog only closes with true after it opened this connection and enumerated
+ these databases — through the database the user named, which is the one some
+ logins (Azure SQL DB, JIT access) can open when master is off limits. Asking
+ again here through a second, hardcoded-master connection was a wasted round trip
+ whose swallowed failure left a green toolbar over a dead database picker. */
+ DatabaseBox.ItemsSource = dialog.ResultDatabases;
+ DatabaseBox.IsEnabled = true;
+
await FetchServerMetadataAsync();
await FetchServerUtcOffset();
@@ -86,32 +101,6 @@ the rest of it has to live. */
}
}
- private async Task PopulateDatabases()
- {
- if (_serverConnection == null) return;
-
- try
- {
- var connStr = _serverConnection.GetConnectionString(_credentialService, "master");
- await using var conn = new SqlConnection(connStr);
- await conn.OpenAsync();
-
- var databases = new List();
- using var cmd = new SqlCommand(
- "SELECT name FROM sys.databases WHERE state_desc = 'ONLINE' ORDER BY name", conn);
- using var reader = await cmd.ExecuteReaderAsync();
- while (await reader.ReadAsync())
- databases.Add(reader.GetString(0));
-
- DatabaseBox.ItemsSource = databases;
- DatabaseBox.IsEnabled = true;
- }
- catch
- {
- DatabaseBox.IsEnabled = false;
- }
- }
-
private async void Database_SelectionChanged(object? sender, SelectionChangedEventArgs e)
{
if (_serverConnection == null || DatabaseBox.SelectedItem == null) return;
diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs b/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs
index e06279d6..7c89d207 100644
--- a/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs
+++ b/src/PlanViewer.App/Controls/QuerySessionControl.EmptyState.cs
@@ -21,17 +21,21 @@ public partial class QuerySessionControl : UserControl
///
/// Decides whether the editor's empty state is showing, and rebuilds it when it is.
///
- /// Shown only when this session holds nothing at all: no text, and no sub-tab beyond
- /// the Query Editor. Both halves matter — a session whose editor is empty because the user
- /// is reading the plan they just ran must not have an overlay waiting behind that plan.
+ /// Shown only when this session holds nothing at all: no text, no sub-tab beyond
+ /// the Query Editor, and no server connection. All three halves matter — a session whose
+ /// editor is empty because the user is reading the plan they just ran must not have an
+ /// overlay waiting behind that plan, and a session that just connected is in use even
+ /// though nothing has been typed yet: the editor IS the offer now, and a panel still
+ /// suggesting "Connect to a server" over it reads as the connection having failed (#540).
///
- /// Called from the editor's TextChanged and from the sub-tab watcher, so it re-decides
- /// in both directions: delete every character with no plan open and the panel comes back,
- /// which is the same state a fresh tab is in and deserves the same offer.
+ /// Called from the editor's TextChanged, from the sub-tab watcher, and from the
+ /// connect block, so it re-decides in both directions: delete every character with no plan
+ /// open and no connection and the panel comes back, which is the same state a fresh tab is
+ /// in and deserves the same offer.
///
private void RefreshEmptyState()
{
- var empty = QueryEditor.Text.Length == 0 && !HasDocuments;
+ var empty = QueryEditor.Text.Length == 0 && !HasDocuments && _serverConnection == null;
if (empty)
{
diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs
index 56f8746a..71bb358f 100644
--- a/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs
+++ b/src/PlanViewer.App/Controls/QuerySessionControl.Execution.cs
@@ -82,6 +82,8 @@ private async Task CaptureAndShowPlan(bool estimated, string? queryTextOverride
Height = 4,
Margin = new Avalonia.Thickness(0, 0, 0, 12)
};
+ // This overlay lives in tab content the user switches away from mid-capture.
+ Helpers.ProgressBarBehaviors.SetRestartOnReattach(progressBar, true);
/* #448: SelectableTextBlock and wrapping, because this label doubles as the place a query
failure is reported. A SQL error is the one string in this app a user most needs to copy
@@ -314,6 +316,8 @@ private async void GetActualPlan_Click(object? sender, RoutedEventArgs e)
Height = 4,
Margin = new Avalonia.Thickness(0, 0, 0, 12)
};
+ // This overlay lives in tab content the user switches away from mid-capture.
+ Helpers.ProgressBarBehaviors.SetRestartOnReattach(progressBar, true);
/* #448: see the note on the estimated-plan path — this label reports failures too. */
var statusLabel = new SelectableTextBlock
@@ -446,7 +450,10 @@ private Task ShowConfirmationDialog(string title, string message, string c
private Window GetParentWindow()
{
- var parent = this.VisualRoot;
+ /* GetTopLevel rather than VisualRoot because Avalonia 12 hosts a Window inside a
+ TopLevelHost, so the visual root is no longer the Window and casting it to one always
+ misses — silently, since VisualRoot still compiles and still returns something. */
+ var parent = TopLevel.GetTopLevel(this);
return parent as Window ?? throw new InvalidOperationException("No parent window");
}
}
diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs
index 2afe020e..f91351c1 100644
--- a/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs
+++ b/src/PlanViewer.App/Controls/QuerySessionControl.Schema.cs
@@ -95,9 +95,22 @@ private void AddSchemaTab(string label, string content, bool isSql)
if (isSql)
{
- var registryOptions = new RegistryOptions(ThemeName.DarkPlus);
- var tm = editor.InstallTextMate(registryOptions);
- tm.SetGrammar(registryOptions.GetScopeByLanguageId("sql"));
+ /* Same lifecycle as the query editor and the plan viewer's schema tabs (#546):
+ install on attach, dispose on detach. The installation's tokenization model runs
+ a thread that roots itself, so a tab closed without disposing leaked it. */
+ TextMate.Installation? tm = null;
+ editor.AttachedToVisualTree += (_, _) =>
+ {
+ if (tm != null) return;
+ var registryOptions = new RegistryOptions(ThemeName.DarkPlus);
+ tm = editor.InstallTextMate(registryOptions);
+ tm.SetGrammar(registryOptions.GetScopeByLanguageId("sql"));
+ };
+ editor.DetachedFromVisualTree += (_, _) =>
+ {
+ tm?.Dispose();
+ tm = null;
+ };
}
// Context menu for read-only schema tabs
diff --git a/src/PlanViewer.App/Controls/QuerySessionControl.axaml b/src/PlanViewer.App/Controls/QuerySessionControl.axaml
index 90818874..5673e1e8 100644
--- a/src/PlanViewer.App/Controls/QuerySessionControl.axaml
+++ b/src/PlanViewer.App/Controls/QuerySessionControl.axaml
@@ -338,11 +338,11 @@
Padding="4"/>
+ empty — nothing typed, no document open, no server connected — so a
+ new Query tab offers its primary actions instead of a blank wall. It
+ never traps typing: the editor keeps focus underneath, a click on the
+ background hands focus back to it, and the overlay leaves the moment
+ there is any text, any document, or a connection (RefreshEmptyState). -->
@@ -94,7 +95,7 @@
@@ -170,14 +171,14 @@
-
+
-
+
-
@@ -186,22 +187,22 @@
-
+
-
+
-
+
-
+
@@ -487,7 +488,8 @@
Background="#80000000" CornerRadius="0"
HorizontalAlignment="Stretch" VerticalAlignment="Stretch">
-
+
diff --git a/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml b/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml
index adda15b0..b59a90c1 100644
--- a/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml
+++ b/src/PlanViewer.App/Controls/QueryStoreHistoryControl.axaml
@@ -1,6 +1,7 @@
@@ -71,7 +72,8 @@
FontSize="12" Foreground="{DynamicResource ForegroundBrush}"/>
-
+
diff --git a/src/PlanViewer.App/Controls/QueryStoreOverviewControl.axaml b/src/PlanViewer.App/Controls/QueryStoreOverviewControl.axaml
index b5c24f24..2fbae420 100644
--- a/src/PlanViewer.App/Controls/QueryStoreOverviewControl.axaml
+++ b/src/PlanViewer.App/Controls/QueryStoreOverviewControl.axaml
@@ -1,6 +1,7 @@
diff --git a/src/PlanViewer.App/Controls/WaitStatsProfileControl.axaml b/src/PlanViewer.App/Controls/WaitStatsProfileControl.axaml
index d8368daf..45680e50 100644
--- a/src/PlanViewer.App/Controls/WaitStatsProfileControl.axaml
+++ b/src/PlanViewer.App/Controls/WaitStatsProfileControl.axaml
@@ -1,6 +1,7 @@
@@ -35,7 +36,8 @@
Background="#80000000" CornerRadius="0"
HorizontalAlignment="Stretch" VerticalAlignment="Stretch">
-
+
diff --git a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml
index d6c02969..684d339c 100644
--- a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml
+++ b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml
@@ -15,7 +15,7 @@
FontSize="12" Margin="0,0,0,4"/>
@@ -66,7 +66,7 @@
@@ -94,7 +94,7 @@
diff --git a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml.cs b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml.cs
index 9ee334f4..b60dcfdd 100644
--- a/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml.cs
+++ b/src/PlanViewer.App/Dialogs/ConnectionDialog.axaml.cs
@@ -23,6 +23,14 @@ public partial class ConnectionDialog : Window
public ServerConnection? ResultConnection { get; private set; }
public string? ResultDatabase { get; private set; }
+ ///
+ /// The databases the login could see when the winning connection opened, for the caller's
+ /// database picker. Handing these over is what lets callers skip re-enumerating through a
+ /// second connection to master — a round trip this dialog deliberately does not make, because
+ /// some logins (Azure SQL DB, JIT access) can only open the database they named.
+ ///
+ public IReadOnlyList ResultDatabases { get; private set; } = Array.Empty();
+
///
/// The database the calling session is already on, when the dialog is opened to reconnect.
/// It is pre-selected once the database list loads; it never connects on its own.
@@ -176,16 +184,17 @@ private async void TestConnection_Click(object? sender, RoutedEventArgs e)
///
/// Opens a connection with the current settings and fills the Database dropdown with the
/// databases the login can see. Shared by Test Connection and Connect so both take the same
- /// path. Reports progress and failures in StatusText; returns true when the connection opened.
+ /// path. Reports progress and failures in StatusText; returns the databases when the
+ /// connection opened, null when it did not.
///
- private async Task ConnectAndLoadDatabasesAsync()
+ private async Task?> ConnectAndLoadDatabasesAsync()
{
var serverName = ServerNameBox.Text?.Trim();
if (string.IsNullOrEmpty(serverName))
{
StatusText.Text = "Enter a server name";
StatusText.Foreground = StatusBrush("ErrorBrush", Avalonia.Media.Brushes.OrangeRed);
- return false;
+ return null;
}
// For Azure SQL DB / JIT access the login often can't open master, so connect
@@ -211,10 +220,13 @@ private async Task ConnectAndLoadDatabasesAsync()
await conn.OpenAsync();
// Fetch databases the login can see. On Azure SQL DB connected to a single user
- // database this returns master + that database, which is expected.
+ // database this returns master + that database, which is expected. Read
+ // uncommitted so the catalog scan cannot sit blocked behind an in-flight
+ // CREATE or RESTORE — carried over from the plan toolbar's enumeration, which
+ // this one replaced.
var databases = new List();
using var cmd = new SqlCommand(
- "SELECT name FROM sys.databases WHERE state_desc = 'ONLINE' ORDER BY name", conn);
+ "SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; SELECT name FROM sys.databases WHERE state_desc = 'ONLINE' ORDER BY name", conn);
using var reader = await cmd.ExecuteReaderAsync();
while (await reader.ReadAsync())
databases.Add(reader.GetString(0));
@@ -234,14 +246,14 @@ private async Task ConnectAndLoadDatabasesAsync()
StatusText.Text = $"Connected ({databases.Count} databases)";
StatusText.Foreground = StatusBrush("SuccessBrush", Avalonia.Media.Brushes.LimeGreen);
- return true;
+ return databases;
}
catch (Exception ex)
{
StatusText.Text = ex.Message;
StatusText.Foreground = StatusBrush("ErrorBrush", Avalonia.Media.Brushes.OrangeRed);
DatabaseBox.IsEnabled = false;
- return false;
+ return null;
}
finally
{
@@ -285,7 +297,8 @@ reading them again after the await could save a server that was never tested. */
// Single step: connect and enumerate databases here, so Test Connection is never a
// prerequisite. On failure the message stays in StatusText and the dialog stays open.
- if (!await ConnectAndLoadDatabasesAsync())
+ var databases = await ConnectAndLoadDatabasesAsync();
+ if (databases == null)
return;
// Cancel stays live while the connection opens, so the dialog may already be gone.
@@ -308,6 +321,7 @@ reading them again after the await could save a server that was never tested. */
ResultConnection = connection;
ResultDatabase = ResolveResultDatabase(typedDatabase);
+ ResultDatabases = databases;
Close(true);
}
diff --git a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs
index 6783c50d..bc9958b9 100644
--- a/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs
+++ b/src/PlanViewer.App/Dialogs/SettingsWindow.axaml.cs
@@ -375,7 +375,7 @@ private void EnsureIntegrationsLoaded()
/* The stored proxy password is deliberately not put into the TextBox. PasswordChar only
masks the glyph — the cleartext still sits in the visual and accessibility trees. The
- watermark says it is saved, and an empty box at save time means "keep what is there". */
+ placeholder says it is saved, and an empty box at save time means "keep what is there". */
var proxy = ProxySettings.Load();
_hasStoredProxyPassword = !string.IsNullOrEmpty(proxy.Password);
_proxyMode = proxy.Mode;
@@ -506,10 +506,10 @@ void OnProxyModeChanged(object? sender, RoutedEventArgs _)
return panel;
}
- private static TextBox CreateProxyInput(string text, string watermark) => new()
+ private static TextBox CreateProxyInput(string text, string placeholder) => new()
{
Text = text,
- Watermark = watermark,
+ PlaceholderText = placeholder,
FontSize = 13,
Height = 32,
Padding = new Thickness(6, 2)
@@ -582,7 +582,7 @@ private void ResetIntegrations(bool clearStoredPassword)
/* This is the only way to delete a stored proxy password, because an empty password box
always means "keep what is there" — otherwise anyone who edited the proxy address would
- have to retype the password. Clearing the flag too stops the watermark claiming a
+ have to retype the password. Clearing the flag too stops the placeholder claiming a
password is saved after the reset has taken it away. */
_clearStoredProxyPassword = _hasStoredProxyPassword;
_hasStoredProxyPassword = false;
diff --git a/src/PlanViewer.App/Helpers/DataGridBehaviors.cs b/src/PlanViewer.App/Helpers/DataGridBehaviors.cs
index cd3d9a8a..1f0445cc 100644
--- a/src/PlanViewer.App/Helpers/DataGridBehaviors.cs
+++ b/src/PlanViewer.App/Helpers/DataGridBehaviors.cs
@@ -35,7 +35,7 @@ public static void AttachCopyGuard(DataGrid grid, Func formatRo
{
// Mirror the built-in copy's exact gate: platform command modifier
// (Ctrl; Cmd on macOS), no Shift, no Alt.
- var commandModifiers = TopLevel.GetTopLevel(grid)?.PlatformSettings?.HotkeyConfiguration.CommandModifiers
+ var commandModifiers = grid.GetPlatformSettings()?.HotkeyConfiguration.CommandModifiers
?? KeyModifiers.Control;
if (e.Key is not (Key.C or Key.Insert)
|| !e.KeyModifiers.HasFlag(commandModifiers)
diff --git a/src/PlanViewer.App/Helpers/ProgressBarBehaviors.cs b/src/PlanViewer.App/Helpers/ProgressBarBehaviors.cs
new file mode 100644
index 00000000..ad10b80e
--- /dev/null
+++ b/src/PlanViewer.App/Helpers/ProgressBarBehaviors.cs
@@ -0,0 +1,65 @@
+using Avalonia;
+using Avalonia.Controls;
+using Avalonia.Threading;
+
+namespace PlanViewer.App.Helpers;
+
+///
+/// Restarts an indeterminate 's animation when it re-enters the
+/// visual tree.
+///
+/// Avalonia 12 completes a style-applied animation when its visual detaches — a handler
+/// its changelog does not document, sitting outside the PlaybackBehavior pause guard —
+/// and re-attaching re-applies the style without resurrecting the finished animation. Switch
+/// tabs while a fetch is running and its loading bar comes back frozen mid-track, indistinguishable
+/// from a hang. Avalonia 11 had no detach handling at all, so the bar just kept animating; this
+/// restores that behavior at the seven bars whose hosts can detach mid-run.
+///
+/// The restart works by deactivating and reactivating the :indeterminate style:
+/// flipping off and, one dispatcher hop later, back on
+/// makes the style system spawn a fresh animation instance. The hop is load-bearing — both flips
+/// in the same pass coalesce into no style change at all.
+///
+/// Verified against the live symptom, not assumed: before this, five window captures
+/// spanning a full animation cycle after a tab round-trip were pixel-identical in the bar's
+/// region; with it, the sweep resumes. The headless suite cannot see any of this (animations
+/// need frames), which is why the proof lives in a driven run rather than a test.
+///
+public static class ProgressBarBehaviors
+{
+ ///
+ /// Set true on any indeterminate bar whose host can detach while it runs — tab content,
+ /// overlays inside documents, detachable panels.
+ ///
+ public static readonly AttachedProperty RestartOnReattachProperty =
+ AvaloniaProperty.RegisterAttached(
+ "RestartOnReattach", typeof(ProgressBarBehaviors));
+
+ public static void SetRestartOnReattach(ProgressBar bar, bool value) =>
+ bar.SetValue(RestartOnReattachProperty, value);
+
+ public static bool GetRestartOnReattach(ProgressBar bar) =>
+ bar.GetValue(RestartOnReattachProperty);
+
+ static ProgressBarBehaviors()
+ {
+ RestartOnReattachProperty.Changed.AddClassHandler((bar, e) =>
+ {
+ if (e.NewValue is true)
+ bar.AttachedToVisualTree += Restart;
+ else
+ bar.AttachedToVisualTree -= Restart;
+ });
+ }
+
+ private static void Restart(object? sender, VisualTreeAttachmentEventArgs e)
+ {
+ /* An idle bar (Overview's LoadingBar between loads) has nothing to restart, and
+ flipping it would wrongly start one. */
+ if (sender is not ProgressBar bar || !bar.IsIndeterminate)
+ return;
+
+ bar.IsIndeterminate = false;
+ Dispatcher.UIThread.Post(() => bar.IsIndeterminate = true, DispatcherPriority.Loaded);
+ }
+}
diff --git a/src/PlanViewer.App/MainWindow.PlanViewer.cs b/src/PlanViewer.App/MainWindow.PlanViewer.cs
index 8f8b75fb..34faebec 100644
--- a/src/PlanViewer.App/MainWindow.PlanViewer.cs
+++ b/src/PlanViewer.App/MainWindow.PlanViewer.cs
@@ -475,6 +475,8 @@ emailed around. Name the target so "which server was that?" is answered
Height = 4,
Margin = new Avalonia.Thickness(0, 0, 0, 12)
};
+ // This loading tab detaches whenever the user switches top-level tabs mid-capture.
+ Helpers.ProgressBarBehaviors.SetRestartOnReattach(progressBar, true);
var statusText = new TextBlock
{
@@ -561,7 +563,10 @@ emailed around. Name the target so "which server was that?" is answered
// new plan tab.
actualViewer.ConnectionString = connectionString;
actualViewer.SetConnectionServices(_credentialService, _connectionStore);
- actualViewer.SetConnectionStatus(dialog.ResultConnection.ServerName, database);
+ /* Adopt, not just paint: this tab's toolbar is visible, and status alone left the
+ database picker disabled and empty under a green server label. The dialog already
+ enumerated the databases this login can see. */
+ actualViewer.AdoptConnection(dialog.ResultConnection, database, dialog.ResultDatabases);
actualViewer.LoadPlan(actualPlanXml, "Actual Plan", queryText);
tab.Content = CreatePlanTabContent(actualViewer);
diff --git a/src/PlanViewer.App/MainWindow.axaml.cs b/src/PlanViewer.App/MainWindow.axaml.cs
index 5ac60b51..8b5ba158 100644
--- a/src/PlanViewer.App/MainWindow.axaml.cs
+++ b/src/PlanViewer.App/MainWindow.axaml.cs
@@ -882,13 +882,8 @@ private void About_Click(object? sender, RoutedEventArgs e)
about.ShowDialog(this);
}
-#pragma warning disable CS0618 // Data/DataFormats.Files deprecated but IDataTransfer API differs
private static readonly string[] _supportedExtensions = { ".sqlplan", ".xml", ".sql" };
-
-#pragma warning restore CS0618
-
-
///
/// Re-decides whether Compare Plans is offered, for every Compare button in the window (#447).
///
diff --git a/src/PlanViewer.App/PlanViewer.App.csproj b/src/PlanViewer.App/PlanViewer.App.csproj
index 0bad8531..497919ca 100644
--- a/src/PlanViewer.App/PlanViewer.App.csproj
+++ b/src/PlanViewer.App/PlanViewer.App.csproj
@@ -9,18 +9,15 @@
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-
@@ -28,11 +25,18 @@
-
+
diff --git a/src/PlanViewer.App/Services/AdviceContentBuilder.Cards.cs b/src/PlanViewer.App/Services/AdviceContentBuilder.Cards.cs
index 01a76d67..e77930aa 100644
--- a/src/PlanViewer.App/Services/AdviceContentBuilder.Cards.cs
+++ b/src/PlanViewer.App/Services/AdviceContentBuilder.Cards.cs
@@ -10,8 +10,9 @@
namespace PlanViewer.App.Services;
///
-/// The card view of an : a header strip of stat chips over one card
-/// per statement, in place of the monospace report the pane used to print.
+/// The card view of an : a header strip — server line, labelled
+/// context facts, stat chips — over one card per statement, in place of the monospace report
+/// the pane used to print.
///
/// The report itself has not moved. still writes it from the same
/// model and the Copy button still hands out exactly those bytes — this file is a second view over
@@ -58,9 +59,12 @@ internal static StackPanel BuildCards(AnalysisResult analysis, Action? onNo
// -----------------------------------------------------------------------------------
///
- /// Server line plus the summary as chips. The counts used to be four lines of "Label: value"
- /// that had to be read to be counted; as chips the critical count is the one red thing on the
- /// screen and lands before the reader has finished the server name.
+ /// Server line, the context facts as labelled rows, the deviating settings as chips, then
+ /// the summary as chips. The counts used to be four lines of "Label: value" that had to be
+ /// read to be counted; as chips the critical count is the one red thing on the screen and
+ /// lands before the reader has finished the server name. The context facts went the other
+ /// way (#540): they started as chips too, and seven neutral facts in identical pills made a
+ /// row to decode where the old report had a block to scan.
///
private static Border BuildHeaderStrip(AnalysisResult analysis)
{
@@ -80,9 +84,13 @@ private static Border BuildHeaderStrip(AnalysisResult analysis)
});
}
- var context = ContextChips(analysis.ServerContext);
- if (context.Count > 0)
- body.Children.Add(ChipRow(context));
+ var facts = ContextFacts(analysis.ServerContext);
+ if (facts != null)
+ body.Children.Add(facts);
+
+ var outliers = OutlierChips(analysis.ServerContext);
+ if (outliers.Count > 0)
+ body.Children.Add(ChipRow(outliers));
var summary = analysis.Summary;
var stats = new List
@@ -147,32 +155,83 @@ private static Border BuildHeaderStrip(AnalysisResult analysis)
}
///
- /// The instance and database settings the report prints under Server Context. Chips rather
- /// than lines: every one of them is a short "name value" pair that a reader scans for an
- /// outlier, which is what a chip row is for.
+ /// The neutral facts the report prints under Server Context — hardware, the three instance
+ /// settings, the database — as labelled rows. These shipped as chips first, and a user said
+ /// the old report was easier to read, naming this section (#540): a chip earns its keep by
+ /// popping out of a row, and seven facts that are all supposed to be there pop nothing.
+ /// Labels give the block back its shape; the pills that remain ()
+ /// are the ones with something to say.
///
- private static List ContextChips(ServerContextResult? ctx)
+ private static Grid? ContextFacts(ServerContextResult? ctx)
{
- var chips = new List();
if (ctx == null)
- return chips;
+ return null;
+
+ var rows = new List<(string Label, string Value)>();
if (ctx.CpuCount > 0)
- chips.Add(Chip($"{ctx.CpuCount:N0} CPUs, {ctx.PhysicalMemoryMB:N0} MB RAM", QuietTextBrush));
- chips.Add(Chip($"MAXDOP {ctx.MaxDop}", QuietTextBrush));
- chips.Add(Chip($"Cost threshold {ctx.CostThresholdForParallelism}", QuietTextBrush));
- chips.Add(Chip($"Max memory {ctx.MaxServerMemoryMB:N0} MB", QuietTextBrush));
+ rows.Add(("Hardware", $"{ctx.CpuCount:N0} CPUs, {ctx.PhysicalMemoryMB:N0} MB RAM"));
- var db = ctx.Database;
- if (db == null)
- return chips;
+ rows.Add(("Instance",
+ $"MAXDOP {ctx.MaxDop}, cost threshold {ctx.CostThresholdForParallelism}, " +
+ $"max memory {ctx.MaxServerMemoryMB:N0} MB"));
+
+ if (ctx.Database is { } db)
+ {
+ var collation = string.IsNullOrWhiteSpace(db.CollationName) ? "" : $", {db.CollationName}";
+ rows.Add(("Database", $"{db.Name} (compat {db.CompatibilityLevel}{collation})"));
+ }
+
+ var grid = new Grid
+ {
+ ColumnDefinitions = new ColumnDefinitions("Auto,*"),
+ // Bottom 4 meets the next chip row's top 4 to keep the strip's 8px block rhythm.
+ Margin = new Avalonia.Thickness(0, 2, 0, 4)
+ };
+
+ for (int i = 0; i < rows.Count; i++)
+ {
+ grid.RowDefinitions.Add(new RowDefinition(GridLength.Auto));
+ var rowTop = i == 0 ? 0 : 2;
- chips.Add(Chip($"{db.Name} (compat {db.CompatibilityLevel})", AccentTokenBrush));
- if (!string.IsNullOrWhiteSpace(db.CollationName))
- chips.Add(Chip(db.CollationName, QuietTextBrush));
+ var label = new SelectableTextBlock
+ {
+ Text = rows[i].Label,
+ FontSize = 12,
+ Foreground = QuietTextBrush,
+ Margin = new Avalonia.Thickness(0, rowTop, 12, 0),
+ VerticalAlignment = VerticalAlignment.Top
+ };
+ Grid.SetRow(label, i);
+ grid.Children.Add(label);
+
+ var value = new SelectableTextBlock
+ {
+ Text = rows[i].Value,
+ FontSize = 12,
+ Foreground = ValueBrush,
+ TextWrapping = TextWrapping.Wrap,
+ Margin = new Avalonia.Thickness(0, rowTop, 0, 0)
+ };
+ Grid.SetRow(value, i);
+ Grid.SetColumn(value, 1);
+ grid.Children.Add(value);
+ }
+
+ return grid;
+ }
+
+ ///
+ /// The settings that deviate from a healthy default — the report's indented "notable" lines,
+ /// plus non-default scoped configs — as chips on the severity colours. A short row of things
+ /// worth a second look is what a chip row is for.
+ ///
+ private static List OutlierChips(ServerContextResult? ctx)
+ {
+ var chips = new List();
+ if (ctx?.Database is not { } db)
+ return chips;
- /* Same rule the report applies: only settings that deviate from a healthy default get
- named, so the row stays a list of things worth a second look. */
if (db.SnapshotIsolationState > 0)
chips.Add(Chip("Snapshot isolation ON", WarningBrush));
if (db.ReadCommittedSnapshot)
diff --git a/src/PlanViewer.Core/PlanViewer.Core.csproj b/src/PlanViewer.Core/PlanViewer.Core.csproj
index 943f6092..50104900 100644
--- a/src/PlanViewer.Core/PlanViewer.Core.csproj
+++ b/src/PlanViewer.Core/PlanViewer.Core.csproj
@@ -8,9 +8,9 @@
-
+
-
+
diff --git a/src/PlanViewer.Core/Services/ParameterSubstitution.cs b/src/PlanViewer.Core/Services/ParameterSubstitution.cs
index 3ab9c521..0e3a2e9c 100644
--- a/src/PlanViewer.Core/Services/ParameterSubstitution.cs
+++ b/src/PlanViewer.Core/Services/ParameterSubstitution.cs
@@ -47,46 +47,60 @@ public static ParameterSubstitutionResult Apply(
if (values.Count == 0)
return new ParameterSubstitutionResult(statementText, 0);
+ /* A plan from the plan cache or Query Store keeps an sp_executesql statement's declaration
+ list in front of it: "(@p1 int, @p2 int)SELECT …". The names in that list declare the
+ parameters, they do not read them. Substituted, they became "(10 int, 20 int)SELECT …",
+ which is neither the plan's text nor runnable. So only the statement after the list gets
+ values, and when it gets any, the list is dropped: this text is meant to run, and a
+ declaration list is not T-SQL on its own. Nothing substituted leaves the text as it was.
+ A list that never closes is text the plan cut off at 4,000 characters inside the list,
+ so there is no statement to put values into. */
+ var bodyStart = DeclarationListEnd(statementText);
+ if (bodyStart < 0)
+ return new ParameterSubstitutionResult(statementText, 0);
+
+ var text = bodyStart == 0 ? statementText : statementText[bodyStart..].TrimStart();
+
/* One fact about the whole statement, settled up front: inside an EXEC statement, every
token sitting to the left of an "=" is an assignment target — the return-status variable
or a named argument's name — because EXEC grammar has no other use for "=" at all. A
per-token back-scan cannot see this for the FIRST named argument (what precedes it is
the procedure name, not a keyword), which is how "EXEC dbo.p @debug = @debug" got its
left-hand side substituted into "EXEC dbo.p 1 = @debug". */
- var assignsThroughEquals = StatementLeadsWithExec(statementText);
+ var assignsThroughEquals = StatementLeadsWithExec(text);
- var sb = new StringBuilder(statementText.Length);
+ var sb = new StringBuilder(text.Length);
var substitutions = 0;
var i = 0;
- while (i < statementText.Length)
+ while (i < text.Length)
{
- var c = statementText[i];
+ var c = text[i];
/* Regions where an @name is text, not a parameter. A string literal is the case that
matters in practice — LIKE 'kexin%' sits right next to the parameters in the #466
repro — but a delimited identifier can hold anything, and a comment is not code. */
if (c == '\'' || c == '"')
{
- i = CopyDelimited(statementText, i, c, c, sb);
+ i = CopyDelimited(text, i, c, c, sb);
continue;
}
if (c == '[')
{
- i = CopyDelimited(statementText, i, '[', ']', sb);
+ i = CopyDelimited(text, i, '[', ']', sb);
continue;
}
- if (c == '-' && i + 1 < statementText.Length && statementText[i + 1] == '-')
+ if (c == '-' && i + 1 < text.Length && text[i + 1] == '-')
{
- i = CopyLineComment(statementText, i, sb);
+ i = CopyLineComment(text, i, sb);
continue;
}
- if (c == '/' && i + 1 < statementText.Length && statementText[i + 1] == '*')
+ if (c == '/' && i + 1 < text.Length && text[i + 1] == '*')
{
- i = CopyBlockComment(statementText, i, sb);
+ i = CopyBlockComment(text, i, sb);
continue;
}
@@ -94,15 +108,15 @@ matters in practice — LIKE 'kexin%' sits right next to the parameters in the #
tail of an identifier such as "t@0" is part of that identifier. The scan below claims
the longest run of identifier characters, which handles the first; the preceding
character is checked here, which handles the second. */
- if (c == '@' && !IsIdentifierPart(i > 0 ? statementText[i - 1] : '\0'))
+ if (c == '@' && !IsIdentifierPart(i > 0 ? text[i - 1] : '\0'))
{
var end = i + 1;
- while (end < statementText.Length && IsIdentifierPart(statementText[end]))
+ while (end < text.Length && IsIdentifierPart(text[end]))
end++;
- var token = statementText[i..end];
+ var token = text[i..end];
if (values.TryGetValue(token, out var value)
- && !IsAssignmentTarget(statementText, i, end, assignsThroughEquals))
+ && !IsAssignmentTarget(text, i, end, assignsThroughEquals))
{
sb.Append(value);
substitutions++;
@@ -125,6 +139,37 @@ tail of an identifier such as "t@0" is part of that identifier. The scan below c
: new ParameterSubstitutionResult(sb.ToString(), substitutions);
}
+ ///
+ /// Where the statement starts in text that opens with an sp_executesql declaration list,
+ /// such as (@p1 int, @p2 decimal(18,2))SELECT …. Returns 0 when the text has no list,
+ /// and -1 when the list never closes: a plan cuts statement text off at 4,000 characters, and
+ /// the declarations for a long IN list can fill all of them. The list ends at the parenthesis
+ /// that closes its first one, so the parentheses of a type are counted, not taken for the end.
+ /// A statement cannot begin with (@, so that opening always means a list.
+ /// ReproScriptBuilder strips the list with this too. (The web project compiles this
+ /// file without it, so the name is not a cref.)
+ ///
+ internal static int DeclarationListEnd(string text)
+ {
+ var i = 0;
+ while (i < text.Length && char.IsWhiteSpace(text[i]))
+ i++;
+
+ if (i + 1 >= text.Length || text[i] != '(' || text[i + 1] != '@')
+ return 0;
+
+ var depth = 0;
+ for (; i < text.Length; i++)
+ {
+ if (text[i] == '(')
+ depth++;
+ else if (text[i] == ')' && --depth == 0)
+ return i + 1;
+ }
+
+ return -1; // the list never closes: the text was cut off inside it
+ }
+
///
/// True when the parameter token spanning to is
/// being assigned TO rather than read from, in which case its value must not be written over it.
diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs
index dbd1b217..3495640b 100644
--- a/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs
+++ b/src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs
@@ -6,6 +6,33 @@
namespace PlanViewer.Core.Services;
+///
+/// Identity of the object a scan or seek predicate belongs to — enough to tell its own columns
+/// apart from a column on another table, or an outer reference a Nested Loops join passed it one
+/// row at a time (#564). Built once in 's
+/// DetectNonSargablePredicate and threaded down through DetectNonSargablePattern in
+/// place of the old isTableVariableScan flag (#561). Internal, not private, so string-level
+/// tests can build one directly instead of loading a fixture plan.
+///
+/// The scan's alias, or null when it has none. The parser strips brackets.
+///
+/// The last part of the scan's object name — a real table's bare name, a temp table cleaned to
+/// #t, or a table variable's own @tv (its ObjectName never carries a schema, so this
+/// is the whole name).
+///
+/// Mirrors PlanAnalyzer.IsTableVariable(node).
+///
+/// Bare column names — never a real table's or a temp table's, always another unaliased table
+/// variable's — that reach this scan as an outer reference rather than one of its own columns. Only
+/// populated by ancestor Nested Loops whose INNER input holds this scan; see
+/// PlanAnalyzer.CollectBareOuterReferences.
+///
+internal readonly record struct ScanIdentity(
+ string? Alias,
+ string? Table,
+ bool IsTableVariable,
+ IReadOnlySet BareOuterReferences);
+
public static partial class PlanAnalyzer
{
private static bool HasBatchModeNode(PlanNode node)
@@ -21,6 +48,14 @@ private static bool HasBatchModeNode(PlanNode node)
return false;
}
+ ///
+ /// True when a node scans or modifies a table variable rather than a real table. The scan's
+ /// Object element renders a table variable's name as "[@tv]", where a real table always has a
+ /// schema: "[db].[dbo].[t]" — so the leading @ alone tells them apart.
+ ///
+ private static bool IsTableVariable(PlanNode node) =>
+ !string.IsNullOrEmpty(node.ObjectName) && node.ObjectName.StartsWith("@");
+
/* #440: collects the operators it found, because this walk already knows exactly which ones
touched a table variable and used to throw that away. Two lists rather than one, since the
two warnings this feeds are about different operators: every operator referencing a table
@@ -29,7 +64,7 @@ private static void CheckForTableVariables(PlanNode node, bool isModification,
ref bool hasTableVar, ref bool modifiesTableVar,
List? referencingNodeIds = null, List? modifyingNodeIds = null)
{
- if (!string.IsNullOrEmpty(node.ObjectName) && node.ObjectName.StartsWith("@"))
+ if (IsTableVariable(node))
{
hasTableVar = true;
referencingNodeIds?.Add(node.NodeId);
@@ -158,8 +193,85 @@ private static bool IsScanOperator(PlanNode node)
if (!IsRowstoreScan(node))
return null;
- var predicate = node.Predicate;
+ return DetectNonSargablePattern(node.Predicate, BuildScanIdentity(node));
+ }
+
+ ///
+ /// The for a scan node, or null when it has no Object element at
+ /// all — DetectNonSargablePattern falls back to its old, coarser behavior in that case rather
+ /// than trying to match ownership against an identity with nothing in it.
+ ///
+ private static ScanIdentity? BuildScanIdentity(PlanNode node)
+ {
+ if (string.IsNullOrEmpty(node.ObjectName))
+ return null;
+
+ // ObjectName is "schema.table" (a table variable's has no schema, so it's just "@tv");
+ // the owner a predicate names is always the bare table, never schema-qualified (#564).
+ var dot = node.ObjectName.LastIndexOf('.');
+ var table = dot >= 0 ? node.ObjectName[(dot + 1)..] : node.ObjectName;
+
+ return new ScanIdentity(node.ObjectAlias, table, IsTableVariable(node), CollectBareOuterReferences(node));
+ }
+
+ ///
+ /// Bare column names (#564) that reach as an outer reference from
+ /// another unaliased table variable, rather than one of node's own columns — the two look
+ /// identical once rendered bare (#561), so a predicate can't tell them apart by text alone.
+ ///
+ /// Walks up through every Nested Loops ancestor whose INNER input (second child) holds
+ /// node, the same as the plan diagram's own outer-vs-inner split, and keeps the OuterReferences
+ /// entries with no ".". FormatColumnRef (ShowPlanParser.Helpers.cs) renders a table-qualified
+ /// outer reference as "Table.Column" — that always names a different table than node's own bare
+ /// columns, so only a dot-free entry can ever collide with one. A Nested Loops whose OUTER input
+ /// holds node contributes nothing: that input is where the reference comes from, not where it is
+ /// consumed, so node is not the one reading it.
+ ///
+ private static HashSet CollectBareOuterReferences(PlanNode node)
+ {
+ var names = new HashSet(StringComparer.OrdinalIgnoreCase);
+ var child = node;
+ var ancestor = node.Parent;
+
+ while (ancestor != null)
+ {
+ if (ancestor.PhysicalOp == "Nested Loops" &&
+ !string.IsNullOrEmpty(ancestor.OuterReferences) &&
+ ancestor.Children.Count > 1 &&
+ ancestor.Children[1] == child)
+ {
+ foreach (var reference in ancestor.OuterReferences.Split(", "))
+ {
+ if (!reference.Contains('.'))
+ names.Add(reference);
+ }
+ }
+
+ child = ancestor;
+ ancestor = ancestor.Parent;
+ }
+
+ return names;
+ }
+ ///
+ /// The pattern half of : which non-SARGable shape, if
+ /// any, a predicate ScalarString has.
+ ///
+ /// Internal so predicate shapes can be tested as raw strings. The shapes that matter
+ /// (compound AND/OR predicates, date ranges, parenthesized groups, AND inside a literal or a
+ /// bracketed name) outnumber any sensible set of plan fixtures, and every one of them is
+ /// decided entirely in this method and the helpers it calls.
+ ///
+ /// is the scanned object the caller has confirmed the predicate
+ /// belongs to (#561, #564) — null keeps today's coarser behavior unchanged for every existing
+ /// caller: any dotted name counts as a column, and no bare name does. A real table or an aliased
+ /// table variable always renders its own column dotted, at minimum [table].[col] or
+ /// [alias].[col], but an unaliased table variable's own column has no dotted qualifier at all —
+ /// see and .
+ ///
+ internal static string? DetectNonSargablePattern(string predicate, ScanIdentity? identity = null)
+ {
// CASE expression in predicate — check first because CASE bodies
// often contain CONVERT_IMPLICIT that isn't the root cause
if (CaseInPredicateRegex.IsMatch(predicate))
@@ -167,12 +279,18 @@ private static bool IsScanOperator(PlanNode node)
// CONVERT_IMPLICIT — most common non-SARGable pattern, but only when it converts the
// COLUMN. Converting the parameter up to the column's type costs nothing (#436).
- if (ConvertImplicitWrapsColumn(predicate))
+ if (ConvertImplicitWrapsColumn(predicate, identity))
return "Implicit conversion (CONVERT_IMPLICIT)";
- // ISNULL / COALESCE wrapping column
- if (Regex.IsMatch(predicate, @"\b(isnull|coalesce)\s*\(", RegexOptions.IgnoreCase))
- return "ISNULL/COALESCE wrapping column";
+ // ISNULL / COALESCE wrapping column — on the column side only. ISNULL(@p, 0) on the
+ // parameter side is a runtime constant and seeks fine; flagging it contradicted this
+ // warning's own "wrapping a column" message. col = ISNULL(@p, col) is still caught,
+ // because the column sits inside the function, on its side of the comparison.
+ foreach (Match isnullMatch in IsnullCoalesceRegex.Matches(predicate))
+ {
+ if (IsFunctionOnColumnSide(predicate, isnullMatch, identity))
+ return "ISNULL/COALESCE wrapping column";
+ }
// Common function calls on columns — but only if the function wraps a column,
// not a parameter/variable. Split on comparison operators to check which side
@@ -183,7 +301,7 @@ private static bool IsScanOperator(PlanNode node)
foreach (Match funcMatch in FunctionInPredicateRegex.Matches(predicate))
{
var funcName = funcMatch.Groups[1].Value.ToUpperInvariant();
- if (funcName != "CONVERT_IMPLICIT" && IsFunctionOnColumnSide(predicate, funcMatch))
+ if (funcName != "CONVERT_IMPLICIT" && IsFunctionOnColumnSide(predicate, funcMatch, identity))
return $"Function call ({funcName}) on column";
}
@@ -213,10 +331,13 @@ private static bool IsScanOperator(PlanNode node)
/// type and carries no brackets; a column reference in the remainder is the conversion input.
///
/// Internal so the column-vs-variable line can be tested against raw predicate strings in
- /// showplan shape — the table-variable case ([@tv].[col] IS a column) has no committed plan
- /// fixture, and the distinction lives entirely in this method and the regex it shares.
+ /// showplan shape. covers the unaliased table-variable case — a bare
+ /// name with no dotted qualifier, e.g. CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n] — which
+ /// this method cannot tell from a parameter or an expression on its own, and tells that scan's
+ /// own bare columns apart from a bare outer reference off a different one (#564); see
+ /// (#561).
///
- internal static bool ConvertImplicitWrapsColumn(string predicate)
+ internal static bool ConvertImplicitWrapsColumn(string predicate, ScanIdentity? identity = null)
{
foreach (Match match in ConvertImplicitRegex.Matches(predicate))
{
@@ -225,13 +346,139 @@ internal static bool ConvertImplicitWrapsColumn(string predicate)
// Unparseable means we cannot tell what is being converted. Assume the worst, matching
// IsFunctionOnColumnSide, rather than silently dropping a real conversion.
- if (arguments == null || ColumnReferenceRegex.IsMatch(arguments))
+ if (arguments == null || IsColumnReference(arguments, identity))
return true;
}
return false;
}
+ ///
+ /// Whether names a column belonging to —
+ /// the scanned object, not a column on some other table, or an outer reference a Nested Loops
+ /// join passed it in one row at a time (#564). A wrapped column stops an index seek only when it
+ /// is actually the scanned table's own column: an outer reference already varies one row at a
+ /// time no matter what wraps it, so it costs nothing extra, and a column on some other table was
+ /// never going to seek this one anyway.
+ ///
+ /// Null means the caller has not identified a scan, and keeps
+ /// this method's old, coarser behavior: alone, so any dotted
+ /// name counts as a column and no bare name does. That regex is still every non-null identity's
+ /// first check too — a real table or an aliased table variable always renders its own column
+ /// dotted, at minimum [table].[col] ('s own comment has
+ /// the full shape) — but there it is followed by an ownership check: an outer reference and a
+ /// column on another table are dotted too, and the fix is telling them apart, not giving up on
+ /// dotted names altogether.
+ ///
+ /// Un-owned dotted names are read with , one at a time:
+ ///
+ /// A run of two or more bracket parts right after literal as is the aliased form —
+ /// [..].[I].[X] as [i].[X] or @tv.[col] as [v].[col] — and it owns the scan only when
+ /// the scan has an alias and it matches the LAST-BUT-ONE part, the alias right before the column
+ /// (case-insensitively). The part before as is not a reference of its own — in a self
+ /// join it names the OTHER instance of the same table, under its own alias — so it is skipped
+ /// outright rather than read as a second, competing reference.
+ /// A run of two or more bracket parts with no as before it is the unaliased dotted
+ /// form — [db].[schema].[table].[col], or [#t].[col] for a temp table — and it owns
+ /// the scan only when the scan has NO alias and its table matches the LAST-BUT-ONE part
+ /// (case-insensitively). A scan with an alias always renders its own column through that alias,
+ /// so an unaliased dotted reference elsewhere in the same predicate names a different object.
+ /// A single bracket part is the bare form (#561) — read as a column only on an unaliased
+ /// table variable, unless it is a parameter or variable ([@p1]), an optimizer expression
+ /// ([Expr1003]), the name of a function call (followed by () rather than a
+ /// reference, or a name lists as another unaliased
+ /// table variable's outer reference rather than this one's own column (#564). A string literal
+ /// that looks bracketed ('[Y]') is never read as a name at all — see
+ /// .
+ ///
+ ///
+ private static bool IsColumnReference(string text, ScanIdentity? identity)
+ {
+ if (identity == null)
+ return ColumnReferenceRegex.IsMatch(text);
+
+ var scan = identity.Value;
+
+ foreach (Match match in BracketedNameRegex.Matches(text))
+ {
+ if (!match.Groups["name"].Success || match.Groups["call"].Success)
+ continue; // a string literal, or the name of a function
+
+ var name = match.Groups["name"].Value;
+
+ // " as [alias].[col]" — the prefix is not a reference of its own; the real
+ // reference is the [alias].[col] pair that follows, matched separately on its own turn
+ // through this loop (#564).
+ if (FollowedByAsBracket(text, match.Index + match.Length))
+ continue;
+
+ var parts = NamePartRegex.Matches(name);
+ if (parts.Count >= 2)
+ {
+ var owner = StripBrackets(parts[^2].Value);
+
+ if (PrecededByAs(text, match.Index))
+ {
+ // Aliased: [alias].[col]. Owned only through a matching alias.
+ if (!string.IsNullOrEmpty(scan.Alias) &&
+ string.Equals(owner, scan.Alias, StringComparison.OrdinalIgnoreCase))
+ return true;
+ }
+ else
+ {
+ // Unaliased dotted: [..].[table].[col]. An aliased scan's own column never
+ // renders this way, so this can only own the scan when the scan has none.
+ // The parser cleans a temp table's full tempdb name (#t___...___000000000003)
+ // down to #t in the scan's own name, so the owner here is cleaned the same way.
+ if (string.IsNullOrEmpty(scan.Alias) && !string.IsNullOrEmpty(scan.Table) &&
+ string.Equals(ShowPlanParser.CleanTempTableName(owner), scan.Table,
+ StringComparison.OrdinalIgnoreCase))
+ return true;
+ }
+
+ continue;
+ }
+
+ // A single bracketed part.
+ if (name.StartsWith("[@", StringComparison.Ordinal))
+ continue; // a parameter or a variable: [@p1]
+
+ if (ExpressionColumnRegex.IsMatch(name))
+ continue; // an optimizer-generated expression, not an actual column: [Expr1003]
+
+ if (scan.IsTableVariable && string.IsNullOrEmpty(scan.Alias) &&
+ !scan.BareOuterReferences.Contains(StripBrackets(name)))
+ return true; // a bare name — this unaliased table variable's own column (#561)
+ }
+
+ return false;
+ }
+
+ ///
+ /// True when [..] starts with the literal
+ /// as [ — the start of the [alias].[col] half of the aliased column-reference
+ /// form, which follows the part that is not a reference of its own (#564).
+ ///
+ private static bool FollowedByAsBracket(string text, int index) =>
+ index >= 0 && index + 5 <= text.Length &&
+ text.AsSpan(index, 5).Equals(" as [", StringComparison.OrdinalIgnoreCase);
+
+ ///
+ /// True when [..] ends with the literal
+ /// as — is a match's own start, so this reads the four
+ /// characters right before it (#564).
+ ///
+ private static bool PrecededByAs(string text, int index) =>
+ index >= 4 && text.AsSpan(index - 4, 4).Equals(" as ", StringComparison.OrdinalIgnoreCase);
+
+ ///
+ /// Strips the brackets off one bracket part matched by — never a
+ /// whole dotted chain, so a plain Replace is enough; a "]]"-escaped literal bracket (which
+ /// FormatColumnRef, elsewhere, does not unescape either) passes through unchanged.
+ ///
+ private static string StripBrackets(string bracketPart) =>
+ bracketPart.Replace("[", "").Replace("]", "");
+
///
/// Returns the text between the parenthesis at and its match,
/// or null if the parentheses do not balance. Needed because the target type of a conversion can
@@ -261,29 +508,75 @@ internal static bool ConvertImplicitWrapsColumn(string predicate)
/// Checks whether a function call in a predicate is on the column side of the comparison.
/// Predicate ScalarStrings look like: [db].[schema].[table].[col]>dateadd(day,(0),[@var])
/// If the function is only on the parameter/literal side, it's still SARGable.
+ ///
+ /// Only the function's own comparison is read (#556). A compound predicate is
+ /// several comparisons joined by AND/OR, and the function belongs to exactly one of them.
+ /// Splitting the whole predicate at its FIRST operator instead put every later comparison,
+ /// column and all, on the function's side: in [t].[A]=[@1] AND [t].[B]=CONVERT(tinyint,[@2],0)
+ /// the CONVERT looked like it shared a side with [t].[B], and so did the dateadd in the
+ /// everyday range [t].[d]>=dateadd(day,(-7),getdate()) AND [t].[d]<getdate().
+ ///
+ /// is passed straight to to
+ /// cover the unaliased table-variable case, e.g. abs([X])=(1) (#561), and to tell that
+ /// scan's own bare column apart from another unaliased table variable's outer reference in the
+ /// same shape, e.g. abs([A]) where A is an outer reference and only X is this scan's own
+ /// column in [X]=abs([A]) (#564).
///
- private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch)
+ private static bool IsFunctionOnColumnSide(string predicate, Match funcMatch, ScanIdentity? identity = null)
{
- // Find the comparison operator that splits the predicate into left/right sides.
- // Operators in ScalarString: >=, <=, <>, >, <, =
- var compMatch = Regex.Match(predicate, @"(?])([<>=!]{1,2})(?![<>=])");
+ var comparison = ComparisonContaining(predicate, funcMatch.Index, out var offset);
+
+ var compMatch = ComparisonOperatorRegex.Match(comparison);
if (!compMatch.Success)
return true; // No comparison found — can't determine side, assume worst case
var compPos = compMatch.Index;
- var funcPos = funcMatch.Index;
+ var funcPos = funcMatch.Index - offset;
- // Determine which side the function is on
- var funcSide = funcPos < compPos ? "left" : "right";
-
- // Check if that side also contains a column reference [...].[...].[...]
- string side = funcSide == "left"
- ? predicate[..compPos]
- : predicate[(compPos + compMatch.Length)..];
+ // The side of this comparison the function is on, and whether a column shares it
+ string side = funcPos < compPos
+ ? comparison[..compPos]
+ : comparison[(compPos + compMatch.Length)..];
// Same column-vs-variable distinction ConvertImplicitWrapsColumn needs, so it shares the
- // one regex rather than keeping a second copy of the pattern in sync by hand.
- return ColumnReferenceRegex.IsMatch(side);
+ // one helper rather than keeping a second copy of the logic in sync by hand.
+ return IsColumnReference(side, identity);
+ }
+
+ ///
+ /// The single comparison around : the text between the nearest
+ /// AND/OR before it and the nearest after it. is where that text
+ /// starts in , so positions can be translated into it.
+ ///
+ /// Operators are split on at every depth, not just the top level: a parenthesized group
+ /// like [t].[A]=(1) AND ([t].[B]=f([@p]) OR [t].[C]=(3)) has to come apart into its
+ /// three comparisons, or the group would be read as one. The leftover grouping parentheses
+ /// cannot move a comparison operator or add a column, so they are harmless. No function in a
+ /// ScalarString takes AND/OR inside its arguments; CASE does, and it is caught earlier.
+ ///
+ private static string ComparisonContaining(string predicate, int position, out int offset)
+ {
+ var start = 0;
+ var end = predicate.Length;
+
+ foreach (Match match in LogicalOperatorRegex.Matches(predicate))
+ {
+ if (!match.Groups[1].Success)
+ continue; // a string literal or bracketed name, skipped whole
+
+ if (match.Index + match.Length <= position)
+ {
+ start = match.Index + match.Length;
+ }
+ else
+ {
+ end = match.Index;
+ break;
+ }
+ }
+
+ offset = start;
+ return predicate[start..end];
}
///
@@ -321,6 +614,54 @@ private static bool IsOrExpansionChain(PlanNode concatenationNode)
return true;
}
+ ///
+ /// True when a lookup branch under an OR expansion's Concatenation builds its seek value from
+ /// another input. A join OR does: in ON u.Id = p.OwnerUserId OR u.Id = p.LastEditorUserId the
+ /// branches produce [Posts].[OwnerUserId] and [Posts].[LastEditorUserId], once per outer row.
+ /// The dynamic seek for an IN list of parameters (#558) has the same operator shape, but its
+ /// branches produce only parameters and literals ([@p1], (62)), which no outer row changes.
+ ///
+ private static bool LookupReadsAnotherInput(PlanNode branch)
+ {
+ var values = branch.PhysicalOp == "Constant Scan"
+ ? branch.ConstantScanValues
+ : branch.DefinedValues;
+
+ // Nothing to read, so nothing proves a parameter list: keep the warning.
+ if (string.IsNullOrEmpty(values))
+ return true;
+
+ return ReadsAnotherInput(values);
+ }
+
+ ///
+ /// True when a ScalarString names anything other than a parameter or a variable: a column
+ /// ([db].[dbo].[T].[c], or @tv.[c] as [v].[c] on a table variable) or an expression column
+ /// ([Expr1003]). Function names ([dbo].[fn](...)) and string literals are skipped. An
+ /// expression column counts too: an OR join on o.X + 1 renders its branches as [Expr1002],
+ /// computed on the outer input. The Constant Scan under a lookup branch is normally empty,
+ /// so the branch has no expression of its own to name, and a name that cannot be proved to
+ /// be a parameter keeps the warning, as the shape check alone did. Internal so the shapes
+ /// can be tested as raw strings.
+ ///
+ internal static bool ReadsAnotherInput(string scalarString)
+ {
+ foreach (Match match in BracketedNameRegex.Matches(scalarString))
+ {
+ if (!match.Groups["name"].Success || match.Groups["call"].Success)
+ continue; // a string literal, or the name of a function
+
+ var name = match.Groups["name"].Value;
+ if (name.StartsWith("[@", StringComparison.Ordinal) &&
+ !name.Contains("].[", StringComparison.Ordinal))
+ continue; // a parameter or a variable: [@p1]
+
+ return true;
+ }
+
+ return false;
+ }
+
///
/// Finds Sort and Hash Match operators in the tree that consume memory.
///
diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs
index 8dcbbc2c..dd35443b 100644
--- a/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs
+++ b/src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs
@@ -642,16 +642,21 @@ private static void Rule15_JoinOrClause(PlanNode node, PlanStatement stmt, Analy
if (!cfg.IsRuleDisabled(15) && node.PhysicalOp == "Concatenation")
{
var constantScanBranches = node.Children
- .Count(c => c.PhysicalOp == "Constant Scan" ||
+ .Where(c => c.PhysicalOp == "Constant Scan" ||
(c.PhysicalOp == "Compute Scalar" &&
- c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")));
-
- if (constantScanBranches >= 2 && IsOrExpansionChain(node))
+ c.Children.Any(gc => gc.PhysicalOp == "Constant Scan")))
+ .ToList();
+
+ /* #558: WHERE t.A IN (@p1, @p2) builds the same operator chain, as a dynamic seek over
+ the parameter values, and there is no join to rewrite. Only a lookup that takes its
+ value from another input's row makes the OR a join OR. */
+ if (constantScanBranches.Count >= 2 && IsOrExpansionChain(node) &&
+ constantScanBranches.Any(LookupReadsAnotherInput))
{
node.Warnings.Add(new PlanWarning
{
WarningType = "Join OR Clause",
- Message = $"OR in a join predicate. SQL Server rewrote the OR as {constantScanBranches} separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change \"FROM a JOIN b ON a.x = b.x OR a.y = b.y\" to \"FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y\".",
+ Message = $"OR in a join predicate. SQL Server rewrote the OR as {constantScanBranches.Count} separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change \"FROM a JOIN b ON a.x = b.x OR a.y = b.y\" to \"FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y\".",
Severity = PlanWarningSeverity.Warning
});
}
@@ -911,6 +916,13 @@ private static void Rule29_ImplicitConversionSeek(PlanNode node, PlanStatement s
}
+ // Rule 35 needs the statement itself to have run long enough that a 20% share
+ // means something. Under this floor, a statement of a few ms is dominated by
+ // one or two operators just because there's almost nothing else to divide the
+ // time among, so the share points at nothing (#562). Same floor rule 19 uses
+ // to fire on compile CPU, and the floor rule 4 uses to call UDF time Critical.
+ private const int Rule35MinStatementElapsedMs = 1000;
+
private static void Rule35_ExpensiveOperator(PlanNode node, PlanStatement stmt, AnalyzerConfig cfg)
{
// Rule 35: Expensive Operator — always show operators that take a significant
@@ -920,7 +932,7 @@ private static void Rule35_ExpensiveOperator(PlanNode node, PlanStatement stmt,
// elapsed. Only emits if no other warning is already on the node to avoid
// doubling up. The benefit % is just the self-time share.
if (!cfg.IsRuleDisabled(35) && node.HasActualStats && node.Warnings.Count == 0
- && stmt.QueryTimeStats != null && stmt.QueryTimeStats.ElapsedTimeMs > 0)
+ && stmt.QueryTimeStats != null && stmt.QueryTimeStats.ElapsedTimeMs >= Rule35MinStatementElapsedMs)
{
var selfMs = GetOperatorOwnElapsedMs(node);
var pct = (double)selfMs / stmt.QueryTimeStats.ElapsedTimeMs * 100;
diff --git a/src/PlanViewer.Core/Services/PlanAnalyzer.cs b/src/PlanViewer.Core/Services/PlanAnalyzer.cs
index aea2befb..d9a58004 100644
--- a/src/PlanViewer.Core/Services/PlanAnalyzer.cs
+++ b/src/PlanViewer.Core/Services/PlanAnalyzer.cs
@@ -30,15 +30,66 @@ public static partial class PlanAnalyzer
/* A column reference in a ScalarString is multi-part bracket-qualified ([schema].[table]).
A variable is a single bracket pair with an @ prefix ([@0]) and no dotted part after it —
- the "].[" sequence is what separates the two, NOT the @. The first cut of this pattern also
- excluded @ from the first part, which read as belt-and-braces but was actually a hole: a
- TABLE-variable column renders as [@tv].[col], so a genuine column-side CONVERT_IMPLICIT on
- one failed the match and the Non-SARGable warning silently vanished. A bare [@p] still
- cannot match, because nothing dotted follows it. */
+ the "].[" sequence is what separates the two, NOT the @. A bare [@p] cannot match, because
+ nothing dotted follows it.
+
+ A table-variable column is dotted only through its alias: SELECT v.X FROM @tv AS v WHERE
+ ABS(v.X) = 1 gives "abs(@tv.[X] as [v].[X])=(1)", and [v].[X] matches. With no alias it is
+ a bare name: SELECT X FROM @tv WHERE ABS(X) = 1 gives "abs([X])=(1)". Checked on SQL Server
+ 2016, 2017, 2019, 2022 and 2025, and none of them renders [@tv].[col]. A bare name has the
+ same shape as a parameter or an expression, so this regex never reads one as a column.
+ IsColumnReference (#561) can, when the caller has identified the scan: a bare name on an
+ unaliased table variable's own scan is a column, unless it is a bare outer reference passed
+ in from another unaliased table variable one row at a time (#564) — that one looks exactly
+ the same and this regex could never have told the two apart either. */
private static readonly Regex ColumnReferenceRegex = new(
@"\[[^\]]+\]\.\[",
RegexOptions.Compiled);
+ /* An optimizer-generated expression name in a ScalarString ([Expr1003]) — a computed value,
+ never an actual column, even on a table variable scan where a bare name is otherwise read
+ as a column (#561). Matched against BracketedNameRegex's whole name group, so it only ever
+ sees a single bracket part or a dotted chain, never raw predicate text. */
+ private static readonly Regex ExpressionColumnRegex = new(
+ @"^\[Expr\d+\]$",
+ RegexOptions.Compiled);
+
+ /* One bracket part of a name BracketedNameRegex already matched whole — [schema], [table],
+ [col], each on its own — so IsColumnReference (#564) can split "[db].[schema].[table].[col]"
+ or "[alias].[col]" into parts and read off the one right before the column, the owner. Reused
+ rather than a plain Split on "].[", so an identifier carrying an escaped "]]" still splits in
+ the right place. */
+ private static readonly Regex NamePartRegex = new(
+ @"\[(?:[^\]]|\]\])*\]",
+ RegexOptions.Compiled);
+
+ /* The operator a comparison turns on in a ScalarString: >=, <=, <>, !=, >, <, = or like.
+ Without like, [col] like upper([@p]) had no operator at all, fell to the assume-the-worst
+ default, and a function on the pattern was reported as a function on the column. */
+ private static readonly Regex ComparisonOperatorRegex = new(
+ @"(?])([<>=!]{1,2})(?![<>=])|\s(like)\s",
+ RegexOptions.IgnoreCase | RegexOptions.Compiled);
+
+ /* What joins one comparison to the next in a compound predicate. String literals and
+ bracketed identifiers are matched first, so an AND inside one of them (N'Tom AND Jerry',
+ [Terms and Conditions]) is consumed whole and never reaches the capture group. Only a
+ Groups[1] match is a real operator. */
+ private static readonly Regex LogicalOperatorRegex = new(
+ @"'(?:[^']|'')*'|\[(?:[^\]]|\]\])*\]|\s(AND|OR)\s",
+ RegexOptions.IgnoreCase | RegexOptions.Compiled);
+
+ private static readonly Regex IsnullCoalesceRegex = new(
+ @"\b(isnull|coalesce)\s*\(",
+ RegexOptions.IgnoreCase | RegexOptions.Compiled);
+
+ /* A name in a ScalarString: one bracketed part or a dotted chain of them ([@p1], [Expr1003],
+ [db].[dbo].[T].[c]). A name followed by ( is a function call. String literals are matched
+ first, so a bracket inside one ('[x]') is never read as a name. Only a match with the
+ name group is a name. */
+ private static readonly Regex BracketedNameRegex = new(
+ @"'(?:[^']|'')*'|(?\[(?:[^\]]|\]\])*\](?:\.\[(?:[^\]]|\]\])*\])*)(?\s*\()?",
+ RegexOptions.Compiled);
+
public static void Analyze(ParsedPlan plan, AnalyzerConfig? config = null, ServerMetadata? serverMetadata = null) =>
AnalyzeCancellable(plan, config, serverMetadata, CancellationToken.None);
diff --git a/src/PlanViewer.Core/Services/ReproScriptBuilder.cs b/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
index c19569c4..a7f8a59b 100644
--- a/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
+++ b/src/PlanViewer.Core/Services/ReproScriptBuilder.cs
@@ -365,39 +365,13 @@ private static List ExtractSetOptionsFromPlan(string planXml)
///
/// Strips the parameter declaration prefix from query text captured via sp_executesql.
/// Query text like "(@p1 int, @p2 nvarchar(50))SELECT ..." becomes "SELECT ...".
- /// Uses same approach as sp_QueryReproBuilder: find the closing ) followed by non-comma.
+ /// The list is found by the same parser that keeps parameter substitution out of it.
///
private static string StripParameterPrefix(string queryText)
{
- if (!queryText.StartsWith("(@", StringComparison.Ordinal))
- {
- return queryText;
- }
-
- /* Find the closing parenthesis that ends the parameter list.
- Look for ) followed by a character that's not a comma (which would indicate
- we're still inside nested parentheses in a type like decimal(18,2)). */
- int depth = 0;
- for (int i = 0; i < queryText.Length; i++)
- {
- char c = queryText[i];
- if (c == '(')
- {
- depth++;
- }
- else if (c == ')')
- {
- depth--;
- if (depth == 0)
- {
- /* Found the closing paren — return everything after it, trimmed */
- return queryText[(i + 1)..].TrimStart();
- }
- }
- }
-
- /* Couldn't find balanced parens — return original */
- return queryText;
+ /* No list (0), or a list that never closes (-1): the text stays as it is, as before. */
+ var bodyStart = ParameterSubstitution.DeclarationListEnd(queryText);
+ return bodyStart <= 0 ? queryText : queryText[bodyStart..].TrimStart();
}
///
diff --git a/src/PlanViewer.Core/Services/ShowPlanParser.Helpers.cs b/src/PlanViewer.Core/Services/ShowPlanParser.Helpers.cs
index c2189280..9bc1978f 100644
--- a/src/PlanViewer.Core/Services/ShowPlanParser.Helpers.cs
+++ b/src/PlanViewer.Core/Services/ShowPlanParser.Helpers.cs
@@ -14,7 +14,7 @@ public static partial class ShowPlanParser
/// SQL Server internally pads #temp names with underscores to 116 chars, then appends a hex suffix.
/// e.g. "#comment_sil_vous_plait_______________________________0000000000A86" → "#comment_sil_vous_plait"
///
- private static string CleanTempTableName(string name)
+ internal static string CleanTempTableName(string name)
{
if (name.Length == 0 || name[0] != '#') return name;
diff --git a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs
index c96bffd5..f0fefa7a 100644
--- a/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs
+++ b/src/PlanViewer.Ssms/Properties/AssemblyInfo.cs
@@ -7,5 +7,5 @@
[assembly: AssemblyProduct("Performance Studio for SSMS")]
[assembly: AssemblyCopyright("Copyright Darling Data 2026")]
[assembly: ComVisible(false)]
-[assembly: AssemblyVersion("1.26.0.0")]
-[assembly: AssemblyFileVersion("1.26.0.0")]
+[assembly: AssemblyVersion("1.27.0.0")]
+[assembly: AssemblyFileVersion("1.27.0.0")]
diff --git a/src/PlanViewer.Ssms/source.extension.vsixmanifest b/src/PlanViewer.Ssms/source.extension.vsixmanifest
index 898d75bb..c1d98b24 100644
--- a/src/PlanViewer.Ssms/source.extension.vsixmanifest
+++ b/src/PlanViewer.Ssms/source.extension.vsixmanifest
@@ -3,7 +3,7 @@
xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
Performance Studio for SSMS
diff --git a/tests/PlanViewer.Core.Tests/AdviceCardsTests.cs b/tests/PlanViewer.Core.Tests/AdviceCardsTests.cs
index c6c97491..885adc05 100644
--- a/tests/PlanViewer.Core.Tests/AdviceCardsTests.cs
+++ b/tests/PlanViewer.Core.Tests/AdviceCardsTests.cs
@@ -205,6 +205,69 @@ public void TheHeaderStripCarriesTheSummaryAsChips()
});
}
+ ///
+ /// #540: the context facts are labelled rows, not chips. A user held the pane against the
+ /// old report and named this section as the readable one — seven neutral facts in identical
+ /// pills gave the reader a row to decode where the report had a block to scan. Pinned here:
+ /// the facts read as label-and-value rows, and the only pills left from the context are the
+ /// settings that deviate, because popping out of a row is the one job a pill has.
+ ///
+ [Fact]
+ public void TheContextFactsAreLabelledRowsAndOnlyOutliersStayChips()
+ {
+ var result = Analyze(GoldenPlan);
+ // Small numbers on purpose: N0 grouping is culture-dependent and this test is not
+ // about separators.
+ result.ServerContext = new ServerContextResult
+ {
+ ServerName = "ASIDELL",
+ Edition = "Developer Edition (64-bit)",
+ ProductVersion = "16.0.4135.4",
+ CpuCount = 16,
+ PhysicalMemoryMB = 512,
+ MaxDop = 8,
+ CostThresholdForParallelism = 50,
+ MaxServerMemoryMB = 400,
+ Database = new DatabaseContextResult
+ {
+ Name = "StackOverflow",
+ CompatibilityLevel = 160,
+ CollationName = "SQL_Latin1_General_CP1_CI_AS",
+ ReadCommittedSnapshot = true,
+ AutoCreateStats = true,
+ AutoUpdateStats = true
+ }
+ };
+
+ HeadlessUi.Run(() =>
+ {
+ var panel = Show(AdviceContentBuilder.Build("", result));
+ var text = AllText(panel);
+
+ Assert.Contains("Hardware", text);
+ Assert.Contains("16 CPUs, 512 MB RAM", text);
+ Assert.Contains("MAXDOP 8, cost threshold 50, max memory 400 MB", text);
+ Assert.Contains("StackOverflow (compat 160, SQL_Latin1_General_CP1_CI_AS)", text);
+
+ /* Chips are the pane's only CornerRadius-10 Borders (cards and the strip are 6).
+ RCSI deviates, so it stays a pill; the neutral facts must not be in one. */
+ var chipTexts = panel.GetLogicalDescendants().OfType()
+ .Where(b => b.CornerRadius == new CornerRadius(10))
+ .Select(b => (b.Child as SelectableTextBlock)?.Text)
+ .ToList();
+ Assert.Contains("RCSI ON", chipTexts);
+ Assert.DoesNotContain(chipTexts, t => t != null && t.Contains("MAXDOP"));
+ Assert.DoesNotContain(chipTexts, t => t != null && t.Contains("compat"));
+
+ /* The press-anywhere test's fixture has no ServerContext, so these rows are the one
+ set of blocks it never sees — assert the backfilled background here or the pane's
+ documented glyph-only hit-test trap re-opens exactly where this test looks. */
+ Assert.All(
+ panel.GetLogicalDescendants().OfType(),
+ b => Assert.NotNull(b.Background));
+ });
+ }
+
///
/// The statement keeps the pane's SQL colouring inside the card — the same highlighter, not a
/// second one — and stays one selectable block, so a drag still takes the whole query (#503).
diff --git a/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt b/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
index e6d8537a..ab8390a1 100644
--- a/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
+++ b/tests/PlanViewer.Core.Tests/ComparisonBaseline.txt
@@ -306,6 +306,22 @@ SELECT * FROM dbo.Users WHERE DisplayName = @d
Estimated rows: 1,000 -> 1,000 (0.0% more)
+##### in_list_dynamic_seek_plan.sqlplan vs in_list_dynamic_seek_plan.sqlplan
+=== Plan Comparison ===
+Plan A: in_list_dynamic_seek_plan.sqlplan
+Plan B: in_list_dynamic_seek_plan.sqlplan
+
+--- Statement 1 ---
+SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 2 -> 2 (0.0% more)
+ Runtime: 7ms -> 7ms (0.0% slower)
+ CPU time: 6ms -> 6ms (0.0% slower)
+ Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
+ DOP: 1 -> 1
+
+
##### isnull_plan.sqlplan vs isnull_plan.sqlplan
=== Plan Comparison ===
Plan A: isnull_plan.sqlplan
@@ -374,6 +390,40 @@ SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.
- HTDELETE 33ms
+##### join_or_expression_plan.sqlplan vs join_or_expression_plan.sqlplan
+=== Plan Comparison ===
+Plan A: join_or_expression_plan.sqlplan
+Plan B: join_or_expression_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X + 1 OR t.A = o.Y + 1
+
+ Estimated cost: 0.0428 -> 0.0428 (0.0% costlier)
+ Estimated rows: 300 -> 300 (0.0% more)
+ Runtime: 3ms -> 3ms (0.0% slower)
+ CPU time: 3ms -> 3ms (0.0% slower)
+ Logical reads: 804 -> 804 (0.0% more)
+ Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
+ DOP: 1 -> 1
+
+
+##### join_or_mixed_parameter_plan.sqlplan vs join_or_mixed_parameter_plan.sqlplan
+=== Plan Comparison ===
+Plan A: join_or_mixed_parameter_plan.sqlplan
+Plan B: join_or_mixed_parameter_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X OR t.A = 5
+
+ Estimated cost: 0.0464 -> 0.0464 (0.0% costlier)
+ Estimated rows: 1,000 -> 1,000 (0.0% more)
+ Runtime: 3ms -> 3ms (0.0% slower)
+ CPU time: 3ms -> 3ms (0.0% slower)
+ Logical reads: 800 -> 800 (0.0% more)
+ Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
+ DOP: 1 -> 1
+
+
##### key_lookup_plan.sqlplan vs key_lookup_plan.sqlplan
=== Plan Comparison ===
Plan A: key_lookup_plan.sqlplan
@@ -641,6 +691,21 @@ UPDATE [dbo].[Users] set [Age] = 138 WHERE [Id]=22656
DOP: 1 -> 1
+##### non_sargable_compound_predicate_plan.sqlplan vs non_sargable_compound_predicate_plan.sqlplan
+=== Plan Comparison ===
+Plan A: non_sargable_compound_predicate_plan.sqlplan
+Plan B: non_sargable_compound_predicate_plan.sqlplan
+
+--- Statement 1 ---
+SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Runtime: 8ms -> 8ms (0.0% slower)
+ CPU time: 7ms -> 7ms (0.0% slower)
+ DOP: 1 -> 1
+
+
##### non_sargable_function_plan.sqlplan vs non_sargable_function_plan.sqlplan
=== Plan Comparison ===
Plan A: non_sargable_function_plan.sqlplan
@@ -687,6 +752,163 @@ SELECT * FROM dbo.Users WHERE Reputation > @rep OPTION (OPTIMIZE FOR UNKNOWN)
Warnings: 1 -> 1 (no change)
+##### outer_reference_function_aliased_plan.sqlplan vs outer_reference_function_aliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_aliased_plan.sqlplan
+Plan B: outer_reference_function_aliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.X = ABS(o.a)) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_on_scanned_column_plan.sqlplan vs outer_reference_function_on_scanned_column_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_on_scanned_column_plan.sqlplan
+Plan B: outer_reference_function_on_scanned_column_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE ABS(i.X) = o.a) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_self_join_plan.sqlplan vs outer_reference_function_self_join_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_self_join_plan.sqlplan
+Plan B: outer_reference_function_self_join_plan.sqlplan
+
+--- Statement 1 ---
+SELECT i1.X, x.X FROM dbo.I AS i1 CROSS APPLY (SELECT TOP (1) i2.X FROM dbo.I AS i2 WHERE i2.X = ABS(i1.X)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 2 -> 2 (0.0% more)
+ Logical reads: 3 -> 3 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_unaliased_plan.sqlplan vs outer_reference_function_unaliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_unaliased_plan.sqlplan
+Plan B: outer_reference_function_unaliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT O.a, x.X FROM dbo.O CROSS APPLY (SELECT TOP (1) I.X FROM dbo.I WHERE I.X = ABS(O.a)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_implicit_conversion_plan.sqlplan vs outer_reference_implicit_conversion_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_implicit_conversion_plan.sqlplan
+Plan B: outer_reference_implicit_conversion_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.S = o.N) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_isnull_plan.sqlplan vs outer_reference_isnull_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_isnull_plan.sqlplan
+Plan B: outer_reference_isnull_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.X = ISNULL(o.a, 0)) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_outer_side_scan_plan.sqlplan vs outer_reference_outer_side_scan_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_outer_side_scan_plan.sqlplan
+Plan B: outer_reference_outer_side_scan_plan.sqlplan
+
+--- Statement 1 ---
+SELECT A, x.Y FROM @a CROSS APPLY (SELECT TOP (1) Y FROM @b WHERE Y = A) AS x WHERE ABS(A) = 1
+
+ Estimated cost: 0.0066 -> 0.0066 (0.0% costlier)
+ Estimated rows: 2 -> 2 (0.0% more)
+ Logical reads: 2 -> 2 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### outer_reference_self_join_unaliased_inner_plan.sqlplan vs outer_reference_self_join_unaliased_inner_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_self_join_unaliased_inner_plan.sqlplan
+Plan B: outer_reference_self_join_unaliased_inner_plan.sqlplan
+
+--- Statement 1 ---
+SELECT i1.X, x.X FROM dbo.I AS i1 CROSS APPLY (SELECT TOP (1) I.X FROM dbo.I WHERE I.X = ABS(i1.X)) AS x
+
+ Estimated cost: 0.0068 -> 0.0068 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_table_variable_aliased_plan.sqlplan vs outer_reference_table_variable_aliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_table_variable_aliased_plan.sqlplan
+Plan B: outer_reference_table_variable_aliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT a.A, x.Y FROM @a AS a CROSS APPLY (SELECT TOP (1) b.Y FROM @b AS b WHERE b.Y = ABS(a.A)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### outer_reference_table_variable_unaliased_plan.sqlplan vs outer_reference_table_variable_unaliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_table_variable_unaliased_plan.sqlplan
+Plan B: outer_reference_table_variable_unaliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT A, x.Y FROM @a CROSS APPLY (SELECT TOP (1) Y FROM @b WHERE Y = ABS(A)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### outer_reference_temp_table_plan.sqlplan vs outer_reference_temp_table_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_temp_table_plan.sqlplan
+Plan B: outer_reference_temp_table_plan.sqlplan
+
+--- Statement 1 ---
+SELECT #t.a, x.X FROM #t CROSS APPLY (SELECT TOP (1) #u.X FROM #u WHERE #u.X = ABS(#t.a)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
##### parallel-skew.sqlplan vs parallel-skew.sqlplan
=== Plan Comparison ===
Plan A: parallel-skew.sqlplan
@@ -972,6 +1194,36 @@ SELECT u.DisplayName, p.Score , p.Title , p.Tags , p.Bo
- SOS_SCHEDULER_YIELD 83ms
+##### table_variable_aliased_function_plan.sqlplan vs table_variable_aliased_function_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_aliased_function_plan.sqlplan
+Plan B: table_variable_aliased_function_plan.sqlplan
+
+--- Statement 1 ---
+SELECT v.X FROM @tv AS v WHERE ABS(v.X) = 1
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Logical reads: 1 -> 1 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### table_variable_implicit_conversion_plan.sqlplan vs table_variable_implicit_conversion_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_implicit_conversion_plan.sqlplan
+Plan B: table_variable_implicit_conversion_plan.sqlplan
+
+--- Statement 1 ---
+SELECT X FROM @tv WHERE S = N'a'
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Logical reads: 1 -> 1 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
##### table_variable_plan.sqlplan vs table_variable_plan.sqlplan
=== Plan Comparison ===
Plan A: table_variable_plan.sqlplan
@@ -987,6 +1239,23 @@ SELECT t.Id FROM @t AS t
Warnings: 1 -> 1 (no change)
+##### table_variable_unaliased_function_plan.sqlplan vs table_variable_unaliased_function_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_unaliased_function_plan.sqlplan
+Plan B: table_variable_unaliased_function_plan.sqlplan
+
+--- Statement 1 ---
+SELECT X FROM @tv WHERE ABS(X) = 1
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Runtime: 21ms -> 21ms (0.0% slower)
+ CPU time: 21ms -> 21ms (0.0% slower)
+ Logical reads: 1 -> 1 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
##### top_above_scan_plan.sqlplan vs top_above_scan_plan.sqlplan
=== Plan Comparison ===
Plan A: top_above_scan_plan.sqlplan
@@ -1411,22 +1680,40 @@ select COUNT(*) from [TimeCard].[Cards] as [t] where exists (select 1 from [Time
Logical reads: 9 -> 0 (eliminated)
-##### implicit_convert_seek_plan.sqlplan vs isnull_plan.sqlplan
+##### implicit_convert_seek_plan.sqlplan vs in_list_dynamic_seek_plan.sqlplan
=== Plan Comparison ===
Plan A: implicit_convert_seek_plan.sqlplan
-Plan B: isnull_plan.sqlplan
+Plan B: in_list_dynamic_seek_plan.sqlplan
Note: Plan A is an estimated plan. Runtime metrics only available for the actual plan.
--- Statement 1 ---
SELECT * FROM dbo.Users WHERE DisplayName = @d
- Estimated cost: 5 -> 3,119.42 (9,999% costlier)
- Estimated rows: 1,000 -> 1 (99.9% fewer)
- Runtime: N/A -> 6.6s
- CPU time: N/A -> 5.7s
+ Estimated cost: 5 -> 0.0033 (99.9% cheaper)
+ Estimated rows: 1,000 -> 2 (99.8% fewer)
+ Runtime: N/A -> 7ms
+ CPU time: N/A -> 6ms
+ Memory grant: 0.0 MB -> 1.0 MB (new)
+ DOP: 0 -> 1
+
+
+##### in_list_dynamic_seek_plan.sqlplan vs isnull_plan.sqlplan
+=== Plan Comparison ===
+Plan A: in_list_dynamic_seek_plan.sqlplan
+Plan B: isnull_plan.sqlplan
+
+--- Statement 1 ---
+SELECT t.Id, t.A FROM dbo.T AS t WHERE t.A IN (10, 20)
+
+ Estimated cost: 0.0033 -> 3,119.42 (9,999% costlier)
+ Estimated rows: 2 -> 1 (50.0% fewer)
+ Runtime: 7ms -> 6.6s (9,999% slower)
+ CPU time: 6ms -> 5.7s (9,999% slower)
Logical reads: 0 -> 4,181,158 (new)
Physical reads: 0 -> 404 (new)
+ Memory grant: 1.0 MB -> 0.0 MB (eliminated)
+ DOP: 1 -> 0
Warnings: 0 -> 4 (4 new)
Wait stats:
@@ -1471,21 +1758,21 @@ SELECT COUNT(*) FROM dbo.Posts AS p WHERE ISNULL(p.LastEditorDisplayName, '') =
- HTDELETE 33ms
-##### join_or_clause_plan.sqlplan vs key_lookup_plan.sqlplan
+##### join_or_clause_plan.sqlplan vs join_or_expression_plan.sqlplan
=== Plan Comparison ===
Plan A: join_or_clause_plan.sqlplan
-Plan B: key_lookup_plan.sqlplan
+Plan B: join_or_expression_plan.sqlplan
--- Statement 1 ---
SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.Posts AS p ON u.Id = p.OwnerUserId OR u.Id = p.LastEditorUserId WHERE p.PostTypeId IN (1, 2) GROUP BY u.Id HAVING MAX(p.Score) >= 5000 ORDER BY MaxScore DESC
- Estimated cost: 3,030.74 -> 0.341 (99.9% cheaper)
- Estimated rows: 135 -> 1 (99.3% fewer)
- Runtime: 33.0s -> 0ms (eliminated)
- CPU time: 3m 48s -> 0ms (eliminated)
- Logical reads: 93,307,943 -> 322 (99.9% fewer)
+ Estimated cost: 3,030.74 -> 0.0428 (99.9% cheaper)
+ Estimated rows: 135 -> 300 (122% more)
+ Runtime: 33.0s -> 3ms (99.9% faster)
+ CPU time: 3m 48s -> 3ms (99.9% faster)
+ Logical reads: 93,307,943 -> 804 (99.9% fewer)
Physical reads: 107,624 -> 0 (eliminated)
- Memory grant: 451.8 MB -> 0.0 MB (eliminated)
+ Memory grant: 451.8 MB -> 1.0 MB (99.8% less)
DOP: 8 -> 1
Warnings: 8 -> 0 (8 resolved)
@@ -1501,6 +1788,40 @@ SELECT u.Id, MaxScore = MAX(p.Score) FROM dbo.Users AS u JOIN dbo.
- HTDELETE 33ms
+##### join_or_expression_plan.sqlplan vs join_or_mixed_parameter_plan.sqlplan
+=== Plan Comparison ===
+Plan A: join_or_expression_plan.sqlplan
+Plan B: join_or_mixed_parameter_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X + 1 OR t.A = o.Y + 1
+
+ Estimated cost: 0.0428 -> 0.0464 (8.6% costlier)
+ Estimated rows: 300 -> 1,000 (233% more)
+ Runtime: 3ms -> 3ms (0.0% slower)
+ CPU time: 3ms -> 3ms (0.0% slower)
+ Logical reads: 804 -> 800 (0.5% fewer)
+ Memory grant: 1.0 MB -> 1.0 MB (0.0% more)
+ DOP: 1 -> 1
+
+
+##### join_or_mixed_parameter_plan.sqlplan vs key_lookup_plan.sqlplan
+=== Plan Comparison ===
+Plan A: join_or_mixed_parameter_plan.sqlplan
+Plan B: key_lookup_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.Id, t.Id FROM dbo.O AS o JOIN dbo.T AS t ON t.A = o.X OR t.A = 5
+
+ Estimated cost: 0.0464 -> 0.341 (634% costlier)
+ Estimated rows: 1,000 -> 1 (99.9% fewer)
+ Runtime: 3ms -> 0ms (eliminated)
+ CPU time: 3ms -> 0ms (eliminated)
+ Logical reads: 800 -> 322 (59.8% fewer)
+ Memory grant: 1.0 MB -> 0.0 MB (eliminated)
+ DOP: 1 -> 1
+
+
##### key_lookup_plan.sqlplan vs lazy_spool_plan.sqlplan
=== Plan Comparison ===
Plan A: key_lookup_plan.sqlplan
@@ -1765,19 +2086,35 @@ INSERT INTO [dbo].[Users]([AboutMe],[Age],[CreationDate],[DisplayName],[DownVote
DOP: 1 -> 1
-##### multi_index_update_plan.sqlplan vs non_sargable_function_plan.sqlplan
+##### multi_index_update_plan.sqlplan vs non_sargable_compound_predicate_plan.sqlplan
=== Plan Comparison ===
Plan A: multi_index_update_plan.sqlplan
-Plan B: non_sargable_function_plan.sqlplan
+Plan B: non_sargable_compound_predicate_plan.sqlplan
--- Statement 1 ---
UPDATE [dbo].[Users] set [Age] = 138 WHERE [Id]=22656
- Estimated cost: 0.0633 -> 3,097.52 (9,999% costlier)
+ Estimated cost: 0.0633 -> 0.0033 (94.8% cheaper)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Runtime: 1ms -> 8ms (700% slower)
+ CPU time: 1ms -> 7ms (600% slower)
+ Logical reads: 3 -> 0 (eliminated)
+ DOP: 1 -> 1
+
+
+##### non_sargable_compound_predicate_plan.sqlplan vs non_sargable_function_plan.sqlplan
+=== Plan Comparison ===
+Plan A: non_sargable_compound_predicate_plan.sqlplan
+Plan B: non_sargable_function_plan.sqlplan
+
+--- Statement 1 ---
+SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52 AND [t].[B]=CONVERT([tinyint],2)
+
+ Estimated cost: 0.0033 -> 3,097.52 (9,999% costlier)
Estimated rows: 1 -> 1 (0.0% more)
- Runtime: 1ms -> 725ms (9,999% slower)
- CPU time: 1ms -> 5.8s (9,999% slower)
- Logical reads: 3 -> 4,226,624 (9,999% more)
+ Runtime: 8ms -> 725ms (8,962% slower)
+ CPU time: 7ms -> 5.8s (9,999% slower)
+ Logical reads: 0 -> 4,226,624 (new)
Memory grant: 0.0 MB -> 24.2 MB (new)
DOP: 1 -> 8
Warnings: 0 -> 5 (5 new)
@@ -1819,24 +2156,186 @@ SELECT COUNT(*) FROM dbo.Posts WHERE YEAR(CreationDate) = 2013
- HTDELETE 1ms
-##### optimize_for_unknown_plan.sqlplan vs parallel-skew.sqlplan
+##### optimize_for_unknown_plan.sqlplan vs outer_reference_function_aliased_plan.sqlplan
=== Plan Comparison ===
Plan A: optimize_for_unknown_plan.sqlplan
-Plan B: parallel-skew.sqlplan
+Plan B: outer_reference_function_aliased_plan.sqlplan
Note: Plan A is an estimated plan. Runtime metrics only available for the actual plan.
--- Statement 1 ---
SELECT * FROM dbo.Users WHERE Reputation > @rep OPTION (OPTIMIZE FOR UNKNOWN)
- Estimated cost: 1 -> 6,729.7 (9,999% costlier)
- Estimated rows: 1,000 -> 2,982,900 (9,999% more)
- Runtime: N/A -> 9.4s
- CPU time: N/A -> 28.6s
- Logical reads: 0 -> 4,947,424 (new)
+ Estimated cost: 1 -> 0.0067 (99.3% cheaper)
+ Estimated rows: 1,000 -> 3 (99.7% fewer)
+ Runtime: N/A -> 0ms
+ CPU time: N/A -> 0ms
+ Logical reads: 0 -> 4 (new)
+ DOP: 0 -> 1
+ Warnings: 1 -> 0 (1 resolved)
+
+
+##### outer_reference_function_aliased_plan.sqlplan vs outer_reference_function_on_scanned_column_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_aliased_plan.sqlplan
+Plan B: outer_reference_function_on_scanned_column_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.X = ABS(o.a)) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_on_scanned_column_plan.sqlplan vs outer_reference_function_self_join_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_on_scanned_column_plan.sqlplan
+Plan B: outer_reference_function_self_join_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE ABS(i.X) = o.a) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (1.3% cheaper)
+ Estimated rows: 3 -> 2 (33.3% fewer)
+ Logical reads: 4 -> 3 (25.0% fewer)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_self_join_plan.sqlplan vs outer_reference_function_unaliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_self_join_plan.sqlplan
+Plan B: outer_reference_function_unaliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT i1.X, x.X FROM dbo.I AS i1 CROSS APPLY (SELECT TOP (1) i2.X FROM dbo.I AS i2 WHERE i2.X = ABS(i1.X)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (1.3% costlier)
+ Estimated rows: 2 -> 3 (50.0% more)
+ Logical reads: 3 -> 4 (33.3% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_function_unaliased_plan.sqlplan vs outer_reference_implicit_conversion_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_function_unaliased_plan.sqlplan
+Plan B: outer_reference_implicit_conversion_plan.sqlplan
+
+--- Statement 1 ---
+SELECT O.a, x.X FROM dbo.O CROSS APPLY (SELECT TOP (1) I.X FROM dbo.I WHERE I.X = ABS(O.a)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_implicit_conversion_plan.sqlplan vs outer_reference_isnull_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_implicit_conversion_plan.sqlplan
+Plan B: outer_reference_isnull_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.S = o.N) AS i
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+
+
+##### outer_reference_isnull_plan.sqlplan vs outer_reference_outer_side_scan_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_isnull_plan.sqlplan
+Plan B: outer_reference_outer_side_scan_plan.sqlplan
+
+--- Statement 1 ---
+SELECT o.a, i.X FROM dbo.O AS o CROSS APPLY (SELECT TOP (1) i.X FROM dbo.I AS i WHERE i.X = ISNULL(o.a, 0)) AS i
+
+ Estimated cost: 0.0067 -> 0.0066 (1.6% cheaper)
+ Estimated rows: 3 -> 2 (42.3% fewer)
+ Logical reads: 4 -> 2 (50.0% fewer)
+ DOP: 1 -> 1
+ Warnings: 0 -> 1 (1 new)
+
+
+##### outer_reference_outer_side_scan_plan.sqlplan vs outer_reference_self_join_unaliased_inner_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_outer_side_scan_plan.sqlplan
+Plan B: outer_reference_self_join_unaliased_inner_plan.sqlplan
+
+--- Statement 1 ---
+SELECT A, x.Y FROM @a CROSS APPLY (SELECT TOP (1) Y FROM @b WHERE Y = A) AS x WHERE ABS(A) = 1
+
+ Estimated cost: 0.0066 -> 0.0068 (1.7% costlier)
+ Estimated rows: 2 -> 3 (73.2% more)
+ Logical reads: 2 -> 4 (100% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 0 (1 resolved)
+
+
+##### outer_reference_self_join_unaliased_inner_plan.sqlplan vs outer_reference_table_variable_aliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_self_join_unaliased_inner_plan.sqlplan
+Plan B: outer_reference_table_variable_aliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT i1.X, x.X FROM dbo.I AS i1 CROSS APPLY (SELECT TOP (1) I.X FROM dbo.I WHERE I.X = ABS(i1.X)) AS x
+
+ Estimated cost: 0.0068 -> 0.0067 (0.1% cheaper)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 0 -> 1 (1 new)
+
+
+##### outer_reference_table_variable_aliased_plan.sqlplan vs outer_reference_table_variable_unaliased_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_table_variable_aliased_plan.sqlplan
+Plan B: outer_reference_table_variable_unaliased_plan.sqlplan
+
+--- Statement 1 ---
+SELECT a.A, x.Y FROM @a AS a CROSS APPLY (SELECT TOP (1) b.Y FROM @b AS b WHERE b.Y = ABS(a.A)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### outer_reference_table_variable_unaliased_plan.sqlplan vs outer_reference_temp_table_plan.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_table_variable_unaliased_plan.sqlplan
+Plan B: outer_reference_temp_table_plan.sqlplan
+
+--- Statement 1 ---
+SELECT A, x.Y FROM @a CROSS APPLY (SELECT TOP (1) Y FROM @b WHERE Y = ABS(A)) AS x
+
+ Estimated cost: 0.0067 -> 0.0067 (0.0% costlier)
+ Estimated rows: 3 -> 3 (0.0% more)
+ Logical reads: 4 -> 4 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 0 (1 resolved)
+
+
+##### outer_reference_temp_table_plan.sqlplan vs parallel-skew.sqlplan
+=== Plan Comparison ===
+Plan A: outer_reference_temp_table_plan.sqlplan
+Plan B: parallel-skew.sqlplan
+
+--- Statement 1 ---
+SELECT #t.a, x.X FROM #t CROSS APPLY (SELECT TOP (1) #u.X FROM #u WHERE #u.X = ABS(#t.a)) AS x
+
+ Estimated cost: 0.0067 -> 6,729.7 (9,999% costlier)
+ Estimated rows: 3 -> 2,982,900 (9,999% more)
+ Runtime: 0ms -> 9.4s (new)
+ CPU time: 0ms -> 28.6s (new)
+ Logical reads: 4 -> 4,947,424 (9,999% more)
Memory grant: 0.0 MB -> 360.2 MB (new)
- DOP: 0 -> 4
- Warnings: 1 -> 11 (10 new)
+ DOP: 1 -> 4
+ Warnings: 0 -> 11 (11 new)
Missing indexes: 0 -> 1 (1 new)
Wait stats:
@@ -2123,19 +2622,19 @@ SELECT p.Id, p.Score, v.VoteTypeId FROM dbo.Posts AS p CROSS A
- SOS_SCHEDULER_YIELD 83ms
-##### spill_plan.sqlplan vs table_variable_plan.sqlplan
+##### spill_plan.sqlplan vs table_variable_aliased_function_plan.sqlplan
=== Plan Comparison ===
Plan A: spill_plan.sqlplan
-Plan B: table_variable_plan.sqlplan
+Plan B: table_variable_aliased_function_plan.sqlplan
--- Statement 1 ---
SELECT u.DisplayName, p.Score , p.Title , p.Tags , p.Body FROM dbo.Users AS u LEFT JOIN ( SELECT p.*, n = ROW_NUMBER() OVER ( PARTITION BY p.OwnerUserId ORDER BY p.Score DESC ) FROM dbo.Posts AS p WHERE p.PostTypeId = 2 AND p.Score > 0 ) AS p ON p.OwnerUserId = u.Id AND p.n = 1 WHERE u.Reputation >= 20000 ORDER BY ...
Estimated cost: 16,976.5 -> 0.0033 (99.9% cheaper)
- Estimated rows: 482,677 -> 3 (99.9% fewer)
+ Estimated rows: 482,677 -> 1 (99.9% fewer)
Runtime: 26.8s -> 0ms (eliminated)
CPU time: 1m 16s -> 0ms (eliminated)
- Logical reads: 4,229,245 -> 2 (99.9% fewer)
+ Logical reads: 4,229,245 -> 1 (99.9% fewer)
Memory grant: 10,597.0 MB -> 0.0 MB (eliminated)
DOP: 8 -> 1
Warnings: 12 -> 1 (11 resolved)
@@ -2155,19 +2654,66 @@ SELECT u.DisplayName, p.Score , p.Title , p.Tags , p.Bo
- SOS_SCHEDULER_YIELD 83ms
-##### table_variable_plan.sqlplan vs top_above_scan_plan.sqlplan
+##### table_variable_aliased_function_plan.sqlplan vs table_variable_implicit_conversion_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_aliased_function_plan.sqlplan
+Plan B: table_variable_implicit_conversion_plan.sqlplan
+
+--- Statement 1 ---
+SELECT v.X FROM @tv AS v WHERE ABS(v.X) = 1
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Logical reads: 1 -> 1 (0.0% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### table_variable_implicit_conversion_plan.sqlplan vs table_variable_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_implicit_conversion_plan.sqlplan
+Plan B: table_variable_plan.sqlplan
+
+--- Statement 1 ---
+SELECT X FROM @tv WHERE S = N'a'
+
+ Estimated cost: 0.0033 -> 0.0033 (0.0% costlier)
+ Estimated rows: 1 -> 3 (200% more)
+ Logical reads: 1 -> 2 (100% more)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### table_variable_plan.sqlplan vs table_variable_unaliased_function_plan.sqlplan
=== Plan Comparison ===
Plan A: table_variable_plan.sqlplan
-Plan B: top_above_scan_plan.sqlplan
+Plan B: table_variable_unaliased_function_plan.sqlplan
--- Statement 1 ---
SELECT t.Id FROM @t AS t
- Estimated cost: 0.0033 -> 4.8408 (9,999% costlier)
+ Estimated cost: 0.0033 -> 0.0033 (0.0% cheaper)
Estimated rows: 3 -> 1 (66.7% fewer)
- Runtime: 0ms -> 5.1s (new)
- CPU time: 0ms -> 5.1s (new)
- Logical reads: 2 -> 88,341 (9,999% more)
+ Runtime: 0ms -> 21ms (new)
+ CPU time: 0ms -> 21ms (new)
+ Logical reads: 2 -> 1 (50.0% fewer)
+ DOP: 1 -> 1
+ Warnings: 1 -> 1 (no change)
+
+
+##### table_variable_unaliased_function_plan.sqlplan vs top_above_scan_plan.sqlplan
+=== Plan Comparison ===
+Plan A: table_variable_unaliased_function_plan.sqlplan
+Plan B: top_above_scan_plan.sqlplan
+
+--- Statement 1 ---
+SELECT X FROM @tv WHERE ABS(X) = 1
+
+ Estimated cost: 0.0033 -> 4.8408 (9,999% costlier)
+ Estimated rows: 1 -> 1 (0.0% more)
+ Runtime: 21ms -> 5.1s (9,999% slower)
+ CPU time: 21ms -> 5.1s (9,999% slower)
+ Logical reads: 1 -> 88,341 (9,999% more)
DOP: 1 -> 1
Warnings: 1 -> 1 (no change)
Missing indexes: 0 -> 1 (1 new)
diff --git a/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs b/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs
index 84a27835..cda6d2a9 100644
--- a/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs
+++ b/tests/PlanViewer.Core.Tests/EmptyQueryTabStateTests.cs
@@ -71,6 +71,34 @@ public void AnOpenPlanKeepsTheEmptyStateAwayEvenWithAnEmptyEditor()
});
}
+ ///
+ /// #540: connect to a server from a fresh tab and the overlay stayed, opaque over the
+ /// editor, still offering "Connect to a server" — the app looked broken, because nothing
+ /// else the user would think to do (they came to type a query, not open a plan) would
+ /// take it down. A connected session is in use even with nothing typed: the editor is
+ /// the offer now, and it stays the offer when the buffer is emptied back out.
+ ///
+ [Fact]
+ public void AConnectedSessionNeverShowsTheEmptyState_EvenWithAnEmptiedBuffer()
+ {
+ HeadlessUi.Run(() =>
+ {
+ var window = new MainWindow();
+ var session = NewSession(window);
+
+ Assert.True(Overlay(session).IsVisible, "a fresh tab, before the connection");
+
+ SessionHarness.PretendConnected(session);
+ SessionHarness.RefreshEmptyState(session);
+ Assert.False(Overlay(session).IsVisible, "this session has a server to run queries on");
+
+ session.QueryEditor.Text = "select 1;";
+ session.QueryEditor.Text = "";
+ Assert.False(Overlay(session).IsVisible,
+ "deleting every character on a connected session must not cover the editor again");
+ });
+ }
+
///
/// Every row is clickable across its whole width, not just where its glyphs happen to fall.
/// A control with a null background hit-tests only what it draws, so a press in the gap
diff --git a/tests/PlanViewer.Core.Tests/HeadlessUi.cs b/tests/PlanViewer.Core.Tests/HeadlessUi.cs
index e7308d41..a7df54ab 100644
--- a/tests/PlanViewer.Core.Tests/HeadlessUi.cs
+++ b/tests/PlanViewer.Core.Tests/HeadlessUi.cs
@@ -16,11 +16,47 @@ namespace PlanViewer.Core.Tests;
///
/// A headless Avalonia session, so UI code can be tested without a display.
///
-/// Why this is hand-rolled rather than Avalonia.Headless.XUnit. That package exists and
-/// would be less code, but at 11.3.20 it depends on xunit.core 2.4.0 — xunit v2 — and this
-/// suite runs on xunit.v3. Putting two xunit frameworks in one test project to get an attribute is a
-/// worse trade than owning fifteen lines. is runner-agnostic
-/// and is what that package wraps anyway.
+/// Why this is hand-rolled rather than Avalonia.Headless.XUnit. The original objection
+/// has expired and the conclusion has not. That package used to depend on xunit.core 2.4.0 —
+/// xunit v2 — against a suite running xunit.v3, so adopting it meant two xunit frameworks in one
+/// project. At 12.1.2 it asks for xunit.v3.extensibility.core 3.2.2, which unifies upward
+/// against this project's own 4.0.1, so it is now merely possible. It is still the wrong trade:
+/// is runner-agnostic and is what that package wraps anyway,
+/// while [AvaloniaFact] offers neither of the two behaviours this class exists for — the #474
+/// queue drain and the canary, which is the only reason a
+/// session-poisoning test fails with its own name on it. Rewriting every entry point to lose both
+/// would be a migration dressed as a simplification.
+///
+/// Inter ships here and is never used. Avalonia.Headless 12.1.2 pulls
+/// Avalonia.Fonts.Inter and Avalonia.HarfBuzz in transitively, so the Inter assembly
+/// sits in the test output directory. Nothing registers it: .WithInterFont() lives on
+/// Program.BuildAvaloniaApp, and the session below boots App, which has no such
+/// method, so the builder never runs it. Text in this suite is measured against headless's own
+/// embedded font, not Inter — see the metrics paragraph below.
+///
+/// What text measures, and why the layout numbers in this suite moved. Stated once
+/// here because several files pin measured widths and heights, and a number repeated with its
+/// reason in every file is a number that goes stale in some of them. Headless 11 bound a stub text
+/// shaper that gave every character a flat 10 DIP advance at any font size, over synthetic font
+/// metrics that worked out to a line height of 0.8 em. Headless 12 drops the stub and shapes for
+/// real through HarfBuzz against its own embedded BareMinimum font, which has no glyph for
+/// ordinary text and so measures every character at one em. Both numbers therefore changed, on
+/// different axes: a string is now exactly FontSize DIP per character, so widths scale by
+/// FontSize / 10 — unchanged at font size 10, 1.1x at the toolbar's 11, 1.4x at Fluent's
+/// default 14 — while a line is 1.0898 em tall, rounded up to whole DIPs, which measures as desired
+/// heights of 11, 12, 14 and 16 at font sizes 10, 11, 12 and 14. That is 1.36x the old line height
+/// at every size, a flat ratio rather than something that scales with the size.
+///
+/// The height figure is worth pinning down because the plausible wrong answer is 1.25x. That
+/// would be the em box alone; 1.36x is the em box plus the font's line gap. The font manager
+/// reports this face as ascent 819, descent 205, line gap 92 over an em of 1024 — the OS/2
+/// typographic values, not the hhea pair (782 and 0) that would have given 0.854 em and a 7%
+/// change. So when a height assertion moves, 1.36x is the tell; if a height moves by something
+/// else, the cause is not this.
+///
+/// Padding, margins and fixed sizes did not move at all, so text-driven measurements grew and
+/// everything else stayed put. Thresholds elsewhere in the suite carry their new numbers and point
+/// back here rather than re-deriving this.
///
/// The real App, not a stub. A bare Application looked tidier but does not load the
/// application XAML, and MainWindow's toolbars resolve styles from it — FindResource("AppButton")
@@ -115,30 +151,104 @@ unusable is still described best by the assertion it failed. */
///
/// Runs one body on the UI thread and hands back whatever it threw rather than throwing here,
- /// so can decide which of two failures to report.
+ /// so can decide which of two failures to report. Retries once, and only,
+ /// when Avalonia 12's own session setup lost a race before the body was ever reached — see
+ /// .
+ ///
+ /// The retry, and why it is not a test retry. Avalonia 12's headless session setup
+ /// has a race that fails one arbitrary test per run. Measured on the 12 bump: 2 of 6 full-suite
+ /// runs, 1 of 6 more with the two classes that were red for an unrelated reason excluded, and 1
+ /// of 6 with xunit parallelization disabled outright — so roughly a quarter of runs, a
+ /// different victim each time, and not caused by test concurrency. The same measurement on
+ /// 11.3.22 was 0 of 6. A quarter of CI runs failing on an upstream race nobody can act on is
+ /// not shippable, and there is no fixed Avalonia release to take instead.
+ ///
+ /// What makes this safe is that it does not re-run tests. It re-runs a dispatch whose
+ /// delegate was never entered, which cannot tell apart from the first
+ /// attempt because nothing of it ran. A test that started and then failed — for any reason,
+ /// including a thread-affinity bug of our own — is reported, never retried: that is what the
+ /// started flag in and the narrow match in
+ /// are for, and why each occurrence writes a line to stderr.
+ /// One retry, no more; if the race hits twice in a row, the run goes red and says so.
+ ///
+ /// Remove this when there is an Avalonia release that fixes the race. Delete the
+ /// retry, and the started flag, then run the full suite ten
+ /// times: at the rate above, ten clean runs leave about a 6% chance of having missed it.
///
/// Why the queue is drained before returning (#474). Avalonia's per-dispatch
/// teardown disposes the session's FontManager and only then calls
/// Dispatcher.ResetForUnitTests, which executes whatever is still queued. A window whose
/// content is involved enough to leave a deferred render pass behind — a
/// PlanViewerControl reliably does, a TextBlock does not — therefore renders text against
- /// a font manager that has just been disposed, throws KeyNotFoundException for
- /// fonts:SystemFonts, and that exception escapes the teardown delegate before it reaches
- /// scope.Dispose(). The locator scope is then never popped: every later dispatch nests
- /// inside the leaked one, resolves the disposed font manager through its parent chain, and dies
- /// constructing any at all. The guilty test passes, because the throw
- /// happens after its result has been recorded.
+ /// a font manager that has just been disposed, and throws KeyNotFoundException for
+ /// fonts:SystemFonts. That ordering is unchanged in 12.1.2 — the dispose and the reset are
+ /// still consecutive lines of EnsureIsolatedApplication's teardown — so draining here,
+ /// which leaves the teardown nothing to run, is still the only thing that prevents the throw.
+ /// It is done even when the body failed, because a failing test is no less capable of poisoning
+ /// the session than a passing one.
///
- /// Draining here leaves the teardown nothing to run, which is the whole fix. It is done
- /// even when the body failed, because a failing test is no less capable of poisoning the
- /// session than a passing one.
+ /// What 12 fixed, and why the drain is not now redundant. Under 11.3.22 the
+ /// escaping exception also skipped scope.Dispose(), so the locator scope was never
+ /// popped: every later dispatch nested inside the leaked one, resolved the disposed font manager
+ /// through its parent chain, and died constructing any at all — while the
+ /// guilty test passed, because the throw happened after its result had been recorded. 12 moved
+ /// the scope disposal into a finally and routes the teardown failure into the dispatch's
+ /// task. So the blast radius is now one test instead of every test after it, and the failure
+ /// lands on the test that caused it. That makes #474 survivable, not absent. Deleting the drain
+ /// would trade a prevented failure for a reported one.
///
private static Exception? Dispatch(Action body)
+ {
+ var failure = DispatchOnce(body, out var bodyStarted);
+
+ if (failure is null || bodyStarted || !IsHeadlessSetupRace(failure))
+ {
+ return failure;
+ }
+
+ /* Loud on purpose, every single time. A retry nobody can see is how a 1-in-4 flake becomes
+ a 1-in-400 mystery that outlives everyone who remembers this comment. */
+ Console.Error.WriteLine(
+ "Avalonia 12 headless setup race — dispatch retried once; see issue #544.");
+
+ return DispatchOnce(body, out _);
+ }
+
+ ///
+ /// Whether is Avalonia 12's session-setup race rather than anything
+ /// this repo wrote.
+ ///
+ /// The failure is an from
+ /// Dispatcher.VerifyAccess, thrown while EnsureIsolatedApplication builds the
+ /// per-dispatch Application: AvaloniaHeadlessPlatform.Initialize constructs a
+ /// Compositor, whose DefaultRenderLoop.Add verifies dispatcher access and finds a
+ /// different thread owning it. No repo code appears above in the stack.
+ /// The match is deliberately narrow — the exception type AND one of those two upstream frames —
+ /// so that a thread-affinity bug in our own code, which would name our own frames, is reported
+ /// rather than retried.
+ ///
+ private static bool IsHeadlessSetupRace(Exception failure) =>
+ failure is InvalidOperationException
+ && failure.StackTrace is { } stack
+ && (stack.Contains("EnsureIsolatedApplication", StringComparison.Ordinal)
+ || stack.Contains("AvaloniaHeadlessPlatform.Initialize", StringComparison.Ordinal));
+
+ ///
+ /// One attempt. reports whether the dispatched delegate was
+ /// entered at all, which is what makes the retry above safe: it is the difference between
+ /// re-running a test and starting one that never ran.
+ ///
+ private static Exception? DispatchOnce(Action body, out bool bodyStarted)
{
Exception? failure = null;
+ var started = false;
- Session.Value.Dispatch(() =>
+ var dispatch = Session.Value.Dispatch(() =>
{
+ /* First statement in the delegate, before anything that could throw, so that "the body
+ never started" is a fact rather than an inference. */
+ started = true;
+
try
{
body();
@@ -158,8 +268,24 @@ unusable is still described best by the assertion it failed. */
}
return Task.CompletedTask;
- }, default).GetAwaiter().GetResult();
+ }, default);
+
+ /* A teardown failure arrives here rather than in either catch above: 12 reports it through
+ the dispatch's own task, which is awaited outside the delegate. Measured under 12.1.2,
+ not assumed — a queued job that throws during teardown comes out of GetResult(), and when
+ the body had failed too, the teardown exception is the one the caller sees, silently
+ replacing the assertion message. That inverts the rule Run documents, so catch it and let
+ the body's failure keep precedence. */
+ try
+ {
+ dispatch.GetAwaiter().GetResult();
+ }
+ catch (Exception ex)
+ {
+ failure ??= ex;
+ }
+ bodyStarted = started;
return failure;
}
diff --git a/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs b/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
index d6440d6b..7ab6ee89 100644
--- a/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
+++ b/tests/PlanViewer.Core.Tests/ParameterSubstitutionTests.cs
@@ -390,4 +390,70 @@ parameter name survives anywhere in the text. */
Assert.Contains("like 'kexin%'", result.Text);
Assert.DoesNotContain("@", result.Text);
}
+
+ [Fact]
+ public void DeclarationList_IsLeftOutAndNotSubstituted()
+ {
+ /* A plan from the plan cache or Query Store keeps an sp_executesql statement's declaration
+ list in front of it. Substituted like the rest, "(@p1 int, @p2 int)" became
+ "(10 int, 20 int)", which is neither the plan's text nor runnable. */
+ var result = ParameterSubstitution.Apply(
+ "(@p1 int, @p2 int)SELECT t.Id FROM dbo.T AS t WHERE t.A IN (@p1, @p2)",
+ new List { Param("@p1", "(10)"), Param("@p2", "(20)") });
+
+ Assert.Equal("SELECT t.Id FROM dbo.T AS t WHERE t.A IN (10, 20)", result.Text);
+ Assert.Equal(2, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void DeclarationListWithParenthesizedTypes_EndsAtItsOwnClosingParenthesis()
+ {
+ /* The comma inside decimal(18,2) and the parentheses of both types are part of the list.
+ Stopping at the first closing parenthesis would leave ",@b nvarchar(50))" in the text. */
+ var result = ParameterSubstitution.Apply(
+ "(@a decimal(18,2),@b nvarchar(50))SELECT * FROM t WHERE x = @a AND y = @b",
+ new List { Param("@a", "(1.50)"), Param("@b", "N'abc'") });
+
+ Assert.Equal("SELECT * FROM t WHERE x = 1.50 AND y = N'abc'", result.Text);
+ }
+
+ [Fact]
+ public void DeclarationList_StaysWhenNothingIsSubstituted()
+ {
+ /* With no value to put back, the text is shown as the plan recorded it, list included. */
+ const string text = "(@p1 int)SELECT 1";
+ var result = ParameterSubstitution.Apply(text, new List { Param("@p1", "(10)") });
+
+ Assert.Equal(text, result.Text);
+ Assert.Equal(0, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void DeclarationListCutOffByTruncation_IsLeftAsItIs()
+ {
+ /* A plan cuts statement text off at 4,000 characters, and the declarations for a long IN
+ list can fill all of them. Then the text is only declarations, with no statement after
+ them, and putting values into it would make "(1 int,2 int,…" again. */
+ const string text = "(@p0 int,@p1 int,@p2 in";
+ var result = ParameterSubstitution.Apply(
+ text, new List { Param("@p0", "(1)"), Param("@p1", "(2)") });
+
+ Assert.Equal(text, result.Text);
+ Assert.Equal(0, result.SubstitutionCount);
+ }
+
+ [Fact]
+ public void AutoParameterizedPlan_LosesItsDeclarationList()
+ {
+ /* The #556 reproduction, an auto-parameterized plan: its text starts with
+ "(@1 tinyint,@2 int)", and the comparison report printed "(52 tinyint,2 int)SELECT". */
+ var plan = PlanTestHelper.LoadAndAnalyze("non_sargable_compound_predicate_plan.sqlplan");
+ var statement = PlanTestHelper.FirstStatement(plan);
+ Assert.StartsWith("(@1 tinyint,@2 int)", statement.StatementText);
+
+ var result = ParameterSubstitution.Apply(statement.StatementText, statement.Parameters);
+
+ Assert.StartsWith("SELECT COUNT(*) FROM [dbo].[T] [t] WHERE [t].[A]=52", result.Text);
+ Assert.DoesNotContain("@", result.Text);
+ }
}
diff --git a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs
index bf98c4bd..8dd60c32 100644
--- a/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs
+++ b/tests/PlanViewer.Core.Tests/PlanAnalyzerTests.cs
@@ -310,29 +310,557 @@ public void Rule12d_NonSargable_BenignConvertDoesNotMaskFunctionOnColumn()
// ---------------------------------------------------------------
///
- /// A table-variable COLUMN renders as [@tv].[col] in a ScalarString — the @ belongs to the
- /// table's name, not to a scalar variable. The old column pattern excluded @ from the first
- /// bracket part to keep [@p] out, which also kept [@tv].[col] out: a genuine column-side
- /// CONVERT_IMPLICIT on one lost its Non-SARGable warning. The "].[" sequence is what a bare
- /// variable can never have, so it alone draws the line.
+ /// Builds a for these string-level tests, without loading a fixture
+ /// plan. defaults to none, matching a scan with no
+ /// Nested Loops ancestor passing it anything (#561, #564).
+ ///
+ private static ScanIdentity Identity(string? alias, string? table, bool isTableVariable = false,
+ params string[] bareOuterReferences) =>
+ new(alias, table, isTableVariable, new HashSet(bareOuterReferences, StringComparer.OrdinalIgnoreCase));
+
+ ///
+ /// An ALIASED table-variable column renders dotted, through the alias: SELECT v.X FROM @tv AS
+ /// v WHERE ABS(v.X) = 1 gives "abs(@tv.[X] as [v].[X])=(1)". Confirmed on SQL Server 2016,
+ /// 2017, 2019, 2022 and 2025 — no version renders [@tv].[col], and that shape never occurs.
+ /// The "].[" sequence through the alias is what a bare parameter can never have, so
+ /// ColumnReferenceRegex catches this case on its own, with no identity needed.
///
[Fact]
- public void Rule12f_NonSargable_TableVariableColumnConversion_IsFlagged()
+ public void Rule12f_NonSargable_AliasedTableVariableColumnConversion_IsFlagged()
{
Assert.True(PlanAnalyzer.ConvertImplicitWrapsColumn(
- "CONVERT_IMPLICIT(nvarchar(40),[@tv].[col],0)=[@p]"));
+ "CONVERT_IMPLICIT(nvarchar(40),@tv.[col] as [v].[col],0)=[@p]"));
}
///
- /// The parameter-side mirror of the case above, and the #436 rule restated: converting the
- /// parameter up to the column's type costs nothing, table variable or not, so widening the
- /// column pattern must not start flagging it.
+ /// An UNALIASED table-variable column has no dotted qualifier at all: SELECT X FROM @tv WHERE
+ /// S = @n (S varchar, @n nvarchar) gives "CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n]". Bare
+ /// [S] cannot match ColumnReferenceRegex, so the caller must identify this scan as an
+ /// unaliased table variable before a bare name is read as a column (#561).
///
[Fact]
- public void Rule12f_NonSargable_TableVariableParameterSideConversion_IsNotFlagged()
+ public void Rule12f_NonSargable_UnaliasedTableVariableColumnConversion_IsFlaggedWithTableVariableFlag()
+ {
+ Assert.True(PlanAnalyzer.ConvertImplicitWrapsColumn(
+ "CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n]", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ ///
+ /// The same bare-name shape, off a scan the caller has not identified at all.
+ /// [S] alone could just as easily be a parameter or an expression, so it is not read as a
+ /// column without an identity.
+ ///
+ [Fact]
+ public void Rule12f_NonSargable_UnaliasedTableVariableColumnConversion_NotFlaggedWithoutTableVariableFlag()
{
Assert.False(PlanAnalyzer.ConvertImplicitWrapsColumn(
- "[@tv].[col]=CONVERT_IMPLICIT(nvarchar(40),[@p],0)"));
+ "CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n]"));
+ }
+
+ ///
+ /// The parameter-side mirror, and the #436 rule restated: converting the parameter up to the
+ /// column's type costs nothing, bare unaliased table-variable column or not.
+ ///
+ [Fact]
+ public void Rule12f_NonSargable_UnaliasedTableVariableParameterSideConversion_IsNotFlagged()
+ {
+ Assert.False(PlanAnalyzer.ConvertImplicitWrapsColumn(
+ "[S]=CONVERT_IMPLICIT(nvarchar(20),[@n],0)", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ // ---------------------------------------------------------------
+ // Rule 12: Non-SARGable Predicate — bare columns on a table variable scan (#561)
+ // ---------------------------------------------------------------
+
+ [Fact]
+ public void Rule12h_NonSargable_BareColumnFunction_IsFlaggedWithTableVariableFlag()
+ {
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([X])=(1)", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ [Fact]
+ public void Rule12h_NonSargable_BareColumnImplicitConversion_IsFlaggedWithTableVariableFlag()
+ {
+ Assert.Equal("Implicit conversion (CONVERT_IMPLICIT)", PlanAnalyzer.DetectNonSargablePattern(
+ "CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n]", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ [Fact]
+ public void Rule12h_NonSargable_FunctionOnParameterSide_NotFlaggedEvenWithTableVariableFlag()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[X]=abs([@i])", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ ///
+ /// [Expr1003] is an optimizer-generated expression name, not a column of the table variable.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_FunctionOnExpressionColumn_NotFlaggedEvenWithTableVariableFlag()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "abs([Expr1003])=(1)", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ ///
+ /// '[Y]' is a string literal that happens to look like a bracketed name. The literal
+ /// alternative in BracketedNameRegex has to win before the name group is even tried.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_FunctionOnStringLiteral_NotFlaggedEvenWithTableVariableFlag()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[X]=upper('[Y]')", Identity(alias: null, table: "@tv", isTableVariable: true)));
+ }
+
+ [Fact]
+ public void Rule12h_NonSargable_BareColumnFunction_NotFlaggedWithoutTableVariableFlag()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern("abs([X])=(1)"));
+ }
+
+ [Fact]
+ public void Rule12h_NonSargable_UnaliasedFunctionPlan_GetsNonSargableNotScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("table_variable_unaliased_function_plan.sqlplan");
+
+ var nonSargable = PlanTestHelper.WarningsOfType(plan, "Non-SARGable Predicate");
+ Assert.Single(nonSargable);
+ Assert.Contains("Function call (ABS)", nonSargable[0].Message);
+ Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Scan With Predicate"));
+ }
+
+ [Fact]
+ public void Rule12h_NonSargable_ImplicitConversionPlan_GetsNonSargableNotScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("table_variable_implicit_conversion_plan.sqlplan");
+
+ var nonSargable = PlanTestHelper.WarningsOfType(plan, "Non-SARGable Predicate");
+ Assert.Single(nonSargable);
+ Assert.Contains("Implicit conversion (CONVERT_IMPLICIT)", nonSargable[0].Message);
+ Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Scan With Predicate"));
+ }
+
+ ///
+ /// The aliased case already worked before #561, since the alias makes the column dotted
+ /// ("abs(@tv.[X] as [v].[X])=(1)"). This fixture's warnings must not change.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_AliasedFunctionPlan_UnchangedByTableVariableFlag()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("table_variable_aliased_function_plan.sqlplan");
+
+ var nonSargable = PlanTestHelper.WarningsOfType(plan, "Non-SARGable Predicate");
+ Assert.Single(nonSargable);
+ Assert.Contains("Function call (ABS)", nonSargable[0].Message);
+ Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Scan With Predicate"));
+ }
+
+ // ---------------------------------------------------------------
+ // Rule 12: Non-SARGable Predicate — outer references (#564)
+ // ---------------------------------------------------------------
+ //
+ // A Nested Loops join passes an outer reference to its inner input one row at a time, so a
+ // function or conversion wrapping only an outer reference never costs the scanned table a seek
+ // — the value was already going to be re-evaluated every row regardless. These test
+ // IsColumnReference's ownership check directly, with a hand-built ScanIdentity. The fixture
+ // tests further down cover CollectBareOuterReferences, the tree walk that finds bare outer
+ // references off a real plan, which a hand-built identity bypasses.
+
+ ///
+ /// A self join's other instance of the same table: the table name "I" matches, but its alias is
+ /// "o", not this scan's own "i". Ownership has to be decided by alias, not table, or a self join
+ /// could never tell its own column from the other instance's.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_AliasedOtherInstanceOfSameTable_NotFlagged()
+ {
+ var identity = Identity(alias: "i", table: "I");
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[I].[X] as [o].[X])=(1)", identity));
+ }
+
+ ///
+ /// Deliberately different strings for alias and table, so confusing the aliased form's owner
+ /// check with the unaliased form's changes the answer — a self join's alias and table are
+ /// usually just different letter-casings of each other and cannot tell the two checks apart.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_AliasedForm_ComparesAgainstAliasNotTable()
+ {
+ var identity = Identity(alias: "i", table: "SomeOtherTable");
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[SomeOtherTable].[X] as [i].[X])=(1)", identity));
+ }
+
+ ///
+ /// The part of an aliased reference before " as " is not a reference of its own — here it names
+ /// the same table this unaliased scan reads, which would otherwise false-positive as this
+ /// scan's own column through the ordinary unaliased-dotted-form check.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_PrefixBeforeAs_IsNotItsOwnReference()
+ {
+ var identity = Identity(alias: null, table: "I");
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[I].[X] as [i1].[X])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_UnaliasedOwnColumn_IsFlagged()
+ {
+ var identity = Identity(alias: null, table: "I");
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[I].[X])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_UnaliasedAnotherTable_NotFlagged()
+ {
+ var identity = Identity(alias: null, table: "I");
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[O].[a])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_TempTableOwnColumn_IsFlagged()
+ {
+ var identity = Identity(alias: null, table: "#u");
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([#u].[X])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_TempTableAnotherTempTable_NotFlagged()
+ {
+ var identity = Identity(alias: null, table: "#u");
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "abs([#t].[a])=(1)", identity));
+ }
+
+ ///
+ /// A plan can name a temp table by its full tempdb name: the name, underscores, then a hex
+ /// suffix. The parser cleans the scan's own name to #u, so the predicate's name must be
+ /// cleaned the same way before the two are compared.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_TempTableOwnColumnWithFullTempdbName_IsFlagged()
+ {
+ var identity = Identity(alias: null, table: "#u");
+ var fullName = "#u" + new string('_', 110) + "000000000004";
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ $"abs([tempdb].[dbo].[{fullName}].[X])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_TempTableAnotherTempTableWithFullTempdbName_NotFlagged()
+ {
+ var identity = Identity(alias: null, table: "#u");
+ var fullName = "#t" + new string('_', 110) + "000000000003";
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ $"abs([tempdb].[dbo].[{fullName}].[a])=(1)", identity));
+ }
+
+ [Fact]
+ public void Rule12i_NonSargable_UnaliasedTableVariableBareOwnColumn_IsFlagged()
+ {
+ var identity = Identity(alias: null, table: "@b", isTableVariable: true, bareOuterReferences: "A");
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([Y])=(1)", identity));
+ }
+
+ ///
+ /// "A" is listed as a bare outer reference, so it is not read as this scan's own column even
+ /// though it has the identical bare-bracketed shape as one (#561, #564).
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_UnaliasedTableVariableBareOuterReference_NotFlagged()
+ {
+ var identity = Identity(alias: null, table: "@b", isTableVariable: true, bareOuterReferences: "A");
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern("abs([A])=(1)", identity));
+ }
+
+ ///
+ /// An aliased table variable's own column always renders through its alias (#561) — a bare name
+ /// here can only be some other, unaliased table variable's, own column or outer reference, never
+ /// this scan's.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_AliasedTableVariableBareName_NotFlagged()
+ {
+ var identity = Identity(alias: "b", table: "@b", isTableVariable: true);
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern("abs([Y])=(1)", identity));
+ }
+
+ ///
+ /// No identity means the caller has not identified a scan — every existing caller of
+ /// DetectNonSargablePattern and ConvertImplicitWrapsColumn with no identity argument keeps this
+ /// coarser behavior: any dotted name counts as a column, whoever it actually belongs to.
+ ///
+ [Fact]
+ public void Rule12i_NonSargable_NoIdentity_AnyDottedNameStillCountsAsColumn()
+ {
+ Assert.Equal("Function call (ABS) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "abs([db].[dbo].[O].[a] as [o].[a])=(1)"));
+ }
+
+ // ---------------------------------------------------------------
+ // Rule 12: Non-SARGable Predicate — outer reference fixtures (#564)
+ // ---------------------------------------------------------------
+ //
+ // Each fixture is a CROSS APPLY with TOP (1) over a heap, so the inner side is a Table Scan
+ // carrying the predicate under test. Asserted per scan node, not per plan: Rule 11 stands down
+ // once Rule 12 has already flagged the same scan, so Non-SARGable and Scan With Predicate are
+ // mutually exclusive on any one node — checking one node at a time is what actually pins down
+ // which scan the fix does, and does not, change.
+
+ private static void AssertScanWarnings(ParsedPlan plan, int nodeId, bool expectNonSargable, string? messageContains = null)
+ {
+ var stmt = PlanTestHelper.FirstStatement(plan);
+ Assert.NotNull(stmt.RootNode);
+ var node = PlanTestHelper.FindNode(stmt.RootNode, nodeId);
+ Assert.NotNull(node);
+
+ var nonSargable = node!.Warnings.Where(w => w.WarningType == "Non-SARGable Predicate").ToList();
+ var scanWithPredicate = node.Warnings.Where(w => w.WarningType == "Scan With Predicate").ToList();
+
+ if (expectNonSargable)
+ {
+ Assert.Single(nonSargable);
+ if (messageContains != null)
+ Assert.Contains(messageContains, nonSargable[0].Message);
+ Assert.Empty(scanWithPredicate);
+ }
+ else
+ {
+ Assert.Empty(nonSargable);
+ Assert.Single(scanWithPredicate);
+ }
+ }
+
+ /// abs() wraps the outer reference (o.a); i.X, the scanned column, is compared bare.
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceFunctionAliased_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_function_aliased_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ /// abs() wraps i.X, the scanned column itself — must still be flagged.
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceFunctionOnScannedColumn_StaysNonSargable()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_function_on_scanned_column_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 4, expectNonSargable: true, messageContains: "Function call (ABS)");
+ }
+
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceFunctionUnaliased_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_function_unaliased_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ /// Self join: abs() wraps the OTHER instance (i1), aliased differently from this scan (i2).
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceFunctionSelfJoin_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_function_self_join_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ /// Self join, inner scan unaliased: the wrapped reference is the other, aliased instance.
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceSelfJoinUnaliasedInner_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_self_join_unaliased_inner_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceImplicitConversion_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_implicit_conversion_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceIsnull_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_isnull_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceTempTable_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_temp_table_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceTableVariableAliased_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_table_variable_aliased_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ ///
+ /// The bare-name case the tree walk exists for: Y (own) and A (outer reference) are both bare,
+ /// and only CollectBareOuterReferences — not the text shape alone — tells them apart.
+ ///
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceTableVariableUnaliased_GetsScanWithPredicate()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_table_variable_unaliased_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 3, expectNonSargable: false);
+ }
+
+ ///
+ /// The same OuterReferences entry ("A") read from both sides of the join it belongs to: the
+ /// OUTER scan of @a, where abs(A) wraps @a's own column and must stay flagged, and the INNER
+ /// scan of @b, whose Y = A comparison has no function at all and was never in question.
+ ///
+ [Fact]
+ public void Rule12j_NonSargable_OuterReferenceOuterSideScan_OuterScanStaysNonSargable_InnerScanUnaffected()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("outer_reference_outer_side_scan_plan.sqlplan");
+ AssertScanWarnings(plan, nodeId: 2, expectNonSargable: true, messageContains: "Function call (ABS)");
+ AssertScanWarnings(plan, nodeId: 4, expectNonSargable: false);
+ }
+
+ // ---------------------------------------------------------------
+ // Rule 12: Non-SARGable Predicate — compound predicates (#556)
+ // ---------------------------------------------------------------
+
+ ///
+ /// #556: the side check split the WHOLE predicate at its first comparison operator. In
+ /// [t].[A]=CONVERT_IMPLICIT(int,[@1],0) AND [t].[B]=CONVERT(tinyint,[@2],0) that operator
+ /// sits after [t].[A], so the second conjunct, [t].[B] included, landed on the CONVERT's side,
+ /// and a conversion of a parameter was reported as a function on a column. Both columns here are
+ /// compared bare. The fixture is the reporter's own SQL Server 2022 actual plan: an
+ /// auto-parameterized query on a table with no index on A or B, which is why it scans. Rule 11
+ /// still reports that scan, which is true and is the actionable half.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_ParameterSideConvertAfterAnotherComparison_NotFlagged()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("non_sargable_compound_predicate_plan.sqlplan");
+
+ Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Non-SARGable Predicate"));
+ Assert.NotEmpty(PlanTestHelper.WarningsOfType(plan, "Scan With Predicate"));
+ }
+
+ ///
+ /// The everyday shape of the same bug, and likely the most common one in the field: a date
+ /// range with dateadd() on the parameter side of the lower bound. Under the old split the
+ /// upper bound's column sat on the dateadd's side, and a perfectly SARGable range was told to
+ /// "remove the function from the column side".
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_DateRangeWithParameterSideDateadd_NotFlagged()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[Posts].[CreationDate] as [p].[CreationDate]>=dateadd(day,(-7),getdate()) " +
+ "AND [db].[dbo].[Posts].[CreationDate] as [p].[CreationDate]
+ /// The fix must narrow the side check, not blunt it: a function that really does wrap a column
+ /// is still caught when it sits in a later comparison of a compound predicate.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_ColumnSideFunctionInLaterComparison_IsFlagged()
+ {
+ Assert.Equal("Function call (DATEPART) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[T].[A] as [t].[A]=(1) " +
+ "AND datepart(year,[db].[dbo].[T].[D] as [t].[D])=(2013)"));
+ }
+
+ ///
+ /// SQL Server keeps the parentheses of a nested OR, so the predicate splits at every AND/OR
+ /// depth, not only the top level. Splitting at the top level alone would leave the group as
+ /// one piece, and its first operator would again put [t].[C] on the upper()'s side.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_ParameterSideFunctionInsideParenthesizedOr_NotFlagged()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[T].[A] as [t].[A]=(1) " +
+ "AND ([db].[dbo].[T].[B] as [t].[B]=upper([@p]) OR [db].[dbo].[T].[C] as [t].[C]=(3))"));
+ }
+
+ ///
+ /// An "and" inside a string literal is text, not an operator. Splitting on it would cut the
+ /// replace() away from its own comparison, and a piece with no operator in it is treated as
+ /// the worst case, which turns a parameter-side function into a false warning.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_AndInsideStringLiteral_DoesNotSplitTheComparison()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "replace([@p],N'Tom and Jerry',N'')=[db].[dbo].[T].[Name] as [t].[Name]"));
+ }
+
+ ///
+ /// LIKE is a comparison too. Without it the side check found no operator, fell back to assuming
+ /// the worst, and reported the upper() on the PATTERN as a function on the column. A pattern
+ /// built from a parameter is a runtime constant and seeks fine.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_ParameterSideFunctionInLikePattern_NotFlagged()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[T].[Name] as [t].[Name] like upper([@p])"));
+ }
+
+ ///
+ /// The mirror image stays flagged: upper() on the COLUMN side of a LIKE forces every row
+ /// through the function before the pattern can be applied.
+ ///
+ [Fact]
+ public void Rule12g_NonSargable_ColumnSideFunctionBeforeLike_IsFlagged()
+ {
+ Assert.Equal("Function call (UPPER) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "upper([db].[dbo].[T].[Name] as [t].[Name]) like N'ABC%'"));
+ }
+
+ ///
+ /// ISNULL was flagged wherever it appeared, so ISNULL(@p, 0) on the parameter side was reported
+ /// as "wrapping a column" even though it is a runtime constant that seeks fine. It now gets the
+ /// same side check as every other function, and in a compound predicate that is the same shape
+ /// #556 reported, with ISNULL in place of CONVERT.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_ParameterSideIsnull_NotFlagged()
+ {
+ Assert.Null(PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[T].[A] as [t].[A]=(52) AND [db].[dbo].[T].[B] as [t].[B]=isnull([@p],(0))"));
+ }
+
+ ///
+ /// The optional-parameter pattern, WHERE col = ISNULL(@p, col), still reads as non-SARGable: the
+ /// column sits inside the ISNULL, so it shares the function's side of the comparison.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_IsnullWithColumnFallback_IsFlagged()
+ {
+ Assert.Equal("ISNULL/COALESCE wrapping column", PlanAnalyzer.DetectNonSargablePattern(
+ "[db].[dbo].[T].[Name] as [t].[Name]=isnull([@p],[db].[dbo].[T].[Name] as [t].[Name])"));
+ }
+
+ ///
+ /// A parameter-side ISNULL used to win the check order and report "ISNULL/COALESCE wrapping
+ /// column" for a predicate whose real problem is a function on a column somewhere else. The
+ /// message now names the function that is actually on the column.
+ ///
+ [Fact]
+ public void Rule12h_NonSargable_ParameterSideIsnullDoesNotMisnameTheRealProblem()
+ {
+ Assert.Equal("Function call (DATEPART) on column", PlanAnalyzer.DetectNonSargablePattern(
+ "datepart(year,[db].[dbo].[T].[D] as [t].[D])=(2013) " +
+ "AND [db].[dbo].[T].[B] as [t].[B]=isnull([@p],(0))"));
}
// ---------------------------------------------------------------
@@ -436,6 +964,88 @@ public void Rule15_JoinOrClause_DetectsConcatenationWithConstantScans()
Assert.Contains("UNION ALL", warnings[0].Message);
}
+ ///
+ /// #558: WHERE t.A IN (@p1, @p2) on an indexed column builds the same operator chain as a join
+ /// OR. It is a dynamic seek: Constant Scans produce [@p1] and [@p2], Merge Interval combines the
+ /// ranges, and one Index Seek reads them. It ran once and returned 2 rows, and a UNION ALL rewrite
+ /// would not help. The fixture is the reporter's own SQL Server 2022 actual plan.
+ ///
+ [Fact]
+ public void Rule15_JoinOrClause_DynamicSeekForParameterInList_NotFlagged()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("in_list_dynamic_seek_plan.sqlplan");
+
+ Assert.Empty(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
+ }
+
+ ///
+ /// The branch values of the reporter's plan, as the parser records them: parameters and a
+ /// literal. The same holds for local variables ([@a]) and for functions of a parameter, such as
+ /// LikeRangeStart([@a]) for an OR of LIKE patterns or abs([@p2]) inside an IN list.
+ ///
+ [Theory]
+ [InlineData("Expr1002 = [@p2]; Expr1003 = [@p2]; Expr1001 = (62)")]
+ [InlineData("Expr1004 = LikeRangeStart([@a]); Expr1005 = LikeRangeEnd([@a]); Expr1006 = LikeRangeInfo([@a])")]
+ [InlineData("Expr1002 = abs([@p2]); Expr1003 = abs([@p2]); Expr1001 = (62)")]
+ [InlineData("Expr1002 = [dbo].[fn]([@p1])")]
+ public void Rule15_JoinOrClause_ParameterOnlyLookup_DoesNotReadAnotherInput(string values)
+ {
+ Assert.False(PlanAnalyzer.ReadsAnotherInput(values));
+ }
+
+ ///
+ /// #558's shape guard must not cost a real join OR. This one mixes a column and a parameter:
+ /// ON t.A = o.X OR t.A = @p. One branch produces [o].[X] and the other produces [@p], and one
+ /// branch that reads the outer row is enough. Captured on SQL Server 2022.
+ ///
+ [Fact]
+ public void Rule15_JoinOrClause_ColumnBranchNextToParameterBranch_IsFlagged()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("join_or_mixed_parameter_plan.sqlplan");
+
+ Assert.Single(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
+ }
+
+ ///
+ /// An OR join on expressions of the outer columns (ON t.A = o.X + 1 OR t.A = o.Y + 1) computes
+ /// o.X + 1 on the outer input, so its branches produce [Expr1002] and [Expr1003] and name no
+ /// column at all. A check that looked for column names only, which is what the issue first
+ /// suggested, would lose this warning. Captured on SQL Server 2022.
+ ///
+ [Fact]
+ public void Rule15_JoinOrClause_OrJoinOnOuterExpressions_IsFlagged()
+ {
+ var plan = PlanTestHelper.LoadAndAnalyze("join_or_expression_plan.sqlplan");
+
+ Assert.Single(PlanTestHelper.WarningsOfType(plan, "Join OR Clause"));
+ }
+
+ ///
+ /// The branch values that real OR joins produce on SQL Server 2022: the outer columns; an
+ /// expression of the outer columns, which renders as [Expr1002]; and a table variable's column,
+ /// which renders without brackets around its name. The second row puts a parameter before a
+ /// column, so the scan must not stop at the first name it can skip.
+ ///
+ [Theory]
+ [InlineData("Expr1005 = [StackOverflow2013].[dbo].[Posts].[OwnerUserId] as [p].[OwnerUserId]; Expr1004 = (62)")]
+ [InlineData("Expr1008 = [@p]; Expr1007 = (62); Expr1010 = [Repro].[dbo].[O].[X] as [o].[X]")]
+ [InlineData("Expr1010 = [Expr1002]; Expr1011 = [Expr1002]; Expr1009 = (62)")]
+ [InlineData("Expr1008 = @tv.[X] as [v].[X]; Expr1007 = (62)")]
+ public void Rule15_JoinOrClause_LookupFromAnotherInput_IsRecognized(string values)
+ {
+ Assert.True(PlanAnalyzer.ReadsAnotherInput(values));
+ }
+
+ ///
+ /// A bracket inside a string literal is text. Read as a name, it would turn an IN list of
+ /// strings back into a false join OR.
+ ///
+ [Fact]
+ public void Rule15_JoinOrClause_BracketInsideStringLiteral_IsNotAName()
+ {
+ Assert.False(PlanAnalyzer.ReadsAnotherInput("Expr1002 = N'[Posts].[OwnerUserId]'; Expr1001 = (62)"));
+ }
+
// ---------------------------------------------------------------
// Rule 16: Nested Loops High Executions
// ---------------------------------------------------------------
@@ -964,6 +1574,57 @@ public void WaitStats_Rule11_CpuWaitsDoNotElevateSeverity()
Assert.All(warnings, w => Assert.Equal(PlanWarningSeverity.Warning, w.Severity));
}
+ // ---------------------------------------------------------------
+ // Rule 35: Expensive Operator
+ // ---------------------------------------------------------------
+
+ [Fact]
+ public void Rule35_ExpensiveOperator_NotFiredWhenStatementUnderOneSecond()
+ {
+ // multi_index_update_plan: statement elapsed 1ms. Before #562, one operator
+ // dominating a sub-second statement always claimed most of the (tiny) elapsed
+ // time, so the share pointed at nothing.
+ var plan = PlanTestHelper.LoadAndAnalyze("multi_index_update_plan.sqlplan");
+ var warnings = PlanTestHelper.WarningsOfType(plan, "Expensive Operator");
+
+ Assert.Empty(warnings);
+ }
+
+ [Fact]
+ public void Rule35_ExpensiveOperator_FiredWhenStatementAtOneSecond()
+ {
+ // parallel_row_over_batch_plan: statement elapsed is exactly 1,000ms, pinning
+ // the #562 floor as >= rather than >.
+ var plan = PlanTestHelper.LoadAndAnalyze("parallel_row_over_batch_plan.sqlplan");
+ var warnings = PlanTestHelper.WarningsOfType(plan, "Expensive Operator");
+
+ Assert.Single(warnings);
+ Assert.Equal(PlanWarningSeverity.Critical, warnings[0].Severity);
+ Assert.Contains("Hash Match", warnings[0].Message);
+ }
+
+ [Fact]
+ public void Rule35_ExpensiveOperator_NotFiredJustUnderOneSecondFloor()
+ {
+ // Same plan as above, statement elapsed forced to 999ms — one ms under the
+ // #562 floor. The Hash Match operator's own share of statement time is still
+ // ~90%, so this isolates the floor from the 20%-share threshold.
+ var plan = PlanTestHelper.LoadAndAnalyzeWithElapsedTimeMs("parallel_row_over_batch_plan.sqlplan", 999);
+ var warnings = PlanTestHelper.WarningsOfType(plan, "Expensive Operator");
+
+ Assert.Empty(warnings);
+ }
+
+ [Fact]
+ public void Rule35_ExpensiveOperator_FiredAtOneSecondFloorExactly()
+ {
+ // Same plan, statement elapsed forced to 1,000ms — exactly the #562 floor.
+ var plan = PlanTestHelper.LoadAndAnalyzeWithElapsedTimeMs("parallel_row_over_batch_plan.sqlplan", 1000);
+ var warnings = PlanTestHelper.WarningsOfType(plan, "Expensive Operator");
+
+ Assert.Single(warnings);
+ }
+
#region Rule 38 — Standard Edition DOP Limitation
private static PlanStatement BuildBatchModeDop2Statement()
diff --git a/tests/PlanViewer.Core.Tests/PlanTestHelper.cs b/tests/PlanViewer.Core.Tests/PlanTestHelper.cs
index f097c4ec..a26192b7 100644
--- a/tests/PlanViewer.Core.Tests/PlanTestHelper.cs
+++ b/tests/PlanViewer.Core.Tests/PlanTestHelper.cs
@@ -37,6 +37,29 @@ public static ParsedPlan LoadAndAnalyze(string planFileName, ServerMetadata? ser
return plan;
}
+ ///
+ /// Same load + analyze + score, but overrides every statement's QueryTimeStats.ElapsedTimeMs
+ /// before analysis runs. Used to test elapsed-time thresholds (e.g. rule 35's #562 floor) on
+ /// either side of the line without needing a fixture captured at an exact millisecond value.
+ ///
+ public static ParsedPlan LoadAndAnalyzeWithElapsedTimeMs(string planFileName, long elapsedTimeMs)
+ {
+ var path = Path.Combine("Plans", planFileName);
+ Assert.True(File.Exists(path), $"Test plan not found: {path}");
+
+ var xml = File.ReadAllText(path);
+ xml = xml.Replace("encoding=\"utf-16\"", "encoding=\"utf-8\"");
+ var plan = ShowPlanParser.Parse(xml);
+
+ foreach (var stmt in PlanStatements.EnumerateAll(plan))
+ if (stmt.QueryTimeStats != null)
+ stmt.QueryTimeStats.ElapsedTimeMs = elapsedTimeMs;
+
+ PlanAnalyzer.Analyze(plan);
+ BenefitScorer.Score(plan);
+ return plan;
+ }
+
///
/// Same load + analyze + score, with an analyzer config (for rules-config behavior like
/// severity overrides). Named rather than overloaded so an existing
diff --git a/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj b/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj
index 1665d098..ea2973a7 100644
--- a/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj
+++ b/tests/PlanViewer.Core.Tests/PlanViewer.Core.Tests.csproj
@@ -32,10 +32,10 @@
-
+
-
-
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/in_list_dynamic_seek_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/in_list_dynamic_seek_plan.sqlplan
new file mode 100644
index 00000000..5a4fcf8d
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/in_list_dynamic_seek_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/join_or_expression_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/join_or_expression_plan.sqlplan
new file mode 100644
index 00000000..f65b99c3
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/join_or_expression_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/join_or_mixed_parameter_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/join_or_mixed_parameter_plan.sqlplan
new file mode 100644
index 00000000..951cf10b
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/join_or_mixed_parameter_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/non_sargable_compound_predicate_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/non_sargable_compound_predicate_plan.sqlplan
new file mode 100644
index 00000000..df55474b
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/non_sargable_compound_predicate_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_aliased_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_aliased_plan.sqlplan
new file mode 100644
index 00000000..65bcf843
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_aliased_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_on_scanned_column_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_on_scanned_column_plan.sqlplan
new file mode 100644
index 00000000..d777c35b
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_on_scanned_column_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_self_join_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_self_join_plan.sqlplan
new file mode 100644
index 00000000..6824eee6
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_self_join_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_unaliased_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_unaliased_plan.sqlplan
new file mode 100644
index 00000000..9e1d0923
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_function_unaliased_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_implicit_conversion_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_implicit_conversion_plan.sqlplan
new file mode 100644
index 00000000..973549d8
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_implicit_conversion_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_isnull_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_isnull_plan.sqlplan
new file mode 100644
index 00000000..5b43c697
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_isnull_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_outer_side_scan_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_outer_side_scan_plan.sqlplan
new file mode 100644
index 00000000..8bf3c58c
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_outer_side_scan_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_self_join_unaliased_inner_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_self_join_unaliased_inner_plan.sqlplan
new file mode 100644
index 00000000..1b75071e
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_self_join_unaliased_inner_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_aliased_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_aliased_plan.sqlplan
new file mode 100644
index 00000000..82dcd7c3
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_aliased_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_unaliased_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_unaliased_plan.sqlplan
new file mode 100644
index 00000000..b733bb6b
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_table_variable_unaliased_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/outer_reference_temp_table_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/outer_reference_temp_table_plan.sqlplan
new file mode 100644
index 00000000..9e37e861
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/outer_reference_temp_table_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/table_variable_aliased_function_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/table_variable_aliased_function_plan.sqlplan
new file mode 100644
index 00000000..74f40268
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/table_variable_aliased_function_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/Plans/table_variable_implicit_conversion_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/table_variable_implicit_conversion_plan.sqlplan
new file mode 100644
index 00000000..d3dd9cdc
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/table_variable_implicit_conversion_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
\ No newline at end of file
diff --git a/tests/PlanViewer.Core.Tests/Plans/table_variable_unaliased_function_plan.sqlplan b/tests/PlanViewer.Core.Tests/Plans/table_variable_unaliased_function_plan.sqlplan
new file mode 100644
index 00000000..5b8a984c
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/Plans/table_variable_unaliased_function_plan.sqlplan
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs b/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs
new file mode 100644
index 00000000..9efbb2e9
--- /dev/null
+++ b/tests/PlanViewer.Core.Tests/ReproScriptBuilderTests.cs
@@ -0,0 +1,46 @@
+using PlanViewer.Core.Services;
+
+namespace PlanViewer.Core.Tests;
+
+///
+/// The repro script declares the parameters itself, so the declaration list that a cached
+/// sp_executesql statement starts with must not reach the script's query text. The list is found
+/// by the parser that parameter substitution uses (ParameterSubstitution.DeclarationListEnd).
+///
+public class ReproScriptBuilderTests
+{
+ private const string Plan = """
+
+
+
+
+
+
+
+
+
+
+
+ """;
+
+ [Fact]
+ public void BuildReproScript_DeclarationList_IsLeftOutOfTheQueryText()
+ {
+ var sql = ReproScriptBuilder.BuildReproScript(
+ "(@id decimal(18,2))SELECT * FROM dbo.T WHERE Id = @id", "db", Plan, null);
+
+ Assert.Contains("SELECT * FROM dbo.T WHERE Id = @id", sql);
+ Assert.DoesNotContain("(@id decimal(18,2))SELECT", sql);
+ Assert.Contains("@id = 42.50", sql);
+ }
+
+ [Fact]
+ public void BuildReproScript_DeclarationListCutOffByTruncation_IsKeptAsItIs()
+ {
+ /* The parser reports a list that never closes as -1. The text stays as it was, as it did
+ before the parser was shared, and the -1 must never reach a slice. */
+ var sql = ReproScriptBuilder.BuildReproScript("(@id decimal(18,2),@x nvarch", "db", Plan, null);
+
+ Assert.Contains("(@id decimal(18,2),@x nvarch", sql);
+ }
+}
diff --git a/tests/PlanViewer.Core.Tests/ScrollBarVisibilityTests.cs b/tests/PlanViewer.Core.Tests/ScrollBarVisibilityTests.cs
index a381b560..5cf27751 100644
--- a/tests/PlanViewer.Core.Tests/ScrollBarVisibilityTests.cs
+++ b/tests/PlanViewer.Core.Tests/ScrollBarVisibilityTests.cs
@@ -137,30 +137,44 @@ public void AnIdleScrollBarIsNarrowerThanAnExpandedOne()
/// Two selectors are needed in App.axaml, not one, and that is the whole reason this case is
/// asserted separately. A ScrollViewer rule alone looks like it covers the app and does not:
/// DataGrid does not scroll through a ScrollViewer, its template hosts PART_HorizontalScrollbar
- /// and PART_VerticalScrollbar as bare s, and it assigns their
- /// AllowAutoHide in code from the ATTACHED ScrollViewer property it reads off itself —
- /// where a local value outranks any style. A style that compiles is not a style that matches.
+ /// and PART_VerticalScrollbar as bare s, and it binds their
+ /// AllowAutoHide to the ATTACHED ScrollViewer property on the grid — which nothing sets
+ /// unless this rule does. A style that compiles is not a style that matches.
///
- /// Why only the flag is checked here. A DataGrid decides it overflows by measuring
- /// its rows, rows measure their text, and text needs a font — which this suite has no Skia for,
- /// so headlessly the rows come out zero-high, the grid concludes it fits, and both bars stay
- /// IsVisible=false with no template and no thumb to measure. The rail thickness is not
- /// DataGrid-specific anyway: it comes from app-level resources on the shared ScrollBar
- /// ControlTheme, and the ScrollViewer cases above prove those resources land.
+ /// Why only the flag is checked here. Nothing in this grid overflows, so neither
+ /// bar ever applies its template: measured under 12, both stay IsVisible=false with no
+ /// thumb to measure, which is why only AllowAutoHide is asserted and why asserting it on
+ /// an untemplated bar is still meaningful — the value is bound, not assigned during
+ /// OnApplyTemplate. The reason the grid does not overflow is worth stating because the
+ /// obvious guess is wrong: it is not that text measures short in this harness — under 12 it
+ /// measures against a real font with real metrics — it is that the grid generates no columns
+ /// for its 50 rows, so no row is ever realized. The rail thickness is not DataGrid-specific
+ /// anyway: it comes from app-level resources on the shared ScrollBar ControlTheme, and the
+ /// ScrollViewer cases above prove those resources land.
///
[Fact]
public void ADataGridsOwnScrollBarsFollowTheSameContract()
{
HeadlessUi.Run(() =>
{
- /* The grid needs columns and rows before its template puts scrollbars in the tree,
- so this is a real grid rather than an empty one. */
+ /* No columns on purpose — the summary above explains that nothing realizes here, so
+ what this case pins is the BOUND flag on untemplated bars, not geometry. The
+ columns-bearing sibling below is where bars realize. */
var grid = new DataGrid
{
ItemsSource = Enumerable.Range(0, 50).Select(i => new { Value = i }).ToList()
};
Show(grid);
+ /* A shown grid is also the only place the suite can falsify the Avalonia 12 migration
+ of DataGridBehaviors.AttachCopyGuard, which moved off the removed
+ TopLevel.PlatformSettings onto Visual.GetPlatformSettings(). Pressing Ctrl+C by hand
+ on Windows cannot falsify it: the guard falls back to KeyModifiers.Control when the
+ lookup yields nothing, and Control is exactly what Windows reports anyway, so a dead
+ lookup and a live one behave identically under the fingers. An attached grid has
+ platform settings, so this asserts the lookup itself rather than its fallback. */
+ Assert.NotNull(grid.GetPlatformSettings());
+
var bars = grid.GetVisualDescendants().OfType().ToList();
Assert.NotEmpty(bars);
@@ -171,6 +185,56 @@ so this is a real grid rather than an empty one. */
});
}
+ ///
+ /// The columns-bearing sibling of the contract above (#548). With real columns the rows
+ /// realize, the grid overflows both ways, and the PART_ bars apply their templates — the
+ /// state every grid in the app actually runs in, and the state the columns-less case above
+ /// structurally cannot reach. Pinned here: templated bars exist at all (zero of them realize
+ /// without columns), every one keeps AllowAutoHide from the attached property the
+ /// App.axaml rule sets (12 binds it in-template; upstream PR #241), and the vertical bar
+ /// reports visible, because fifty realized rows cannot fit a 250px grid.
+ ///
+ [Fact]
+ public void AGridWithColumnsRealizesItsBarsAndKeepsAllowAutoHide()
+ {
+ HeadlessUi.Run(() =>
+ {
+ var grid = new DataGrid
+ {
+ Width = 350,
+ Height = 250,
+ ItemsSource = Enumerable.Range(0, 50).Select(i => new BarRow(i, $"row {i}")).ToList()
+ };
+ grid.Columns.Add(new DataGridTextColumn
+ {
+ Header = "Value",
+ Binding = new Avalonia.Data.Binding(nameof(BarRow.Value))
+ });
+ /* Wider than the grid on purpose, so the horizontal bar has a reason to exist too. */
+ grid.Columns.Add(new DataGridTextColumn
+ {
+ Header = "Name",
+ Binding = new Avalonia.Data.Binding(nameof(BarRow.Name)),
+ Width = new DataGridLength(400)
+ });
+ Show(grid);
+
+ var templated = ScrollBarsOf(grid);
+
+ Assert.NotEmpty(templated);
+ Assert.All(templated, bar => Assert.True(
+ bar.AllowAutoHide,
+ "a realized DataGrid bar takes AllowAutoHide from the attached property on the " +
+ "grid via its template binding — losing it here means the slim-rail contract " +
+ "silently died on the grids people actually scroll"));
+ Assert.True(
+ templated.First(bar => bar.Orientation == Orientation.Vertical).IsVisible,
+ "fifty realized rows cannot fit a 250px grid");
+ });
+ }
+
+ private sealed record BarRow(int Value, string Name);
+
///
/// How much of the bar is actually painted while it is idle.
///
diff --git a/tests/PlanViewer.Core.Tests/SessionTestHarness.cs b/tests/PlanViewer.Core.Tests/SessionTestHarness.cs
index 9fb96386..74c31697 100644
--- a/tests/PlanViewer.Core.Tests/SessionTestHarness.cs
+++ b/tests/PlanViewer.Core.Tests/SessionTestHarness.cs
@@ -193,6 +193,18 @@ internal static void InvalidateOverview(QuerySessionControl session) =>
.GetMethod("InvalidateOverviewView", BindingFlags.Instance | BindingFlags.NonPublic)!
.Invoke(session, null);
+ ///
+ /// Calls the session's private RefreshEmptyState, the call the connect block makes
+ /// after flipping the toolbar. Reached this way for the same reason
+ /// is: the block that calls it in the app is a connection
+ /// dialog and three round trips to a server. Tests using this pin what the refresh decides
+ /// once a connection exists, not that connecting calls it.
+ ///
+ internal static void RefreshEmptyState(QuerySessionControl session) =>
+ typeof(QuerySessionControl)
+ .GetMethod("RefreshEmptyState", BindingFlags.Instance | BindingFlags.NonPublic)!
+ .Invoke(session, null);
+
///
/// Opens a read-only schema document, the third of the three places in the app that builds a
/// document header. Its own entry point fetches DDL from a server first.
diff --git a/tests/PlanViewer.Core.Tests/SessionToolbarLayoutTests.cs b/tests/PlanViewer.Core.Tests/SessionToolbarLayoutTests.cs
index e9132600..12b85c42 100644
--- a/tests/PlanViewer.Core.Tests/SessionToolbarLayoutTests.cs
+++ b/tests/PlanViewer.Core.Tests/SessionToolbarLayoutTests.cs
@@ -40,8 +40,9 @@ public void ConnectingMovesNothingInTheToolbarOrTheSubTabRowBelowIt()
{
HeadlessUi.Run(() =>
{
- /* Wide enough for the whole row (natural 1910px in this harness's font metrics), because
- the first half of this test is about every slot HAVING a geometry to hold still. */
+ /* Wide enough for the whole row (natural 2116px in this harness's font metrics, which
+ HeadlessUi explains), because the first half of this test is about every slot HAVING
+ a geometry to hold still. */
var window = new MainWindow { Width = 2200, Height = 800 };
try
{
diff --git a/tests/PlanViewer.Core.Tests/SessionViewLifecycleTests.cs b/tests/PlanViewer.Core.Tests/SessionViewLifecycleTests.cs
index 2cb8e5a5..d650a25e 100644
--- a/tests/PlanViewer.Core.Tests/SessionViewLifecycleTests.cs
+++ b/tests/PlanViewer.Core.Tests/SessionViewLifecycleTests.cs
@@ -288,8 +288,10 @@ selection a range. */
}
///
- /// A session with the Overview open still has an empty editor behind it, and going back to that
- /// editor gets the get-started panel — opening a view is not opening anything.
+ /// A session with the Overview open still has an empty editor behind it, and opening a view
+ /// is not opening anything: the document strip stays empty. The get-started panel proved
+ /// that half before #540; now the connection the Overview needs is itself enough to retire
+ /// the panel, so the strip carries the claim and the overlay is pinned to staying down.
///
[Fact]
public void TheOverviewDoesNotCountAsHavingOpenedSomething()
@@ -300,6 +302,8 @@ public void TheOverviewDoesNotCountAsHavingOpenedSomething()
try
{
SessionHarness.PretendConnected(session);
+ // The refresh the real connect block runs after flipping the toolbar (#540).
+ SessionHarness.RefreshEmptyState(session);
Assert.Empty(session.QueryEditor.Text);
SessionHarness.OverviewSegment(session).IsChecked = true;
@@ -312,7 +316,8 @@ public void TheOverviewDoesNotCountAsHavingOpenedSomething()
SessionHarness.EditorSegment(session).IsChecked = true;
window.UpdateLayout();
- Assert.True(SessionHarness.EmptyStateOverlay(session).IsVisible);
+ Assert.False(SessionHarness.EmptyStateOverlay(session).IsVisible,
+ "connected — coming back to the editor shows the editor, not get-started (#540)");
Assert.True(SessionHarness.EditorView(session).IsVisible);
}
finally
diff --git a/tests/PlanViewer.Core.Tests/ToolbarOverflowTests.cs b/tests/PlanViewer.Core.Tests/ToolbarOverflowTests.cs
index f2736a48..ef558c64 100644
--- a/tests/PlanViewer.Core.Tests/ToolbarOverflowTests.cs
+++ b/tests/PlanViewer.Core.Tests/ToolbarOverflowTests.cs
@@ -15,10 +15,13 @@ namespace PlanViewer.Core.Tests;
///
/// The session toolbar is one fixed row of slots inside a ScrollViewer whose rail is deliberately
-/// suppressed. Measured in this harness, that row wants 1910px. Erik's maximized display is 1536
-/// logical, so Format and part of Run Repro were simply not on screen, behind a scrollbar that had
-/// been hidden on purpose because a rail under a 28px button row grows the strip and drags the
-/// sub-tab row with it. Hidden content, hidden affordance.
+/// suppressed. Measured in this harness, that row wants 2116px — it wanted 1910px until the headless
+/// text metrics changed under Avalonia 12, which HeadlessUi explains once for every number in these
+/// files. Neither figure is what the row measures on the fonts a user has; they are the widths the
+/// thresholds below are chosen against. What put this file here is real: on Erik's maximized
+/// 1536-logical display, Format and part of Run Repro were simply not on screen, behind a scrollbar
+/// that had been hidden on purpose because a rail under a 28px button row grows the strip and drags
+/// the sub-tab row with it. Hidden content, hidden affordance.
///
/// The fix is a chevron that holds whatever did not fit. What these tests pin is the part of
/// it that is easy to get subtly wrong and impossible to see in a screenshot: that a command in the
@@ -105,7 +108,7 @@ private static readonly (string Name, string Label)[] CollapseOrder =
private static string[] CollapseLabels => CollapseOrder.Select(c => c.Label).ToArray();
///
- /// The width Erik actually runs at. At a 1520px row the three trailing commands move into the
+ /// The width Erik actually runs at. At a 1520px row the four trailing commands move into the
/// menu, in the order they left it, and every button in front of them is exactly where it was
/// on a row wide enough for all of them.
///
@@ -140,16 +143,29 @@ public void ANarrowRowMovesItsTailIntoTheMenuAndMovesNothingElse()
SetViewport(window, scroll, session.Overflow, 1520);
- Assert.True(chevron.IsVisible, "a 1520px row cannot hold a 1910px toolbar");
+ Assert.True(chevron.IsVisible, "a 1520px row cannot hold a 2116px toolbar");
/* The one failure every other assertion here would sail past: a chevron that
appears on cue, holds the right commands, and opens nothing when you press it. */
Assert.Same(session.Overflow.Menu, chevron.Flyout);
- /* Three commands, in the order the row gives them up: the end of the row first.
+ /* Four commands, in the order the row gives them up: the end of the row first.
Format being the first entry is the contract — the menu grows and shrinks at its
- end, so an entry never changes position under the pointer as the window moves. */
- Assert.Equal(new[] { "Format", "Run Repro", "Copy Repro" }, MenuHeaders(session.Overflow));
+ end, so an entry never changes position under the pointer as the window moves.
+
+ How many end up here is a function of the harness's text metrics and is allowed
+ to move with them; it was three until Avalonia 12 made text wider, and QS
+ Overview is simply the next name in CollapseOrder. What is NOT allowed to move
+ is which commands may leave at all: Connect, Execute and Execute-with-estimate
+ are absent from CollapseOrder and must never appear in this menu at any width.
+ A metric change adds the next name in the documented order; a regression takes
+ a protected one. This exact-collection assertion is itself the guard: a
+ protected command that collapsed would appear in this list and fail it. That is
+ why it stays an exact collection and never becomes a count or a containment
+ check. (The X comparison below skips invisible buttons, so it would not catch
+ a stayer leaving — it pins that the survivors did not shift.) */
+ Assert.Equal(new[] { "Format", "Run Repro", "Copy Repro", "QS Overview" },
+ MenuHeaders(session.Overflow));
Assert.False(format.IsVisible);
// Everything still on the row is where it was, to the pixel, and the sub-tab row
@@ -513,7 +529,7 @@ public void TheScrollingRowIsStillThereBelowTheCollapseFloor()
var subTabs = session.FindControl("SubTabControl")!;
var subTabsY = subTabs.Bounds.Y;
- /* Connect, the server label, the database picker and the two plan verbs want 708px
+ /* Connect, the server label, the database picker and the two plan verbs want 746px
between them and cannot be collapsed, so under that the row has to scroll. */
window.Width = 640;
Settle(window, session.Overflow);
@@ -746,7 +762,8 @@ Clear it first so the width this test names is the one it keeps coming back to.
Settle(window, session.Overflow, viewer.Overflow);
Assert.Equal(planRevisions, viewer.Overflow.Revisions);
- Assert.Equal(new[] { "Format", "Run Repro", "Copy Repro" }, MenuHeaders(session.Overflow));
+ Assert.Equal(new[] { "Format", "Run Repro", "Copy Repro", "QS Overview" },
+ MenuHeaders(session.Overflow));
}
finally
{
diff --git a/tests/PlanViewer.Core.Tests/WarningBaseline.txt b/tests/PlanViewer.Core.Tests/WarningBaseline.txt
index 3ffa1daf..18794195 100644
--- a/tests/PlanViewer.Core.Tests/WarningBaseline.txt
+++ b/tests/PlanViewer.Core.Tests/WarningBaseline.txt
@@ -92,6 +92,8 @@ Table Variable | Warning | Table variable detected. Table variables lack column-
Implicit Conversion | Critical | Implicit conversion prevented an index seek, forcing a scan instead. Fix the data type mismatch: ensure the parameter or variable type matches the column type exactly. Seek Plan: CONVERT_IMPLICIT(nvarchar(40),[TestDB].[dbo].[Users].[DisplayName],0)=[@d]
Bare Scan | Warning | Clustered index scan reads the full table with no predicate, outputting 1 column(s): Users.Id. Consider a nonclustered index on the output columns (as key or INCLUDE) so SQL Server can read a narrower structure. For analytical workloads, a columnstore index may be a better fit.
+### in_list_dynamic_seek_plan.sqlplan
+
### isnull_plan.sqlplan
Serial Plan | Warning | Query running serially: MAXDOP is set to 1 using a query hint.
Wait: PAGEIOLATCH_SH | Info | PAGEIOLATCH_SH Observed 500 ms across 405 waits. Effective latency: 1.2 ms per wait.
@@ -116,6 +118,12 @@ Expensive Operator | Critical | Sort took 18,580ms (56.2% of statement elapsed)
Join OR Clause | Warning | OR in a join predicate. SQL Server rewrote the OR as 2 separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change "FROM a JOIN b ON a.x = b.x OR a.y = b.y" to "FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y".
Expensive Operator | Warning | Clustered Index Seek took 14,355ms (43.4% of statement elapsed) but no specific rule identified a fix. Worth investigating: is the row volume necessary? Are upstream estimates driving this operator harder than it should be?
+### join_or_expression_plan.sqlplan
+Join OR Clause | Warning | OR in a join predicate. SQL Server rewrote the OR as 2 separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change "FROM a JOIN b ON a.x = b.x OR a.y = b.y" to "FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y".
+
+### join_or_mixed_parameter_plan.sqlplan
+Join OR Clause | Warning | OR in a join predicate. SQL Server rewrote the OR as 2 separate lookups, each evaluated independently — this multiplies the work on the inner side. Rewrite as separate queries joined with UNION ALL. For example, change "FROM a JOIN b ON a.x = b.x OR a.y = b.y" to "FROM a JOIN b ON a.x = b.x UNION ALL FROM a JOIN b ON a.y = b.y".
+
### key_lookup_plan.sqlplan
Key Lookup | Critical | Key Lookup — SQL Server found rows via a nonclustered index but had to go back to the clustered index for additional columns.\nColumns fetched: Users.AboutMe, Users.Age, Users.CreationDate, Users.DisplayName, Users.DownVotes, Users.EmailHash, Users.LastAccessDate, Users.Location, Users.UpVotes, Users.Views, Users.WebsiteUrl, Users.AccountId\nResidual predicate (filtered 116 rows): [StackOverflow2013].[dbo].[Users].[DisplayName] as [u].[DisplayName]=N'Eggs McLaren'\nTo eliminate the lookup, consider adding the needed columns as INCLUDE columns on the nonclustered index. This widens the index, so weigh the read benefit against write and storage overhead.
@@ -194,7 +202,9 @@ Scan With Predicate | Critical | Scan with residual predicate — SQL Server is
### multi_index_insert_plan.sqlplan
### multi_index_update_plan.sqlplan
-Expensive Operator | Critical | Clustered Index Update took 1ms (100.0% of statement elapsed) but no specific rule identified a fix. Worth investigating: is the row volume necessary? Are upstream estimates driving this operator harder than it should be?
+
+### non_sargable_compound_predicate_plan.sqlplan
+Scan With Predicate | Critical | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 100% of the plan cost. Check that you have appropriate indexes.\nPredicate: [Repro].[dbo].[T].[A] as [t].[A]=CONVERT_IMPLICIT(int,[@1],0) AND [Repro].[dbo].[T].[B] as [t].[B]=CONVERT(tinyint,[@2],0)
### non_sargable_function_plan.sqlplan
Wait: LATCH_EX | Info | LATCH_EX Observed 22 ms across 121 waits.
@@ -208,6 +218,60 @@ Non-SARGable Predicate | Warning | Function call (DATEPART) on column prevents a
Optimize For Unknown | Warning | OPTIMIZE FOR UNKNOWN uses average density estimates instead of sniffed parameter values. This can help when parameter sniffing causes plan instability, but may produce suboptimal plans for skewed data distributions.
Bare Scan | Warning | Clustered index scan reads the full table with no predicate, outputting 1 column(s): Users.Id. Consider a nonclustered index on the output columns (as key or INCLUDE) so SQL Server can read a narrower structure. For analytical workloads, a columnstore index may be a better fit.
+### outer_reference_function_aliased_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[X] as [i].[X]=abs([ReproOuterRef].[dbo].[O].[a] as [o].[a])
+
+### outer_reference_function_on_scanned_column_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 4). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Non-SARGable Predicate | Warning | Function call (ABS) on column prevents an index seek. Remove the function from the column side — apply it to the parameter instead, or create a computed column with the expression and index that.\nPredicate: abs([ReproOuterRef].[dbo].[I].[X] as [i].[X])=[ReproOuterRef].[dbo].[O].[a] as [o].[a]
+
+### outer_reference_function_self_join_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[X] as [i2].[X]=abs([ReproOuterRef].[dbo].[I].[X] as [i1].[X])
+
+### outer_reference_function_unaliased_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[X]=abs([ReproOuterRef].[dbo].[O].[a])
+
+### outer_reference_implicit_conversion_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[S] as [i].[S]=CONVERT_IMPLICIT(nvarchar(20),[ReproOuterRef].[dbo].[O].[N] as [o].[N],0)
+
+### outer_reference_isnull_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[X] as [i].[X]=isnull([ReproOuterRef].[dbo].[O].[a] as [o].[a],(0))
+
+### outer_reference_outer_side_scan_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Non-SARGable Predicate | Warning | Function call (ABS) on column prevents an index seek. Remove the function from the column side — apply it to the parameter instead, or create a computed column with the expression and index that.\nPredicate: abs([A])=(1)
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Top Above Scan | Critical | Top reads from Table Scan on @b (Node 4). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 50% of the plan cost. Only 0.000% of rows survived filtering (0 of 2). Check that you have appropriate indexes.\nPredicate: [Y]=[A]
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
+### outer_reference_self_join_unaliased_inner_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on ReproOuterRef.dbo.I (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [ReproOuterRef].[dbo].[I].[X]=abs([ReproOuterRef].[dbo].[I].[X] as [i1].[X])
+
+### outer_reference_table_variable_aliased_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Top Above Scan | Critical | Top reads from Table Scan on @b (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: @b.[Y] as [b].[Y]=abs(@a.[A] as [a].[A])
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
+### outer_reference_table_variable_unaliased_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Top Above Scan | Critical | Top reads from Table Scan on @b (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [Y]=abs([A])
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
+### outer_reference_temp_table_plan.sqlplan
+Top Above Scan | Critical | Top reads from Table Scan on tempdb.dbo.#u (Node 3). This is on the inner side of Nested Loops (Node 0), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.
+Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. This scan is 51% of the plan cost. Check that you have appropriate indexes.\nPredicate: [#u].[X]=abs([#t].[a])
+
### parallel-skew.sqlplan
Wide Index Suggestion | Warning | Missing index suggestion for Posts has 17 INCLUDE columns. This is a "kitchen sink" index — SQL Server suggests covering every column the query touches, but the resulting index would be very wide and expensive to maintain. Evaluate which columns are actually needed, or consider a narrower index with fewer includes.
Wait: MEMORY_ALLOCATION_EXT | Info | MEMORY_ALLOCATION_EXT Observed 1 ms across 1,900 waits.
@@ -322,10 +386,25 @@ Sort Spill | Critical | Sort spill level 1, 8 thread(s) — Granted: 10,815,552
Expensive Operator | Warning | Parallelism took 12,362ms (46.1% of statement elapsed) but no specific rule identified a fix. Worth investigating: is the row volume necessary? Are upstream estimates driving this operator harder than it should be?
Scan With Predicate | Warning | Scan with residual predicate — SQL Server is reading every row and filtering after the fact. Check that you have appropriate indexes.\nPredicate: [StackOverflow2013].[dbo].[Posts].[PostTypeId] as [p].[PostTypeId]=(2) AND [StackOverflow2013].[dbo].[Posts].[Score] as [p].[Score]>(0)
+### table_variable_aliased_function_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Non-SARGable Predicate | Warning | Function call (ABS) on column prevents an index seek. Remove the function from the column side — apply it to the parameter instead, or create a computed column with the expression and index that.\nPredicate: abs(@tv.[X] as [v].[X])=(1)
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
+### table_variable_implicit_conversion_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Non-SARGable Predicate | Warning | Implicit conversion (CONVERT_IMPLICIT) prevents an index seek. Match the parameter or variable data type to the column data type.\nPredicate: CONVERT_IMPLICIT(nvarchar(20),[S],0)=[@n]
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
### table_variable_plan.sqlplan
Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+### table_variable_unaliased_function_plan.sqlplan
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+Non-SARGable Predicate | Warning | Function call (ABS) on column prevents an index seek. Remove the function from the column side — apply it to the parameter instead, or create a computed column with the expression and index that.\nPredicate: abs([X])=(1)
+Table Variable | Warning | Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.
+
### top_above_scan_plan.sqlplan
Wait: SOS_SCHEDULER_YIELD | Info | SOS_SCHEDULER_YIELD Observed 3 ms across 1,269 waits.
Top Above Scan | Critical | Top reads from Clustered Index Scan on StackOverflow2013.dbo.Votes (Node 5). This is on the inner side of Nested Loops (Node 1), so the scan repeats for every outer row. The scan has a residual predicate, so it may read many rows before the Top is satisfied. An index on the ORDER BY columns could eliminate the scan and sort entirely.