Fase 2: plan cache analysis

Collector takes the 50 heaviest statements (CPU, last 24 h), fetches
statement plans as text (dm_exec_query_plan returns NULL for deeply
nested plans) and analyzes them in C#. Rules:
- SQLI-PLAN-001 implicit conversion preventing an index seek
- SQLI-PLAN-002 join without predicate (deliberate CROSS JOIN excluded)
- SQLI-PLAN-003 non-inlined scalar UDF in a heavy statement
- SQLI-PLAN-004 spills to tempdb, SQLI-PLAN-005 oversized memory grants
  (2016 SP2+ / 2017 CU3+ columns, new capability)
Verified against real plans in SQL Server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E3EExwndy8Z22EK2Qpt6BK

src/SQLInsight.Collectors/BuiltInCollectors.cs

@@ -22,5 +22,6 @@ public static class BuiltInCollectors
         new BlockingCollector(),
         new DeadlockCollector(),
         new QueryStoreCollector(),
+        new PlanAnalysisCollector(),
     ];
 }

src/SQLInsight.Collectors/Performance/PlanAnalysisCollector.cs

@@ -0,0 +1,68 @@
+using SQLInsight.Core.Data;
+using SQLInsight.Core.Instances;
+
+namespace SQLInsight.Collectors.Performance;
+
+/// <summary>
+/// The heaviest statements (CPU, last 24 hours) from the plan cache, with their cached plans analyzed.
+/// Plans are fetched as text with <c>sys.dm_exec_text_query_plan</c>: <c>sys.dm_exec_query_plan</c> returns NULL
+/// for plans nested deeper than the xml type allows. TOP is applied before any plan is fetched.
+/// </summary>
+public sealed class PlanAnalysisCollector : Collector<PlanAnalysisSnapshot>
+{
+    public const int TopStatements = 50;
+
+    public override string Name => "plan-analysis";
+    public override TimeSpan DefaultInterval => TimeSpan.FromMinutes(30);
+    public override IReadOnlyCollection<Capability> RequiredCapabilities => [Capability.DynamicManagementViews];
+
+    protected override async Task<PlanAnalysisSnapshot> CollectAsync(CollectorContext context, PlanAnalysisSnapshot? previous)
+    {
+        var memory = context.Capabilities.Has(Capability.QueryStatsMemoryColumns);
+        var statements = await Db.QueryAsync(context.Connection, BuildSql(memory), r => new AnalyzedStatement
+        {
+            QueryHash = r.String(0),
+            DatabaseName = r.NullableString(1),
+            StatementText = r.NullableString(2),
+            Executions = r.Long(3),
+            TotalWorkerTimeUs = r.Long(4),
+            TotalElapsedTimeUs = r.Long(5),
+            TotalLogicalReads = r.Long(6),
+            TotalSpills = r.NullableLong(7),
+            TotalGrantKb = r.NullableLong(8),
+            TotalUsedGrantKb = r.NullableLong(9),
+            LastExecution = r.NullableDateTime(10),
+            Warnings = ShowplanAnalyzer.Analyze(r.NullableString(11)),
+        }, context.CancellationToken).ConfigureAwait(false);
+
+        return new PlanAnalysisSnapshot { CapturedAt = context.Now, Statements = statements };
+    }
+
+    internal static string BuildSql(bool memoryColumns)
+    {
+        var memory = memoryColumns
+            ? "qs.total_spills, qs.total_grant_kb, qs.total_used_grant_kb"
+            : "CAST(NULL AS bigint), CAST(NULL AS bigint), CAST(NULL AS bigint)";
+        return $"""
+            WITH top_statements AS (
+                SELECT TOP ({TopStatements}) qs.*
+                FROM sys.dm_exec_query_stats qs
+                WHERE qs.last_execution_time >= DATEADD(hour, -24, GETDATE())
+                ORDER BY qs.total_worker_time DESC
+            )
+            SELECT CONVERT(varchar(20), qs.query_hash, 1),
+                   DB_NAME(CONVERT(int, pa.value)),
+                   LEFT(SUBSTRING(t.text, qs.statement_start_offset / 2 + 1,
+                        (CASE WHEN qs.statement_end_offset = -1 THEN DATALENGTH(t.text) ELSE qs.statement_end_offset END - qs.statement_start_offset) / 2 + 1), 4000),
+                   qs.execution_count, qs.total_worker_time, qs.total_elapsed_time, qs.total_logical_reads,
+                   {memory},
+                   qs.last_execution_time,
+                   tp.query_plan
+            FROM top_statements qs
+            CROSS APPLY sys.dm_exec_sql_text(qs.sql_handle) t
+            OUTER APPLY sys.dm_exec_text_query_plan(qs.plan_handle, qs.statement_start_offset, qs.statement_end_offset) tp
+            OUTER APPLY (SELECT value FROM sys.dm_exec_plan_attributes(qs.plan_handle) WHERE attribute = N'dbid') pa
+            ORDER BY qs.total_worker_time DESC;
+            """;
+    }
+}

src/SQLInsight.Collectors/Performance/ShowplanAnalyzer.cs

@@ -0,0 +1,63 @@
+using System.Xml;
+using System.Xml.Linq;
+using SQLInsight.Core.Data;
+
+namespace SQLInsight.Collectors.Performance;
+
+/// <summary>
+/// Reads the warnings that matter out of a showplan XML document.
+/// </summary>
+internal static class ShowplanAnalyzer
+{
+    private static readonly XNamespace Ns = "http://schemas.microsoft.com/sqlserver/2004/07/showplan";
+
+    /// <summary>Plans larger than this are skipped: parsing them costs more than it tells.</summary>
+    public const int MaxPlanLength = 4 * 1024 * 1024;
+
+    public static PlanWarnings Analyze(string? planXml)
+    {
+        if (string.IsNullOrEmpty(planXml) || planXml.Length > MaxPlanLength)
+        {
+            return new PlanWarnings { NotAnalyzed = true };
+        }
+
+        XDocument plan;
+        try
+        {
+            using var reader = XmlReader.Create(new StringReader(planXml), new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit, XmlResolver = null });
+            plan = XDocument.Load(reader);
+        }
+        catch (XmlException)
+        {
+            return new PlanWarnings { NotAnalyzed = true };
+        }
+
+        var conversions = plan.Descendants(Ns + "PlanAffectingConvert")
+            .Where(c => (string?)c.Attribute("ConvertIssue") == "Seek Plan")
+            .Select(c => (string?)c.Attribute("Expression"))
+            .OfType<string>()
+            .Distinct()
+            .ToList();
+
+        var noJoinPredicate = plan.Descendants(Ns + "Warnings").Any(w => (string?)w.Attribute("NoJoinPredicate") is "true" or "1");
+
+        var udfs = plan.Descendants(Ns + "UserDefinedFunction")
+            .Select(f => (string?)f.Attribute("FunctionName"))
+            .OfType<string>()
+            .Distinct()
+            .ToList();
+
+        var noStats = plan.Descendants(Ns + "ColumnsWithNoStatistics").Descendants(Ns + "ColumnReference")
+            .Select(c => string.Join('.', new[] { (string?)c.Attribute("Table"), (string?)c.Attribute("Column") }.Where(p => !string.IsNullOrEmpty(p))))
+            .Distinct()
+            .ToList();
+
+        return new PlanWarnings
+        {
+            SeekPreventingConversions = conversions,
+            NoJoinPredicate = noJoinPredicate,
+            ScalarUdfs = udfs,
+            ColumnsWithNoStatistics = noStats,
+        };
+    }
+}

src/SQLInsight.Core/Data/PlanAnalysis.cs

@@ -0,0 +1,54 @@
+namespace SQLInsight.Core.Data;
+
+/// <summary>
+/// The heaviest statements in the plan cache with what their cached (estimated) plans reveal.
+/// </summary>
+public sealed record PlanAnalysisSnapshot
+{
+    public required DateTimeOffset CapturedAt { get; init; }
+    public required IReadOnlyList<AnalyzedStatement> Statements { get; init; }
+}
+
+public sealed record AnalyzedStatement
+{
+    /// <summary>query_hash as hex; stable across recompiles and identical statements.</summary>
+    public required string QueryHash { get; init; }
+
+    public string? DatabaseName { get; init; }
+    public string? StatementText { get; init; }
+    public required long Executions { get; init; }
+    public required long TotalWorkerTimeUs { get; init; }
+    public required long TotalElapsedTimeUs { get; init; }
+    public required long TotalLogicalReads { get; init; }
+
+    /// <summary>Pages spilled to tempdb (2016 SP2+/2017 CU3+).</summary>
+    public long? TotalSpills { get; init; }
+
+    public long? TotalGrantKb { get; init; }
+    public long? TotalUsedGrantKb { get; init; }
+    public DateTime? LastExecution { get; init; }
+    public PlanWarnings Warnings { get; init; } = new();
+
+    public double AverageWorkerTimeUs => Executions == 0 ? 0 : (double)TotalWorkerTimeUs / Executions;
+}
+
+/// <summary>
+/// Problems found in a showplan.
+/// </summary>
+public sealed record PlanWarnings
+{
+    /// <summary>CONVERT_IMPLICIT expressions that prevent an index seek ("Seek Plan").</summary>
+    public IReadOnlyList<string> SeekPreventingConversions { get; init; } = [];
+
+    /// <summary>An operator joins without a predicate (a cartesian product).</summary>
+    public bool NoJoinPredicate { get; init; }
+
+    /// <summary>Scalar user-defined functions executed per row (not inlined).</summary>
+    public IReadOnlyList<string> ScalarUdfs { get; init; } = [];
+
+    /// <summary>Columns the optimizer had no statistics for.</summary>
+    public IReadOnlyList<string> ColumnsWithNoStatistics { get; init; } = [];
+
+    /// <summary>The plan was too large to analyze or could not be read.</summary>
+    public bool NotAnalyzed { get; init; }
+}

src/SQLInsight.Core/Instances/Capability.cs

@@ -44,6 +44,12 @@ public enum Capability
     /// <summary><c>sys.dm_db_log_info</c>, so VLFs can be counted without sysadmin. SQL Server 2016 SP2+.</summary>
     LogInfoDmv,
 
+    /// <summary>
+    /// Memory grant and spill columns in <c>sys.dm_exec_query_stats</c> (total_grant_kb, total_spills …).
+    /// SQL Server 2016 SP2+ and 2017 CU3+.
+    /// </summary>
+    QueryStatsMemoryColumns,
+
     /// <summary><c>sys.dm_exec_query_plan_stats</c> (last actual execution plan). SQL Server 2019+.</summary>
     LastActualPlan,
 

src/SQLInsight.Core/Instances/InstanceCapabilities.cs

@@ -44,6 +44,13 @@ public sealed class InstanceCapabilities
             }
         }
 
+        // Added in 2016 SP2 and 2017 CU3, so not a single minimum version.
+        var v = instance.Version;
+        if ((v.Major == 13 && v >= new SqlServerVersion(13, 0, 5026, 0)) || (v.Major == 14 && v >= new SqlServerVersion(14, 0, 3015, 0)) || v.Major >= 15)
+        {
+            set.Add(Capability.QueryStatsMemoryColumns);
+        }
+
         if (instance.IsHadrEnabled == true && set.Contains(Capability.AvailabilityGroupDmvs))
         {
             set.Add(Capability.HadrEnabled);

src/SQLInsight.Rules/BuiltInRules.cs

@@ -48,6 +48,11 @@ public static class BuiltInRules
         new PlanRegressionRule(),
         new FailedForcedPlanRule(),
         new ParameterSensitivityRule(),
+        new ImplicitConversionRule(),
+        new NoJoinPredicateRule(),
+        new ScalarUdfRule(),
+        new SpillRule(),
+        new ExcessiveGrantRule(),
 
         // Concurrency
         new BlockingRule(),

src/SQLInsight.Rules/Performance/PlanRules.cs

@@ -0,0 +1,185 @@
+using SQLInsight.Core.Data;
+using SQLInsight.Core.Findings;
+using SQLInsight.Core.Instances;
+using SQLInsight.Rules.Engine;
+
+namespace SQLInsight.Rules.Performance;
+
+/// <summary>
+/// Base for rules that look at the heaviest statements in the plan cache.
+/// </summary>
+public abstract class PlanRule : Rule
+{
+    public const int MaxFindingsPerRule = 10;
+
+    public override IReadOnlyCollection<Capability> RequiredCapabilities => [Capability.DynamicManagementViews];
+
+    public override RuleEvaluation Evaluate(RuleContext context)
+    {
+        ArgumentNullException.ThrowIfNull(context);
+        return Result(context.Data.Get<PlanAnalysisSnapshot>().Statements
+            .OrderByDescending(s => s.TotalWorkerTimeUs)
+            .Select(s => Check(context, s))
+            .OfType<Finding>()
+            .Take(MaxFindingsPerRule));
+    }
+
+    protected abstract Finding? Check(RuleContext context, AnalyzedStatement s);
+
+    protected static List<Evidence> Basics(AnalyzedStatement s) =>
+    [
+        new("Statement", QueryStoreText.Shorten(s.StatementText)),
+        new("Database", s.DatabaseName ?? "?"),
+        new("Udførelser", Format.Number(s.Executions)),
+        new("CPU i alt / gns.", $"{Format.Number(s.TotalWorkerTimeUs / 1_000_000.0)} s / {QueryStoreText.Ms(s.AverageWorkerTimeUs)}"),
+    ];
+
+    protected static string Subject(AnalyzedStatement s) => $"statement: {s.QueryHash}";
+}
+
+public sealed class ImplicitConversionRule : PlanRule
+{
+    public override string Id => "SQLI-PLAN-001";
+    public override string Title => "Implicit konvertering forhindrer index seek";
+
+    protected override Finding? Check(RuleContext context, AnalyzedStatement s)
+    {
+        if (s.Warnings.SeekPreventingConversions.Count == 0)
+        {
+            return null;
+        }
+
+        var evidence = Basics(s);
+        evidence.AddRange(s.Warnings.SeekPreventingConversions.Take(5).Select(c => new Evidence("Konvertering", c)));
+        return CreateFinding(
+            Severity.Medium,
+            $"Implicit konvertering forhindrer index seek i en tung forespørgsel ({s.DatabaseName})",
+            "En kolonne sammenlignes med en værdi af en anden datatype, så SQL Server må konvertere kolonnen i hver række. "
+            + "Et index på kolonnen kan derfor ikke bruges til at søge; i stedet scannes det. Den klassiske årsag er en "
+            + "applikation, der sender strenge som nvarchar til en varchar-kolonne (.NET sender string som nvarchar som standard).",
+            "Send parameteren med kolonnens datatype: i ADO.NET/Dapper SqlDbType.VarChar eller DbType.AnsiString for varchar-kolonner; "
+            + "i Entity Framework .IsUnicode(false) på egenskaben. Alternativt kan kolonnen ændres til nvarchar, hvis data kræver det.",
+            evidence,
+            subject: Subject(s),
+            references: Docs.Learn("Data type precedence", "t-sql/data-types/data-type-precedence-transact-sql"));
+    }
+}
+
+public sealed class NoJoinPredicateRule : PlanRule
+{
+    public override string Id => "SQLI-PLAN-002";
+    public override string Title => "Join uden join-betingelse (kartesisk produkt)";
+
+    // SQL Server raises the same warning for a deliberate CROSS JOIN; only a forgotten predicate is a problem.
+    protected override Finding? Check(RuleContext context, AnalyzedStatement s) =>
+        !s.Warnings.NoJoinPredicate || s.StatementText?.Contains("CROSS JOIN", StringComparison.OrdinalIgnoreCase) == true ? null : CreateFinding(
+        Severity.High,
+        $"Forespørgsel joiner uden betingelse ({s.DatabaseName})",
+        "Planen indeholder en join uden join-prædikat: hver række i den ene tabel kombineres med hver række i den anden. "
+        + "Det er næsten altid en fejl (en glemt ON- eller WHERE-betingelse ved gammeldags komma-joins) og vokser kvadratisk med datamængden.",
+        "Find forespørgslen, og tilføj den manglende join-betingelse. Brug eksplicit JOIN … ON-syntaks, så betingelsen ikke kan glemmes.",
+        Basics(s),
+        subject: Subject(s),
+        references: Docs.Learn("Joins", "relational-databases/performance/joins"));
+}
+
+public sealed class ScalarUdfRule : PlanRule
+{
+    public const long MinTotalWorkerTimeUs = 60_000_000;
+
+    public override string Id => "SQLI-PLAN-003";
+    public override string Title => "Skalar funktion kaldes for hver række";
+
+    protected override Finding? Check(RuleContext context, AnalyzedStatement s)
+    {
+        if (s.Warnings.ScalarUdfs.Count == 0 || s.TotalWorkerTimeUs < MinTotalWorkerTimeUs)
+        {
+            return null;
+        }
+
+        var evidence = Basics(s);
+        evidence.Add(new Evidence("Funktioner", string.Join(", ", s.Warnings.ScalarUdfs)));
+        var inlining = context.Instance.Version.Major >= 15
+            ? "SQL Server 2019+ kan inline skalare funktioner automatisk (compatibility level 150+), men kun hvis funktionen overholder kravene "
+              + "(fx ingen GETDATE(), ingen tabelvariabler, ingen løkker); se is_inlineable i sys.sql_modules. "
+            : string.Empty;
+        return CreateFinding(
+            Severity.Medium,
+            $"Skalar funktion kaldes for hver række i en tung forespørgsel ({s.DatabaseName})",
+            "Skalare T-SQL-funktioner udføres én gang pr. række i deres egen kontekst. Det forhindrer parallelisme for hele planen "
+            + "og gør forespørgslen langsom på store datamængder, uden at det kan ses i planens omkostninger.",
+            inlining + "Omskriv funktionen til en inline table-valued function (RETURNS TABLE AS RETURN SELECT …) og brug den med CROSS APPLY, "
+            + "eller flyt logikken direkte ind i forespørgslen.",
+            evidence,
+            subject: Subject(s),
+            references: Docs.Learn("Scalar UDF inlining", "relational-databases/user-defined-functions/scalar-udf-inlining"));
+    }
+}
+
+public sealed class SpillRule : PlanRule
+{
+    /// <summary>Average pages spilled per execution before it matters (≈ 8 MB).</summary>
+    public const double MinSpillPagesPerExecution = 1024;
+
+    public override string Id => "SQLI-PLAN-004";
+    public override string Title => "Sorteringer og hash joins spilder til tempdb";
+    public override IReadOnlyCollection<Capability> RequiredCapabilities => [Capability.QueryStatsMemoryColumns];
+
+    protected override Finding? Check(RuleContext context, AnalyzedStatement s)
+    {
+        if (s.TotalSpills is not { } spills || s.Executions == 0 || (double)spills / s.Executions < MinSpillPagesPerExecution)
+        {
+            return null;
+        }
+
+        var evidence = Basics(s);
+        evidence.Add(new Evidence("Spill pr. udførelse", $"{Format.Number(spills * 8.0 / 1024 / s.Executions)} MB"));
+        return CreateFinding(
+            Severity.Medium,
+            $"Tung forespørgsel spilder til tempdb ({s.DatabaseName})",
+            "Forespørgslen fik tildelt for lidt hukommelse til sin sortering eller hash join og måtte skrive mellemresultater til tempdb. "
+            + "Det gør den langsommere og belaster tempdb. Årsagen er næsten altid, at optimizeren har undervurderet antallet af rækker.",
+            "Find den dårlige estimering i den faktiske plan (estimeret mod faktisk antal rækker): forældede statistikker, "
+            + "tabelvariabler, lokale variabler eller komplekse prædikater. Memory grant feedback (2019+ / batch mode fra 2017) "
+            + "retter gentagne tilfælde automatisk.",
+            evidence,
+            subject: Subject(s),
+            references: Docs.Learn("Memory grant feedback", "relational-databases/performance/intelligent-query-processing-memory-grant-feedback"));
+    }
+}
+
+public sealed class ExcessiveGrantRule : PlanRule
+{
+    public const double MinAverageGrantKb = 100 * 1024;
+
+    public override string Id => "SQLI-PLAN-005";
+    public override string Title => "Forespørgsel får langt mere hukommelse, end den bruger";
+    public override IReadOnlyCollection<Capability> RequiredCapabilities => [Capability.QueryStatsMemoryColumns];
+
+    protected override Finding? Check(RuleContext context, AnalyzedStatement s)
+    {
+        if (s.TotalGrantKb is not { } granted || s.TotalUsedGrantKb is not { } used || s.Executions == 0)
+        {
+            return null;
+        }
+
+        var average = (double)granted / s.Executions;
+        if (average < MinAverageGrantKb || used > granted * 0.1)
+        {
+            return null;
+        }
+
+        var evidence = Basics(s);
+        evidence.Add(new Evidence("Tildelt / brugt pr. udførelse", $"{Format.Number(average / 1024)} MB / {Format.Number((double)used / s.Executions / 1024)} MB"));
+        return CreateFinding(
+            Severity.Medium,
+            $"Forespørgsel får tildelt {Format.Number(average / 1024)} MB, men bruger under 10 % ({s.DatabaseName})",
+            "Hukommelsen reserveres, før forespørgslen starter, og er optaget, til den er færdig. Store, overvurderede tildelinger "
+            + "tager plads fra buffer pool og får andre forespørgsler til at vente i kø (RESOURCE_SEMAPHORE).",
+            "Ret overvurderingen: typisk store varchar(max)/nvarchar(max)-kolonner i sortering, forkerte statistikker eller "
+            + "for brede SELECT-lister. Som midlertidig løsning: MAX_GRANT_PERCENT-hint eller Resource Governor.",
+            evidence,
+            subject: Subject(s),
+            references: Docs.Learn("sys.dm_exec_query_stats", "relational-databases/system-dynamic-management-views/sys-dm-exec-query-stats-transact-sql"));
+    }
+}

tests/SQLInsight.Collectors.Tests/ShowplanAnalyzerTests.cs

@@ -0,0 +1,54 @@
+using SQLInsight.Collectors.Performance;
+
+namespace SQLInsight.Collectors.Tests;
+
+public class ShowplanAnalyzerTests
+{
+    private const string Plan = """
+        <ShowPlanXML xmlns="http://schemas.microsoft.com/sqlserver/2004/07/showplan" Version="1.564" Build="16.0.4295.3">
+          <BatchSequence><Batch><Statements><StmtSimple StatementText="SELECT ...">
+            <QueryPlan>
+              <Warnings>
+                <PlanAffectingConvert ConvertIssue="Cardinality Estimate" Expression="CONVERT_IMPLICIT(nvarchar(20),[x].[Code],0)" />
+                <PlanAffectingConvert ConvertIssue="Seek Plan" Expression="CONVERT_IMPLICIT(nvarchar(20),[x].[Code],0)=[@code]" />
+              </Warnings>
+              <RelOp PhysicalOp="Nested Loops">
+                <Warnings NoJoinPredicate="true">
+                  <ColumnsWithNoStatistics><ColumnReference Table="[Orders]" Column="Status" /></ColumnsWithNoStatistics>
+                </Warnings>
+                <ComputeScalar><DefinedValues><DefinedValue><ScalarOperator>
+                  <UserDefinedFunction FunctionName="[Sales].[dbo].[fn_Price]" />
+                </ScalarOperator></DefinedValue></DefinedValues></ComputeScalar>
+              </RelOp>
+            </QueryPlan>
+          </StmtSimple></Statements></Batch></BatchSequence>
+        </ShowPlanXML>
+        """;
+
+    [Fact]
+    public void Finds_the_warnings_that_matter()
+    {
+        var warnings = ShowplanAnalyzer.Analyze(Plan);
+
+        Assert.Equal(["CONVERT_IMPLICIT(nvarchar(20),[x].[Code],0)=[@code]"], warnings.SeekPreventingConversions);
+        Assert.True(warnings.NoJoinPredicate);
+        Assert.Equal(["[Sales].[dbo].[fn_Price]"], warnings.ScalarUdfs);
+        Assert.Equal(["[Orders].Status"], warnings.ColumnsWithNoStatistics);
+        Assert.False(warnings.NotAnalyzed);
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("<not-closed")]
+    public void Missing_or_broken_plans_are_marked_not_analyzed(string? plan)
+    {
+        Assert.True(ShowplanAnalyzer.Analyze(plan).NotAnalyzed);
+    }
+
+    [Fact]
+    public void Dtd_is_rejected()
+    {
+        Assert.True(ShowplanAnalyzer.Analyze("<!DOCTYPE x [<!ENTITY e SYSTEM \"file:///etc/passwd\">]><x>&e;</x>").NotAnalyzed);
+    }
+}

tests/SQLInsight.Core.Tests/InstanceCapabilitiesTests.cs

@@ -83,3 +83,19 @@ public class InstanceCapabilitiesTests
         Assert.Equal([Capability.QueryStore, Capability.LastActualPlan], missing);
     }
 }
+
+public class QueryStatsMemoryColumnsTests
+{
+    [Theory]
+    [InlineData("13.0.4001.0", false)]
+    [InlineData("13.0.5026.0", true)]
+    [InlineData("14.0.1000.169", false)]
+    [InlineData("14.0.3015.40", true)]
+    [InlineData("15.0.2000.5", true)]
+    public void Available_from_2016_sp2_and_2017_cu3(string version, bool expected)
+    {
+        var info = new InstanceInfo { ServerName = "x", Version = SqlServerVersion.Parse(version), Edition = "e", EngineEdition = EngineEdition.Standard };
+
+        Assert.Equal(expected, InstanceCapabilities.From(info).Has(Capability.QueryStatsMemoryColumns));
+    }
+}

tests/SQLInsight.IntegrationTests/CollectorTests.cs

@@ -226,6 +226,47 @@ public class CollectorTests(SqlServerFixture sql)
         Assert.Equal("NO_INDEX", failed.Reason);
     }
 
+    [Fact]
+    public async Task Plan_analysis_finds_implicit_conversion_udf_and_missing_join_predicate()
+    {
+        await sql.ExecuteAsSaAsync($"""
+            USE [{SqlServerFixture.TestDatabase}];
+            IF OBJECT_ID(N'dbo.PlanProbe') IS NULL
+            BEGIN
+                CREATE TABLE dbo.PlanProbe (Id int IDENTITY PRIMARY KEY, Code varchar(20) NOT NULL, Price int NOT NULL);
+                INSERT dbo.PlanProbe (Code, Price) SELECT TOP (20000) CONCAT('C', ROW_NUMBER() OVER (ORDER BY (SELECT 1))), 10 FROM sys.all_objects a CROSS JOIN sys.all_objects b;
+                CREATE INDEX IX_PlanProbe_Code ON dbo.PlanProbe (Code);
+            END;
+            EXEC (N'CREATE OR ALTER FUNCTION dbo.fn_PlanProbePrice (@p int) RETURNS int AS BEGIN RETURN @p * DATEPART(ms, GETDATE()); END;');
+            """);
+        // Run in the database (not after USE in a master batch) so the plans belong to it, and put the tags inside the
+        // statements: the statement text ends at the statement, not at a trailing comment.
+        await using (var connection = new Microsoft.Data.SqlClient.SqlConnection(sql.DatabaseConnectionString(SqlServerFixture.TestDatabase)))
+        {
+            await connection.OpenAsync();
+            for (var i = 0; i < 3; i++)
+            {
+                await using var command = connection.CreateCommand();
+                command.CommandText = """
+                    EXEC sys.sp_executesql N'SELECT /* plan-probe-convert */ Id FROM dbo.PlanProbe WHERE Code = @code', N'@code nvarchar(20)', N'C42';
+                    SELECT /* plan-probe-udf */ SUM(dbo.fn_PlanProbePrice(Price)) FROM dbo.PlanProbe;
+                    SELECT /* plan-probe-cross */ a.Id, b.Id FROM dbo.PlanProbe a, dbo.CustomerData b WHERE a.Id < 3;
+                    """;
+                await command.ExecuteNonQueryAsync();
+            }
+        }
+
+        await using var session = await AgentSession.OpenAsync(sql);
+        var snapshot = await session.CollectAsync<PlanAnalysisSnapshot>(new PlanAnalysisCollector());
+
+        AnalyzedStatement Probe(string tag) => Assert.Single(snapshot.Statements, s => s.StatementText?.Contains(tag, StringComparison.Ordinal) == true);
+        Assert.NotEmpty(Probe("plan-probe-convert").Warnings.SeekPreventingConversions);
+        Assert.Contains(Probe("plan-probe-udf").Warnings.ScalarUdfs, f => f.Contains("fn_PlanProbePrice", StringComparison.Ordinal));
+        Assert.True(Probe("plan-probe-cross").Warnings.NoJoinPredicate);
+        Assert.Equal(SqlServerFixture.TestDatabase, Probe("plan-probe-udf").DatabaseName);
+        Assert.NotNull(Probe("plan-probe-udf").TotalGrantKb); // 2017 CU3+ and 2022
+    }
+
     private static async Task<DeadlockSnapshot> WaitForDeadlockAsync(AgentSession session, DeadlockCollector collector, DeadlockSnapshot previous)
     {
         // system_health dispatches events to its targets with up to 120 seconds latency.

tests/SQLInsight.Rules.Tests/PlanRuleTests.cs

@@ -0,0 +1,104 @@
+using SQLInsight.Core.Data;
+using SQLInsight.Core.Findings;
+using SQLInsight.Rules.Engine;
+using SQLInsight.Rules.Performance;
+using static SQLInsight.Rules.Tests.TestInstances;
+
+namespace SQLInsight.Rules.Tests;
+
+public class PlanRuleTests
+{
+    private static RuleResult Run(IRule rule, RuleContext context) => Assert.Single(new RuleEngine([rule]).Evaluate(context));
+
+    private static AnalyzedStatement Statement(PlanWarnings? warnings = null, long execs = 1000, long workerUs = 120_000_000, long? spills = null, long? grantKb = null, long? usedKb = null) => new()
+    {
+        QueryHash = "0x1234ABCD",
+        DatabaseName = "Sales",
+        StatementText = "SELECT * FROM dbo.Customers WHERE Code = @code",
+        Executions = execs,
+        TotalWorkerTimeUs = workerUs,
+        TotalElapsedTimeUs = workerUs,
+        TotalLogicalReads = execs * 1000,
+        TotalSpills = spills,
+        TotalGrantKb = grantKb,
+        TotalUsedGrantKb = usedKb,
+        Warnings = warnings ?? new PlanWarnings(),
+    };
+
+    private static RuleContext Plans(params AnalyzedStatement[] statements) => With(new PlanAnalysisSnapshot { CapturedAt = Now, Statements = statements });
+
+    [Fact]
+    public void Clean_plan_passes_all_plan_rules()
+    {
+        foreach (var rule in new IRule[] { new ImplicitConversionRule(), new NoJoinPredicateRule(), new ScalarUdfRule(), new SpillRule(), new ExcessiveGrantRule() })
+        {
+            Assert.Equal(RuleOutcome.Passed, Run(rule, Plans(Statement(spills: 0, grantKb: 1000, usedKb: 900))).Outcome);
+        }
+    }
+
+    [Fact]
+    public void Implicit_conversion_lists_the_expression()
+    {
+        var statement = Statement(new PlanWarnings { SeekPreventingConversions = ["CONVERT_IMPLICIT(nvarchar(20),[Sales].[dbo].[Customers].[Code],0)=[@code]"] });
+
+        var finding = Assert.Single(Run(new ImplicitConversionRule(), Plans(statement)).Findings);
+
+        Assert.Equal("statement: 0x1234ABCD", finding.Subject);
+        Assert.Contains(finding.Evidence, e => e.Value.StartsWith("CONVERT_IMPLICIT", StringComparison.Ordinal));
+        Assert.Contains("AnsiString", finding.Recommendation, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public void Missing_join_predicate_is_high()
+    {
+        Assert.Equal(Severity.High, Assert.Single(Run(new NoJoinPredicateRule(), Plans(Statement(new PlanWarnings { NoJoinPredicate = true }))).Findings).Severity);
+    }
+
+    [Fact]
+    public void Deliberate_cross_join_is_not_reported()
+    {
+        var statement = Statement(new PlanWarnings { NoJoinPredicate = true }) with { StatementText = "SELECT * FROM dbo.Sizes s CROSS JOIN dbo.Colors c" };
+
+        Assert.Equal(RuleOutcome.Passed, Run(new NoJoinPredicateRule(), Plans(statement)).Outcome);
+    }
+
+    [Fact]
+    public void Scalar_udf_only_matters_in_heavy_statements()
+    {
+        var udf = new PlanWarnings { ScalarUdfs = ["[Sales].[dbo].[fn_Price]"] };
+
+        Assert.Equal(RuleOutcome.Findings, Run(new ScalarUdfRule(), Plans(Statement(udf))).Outcome);
+        Assert.Equal(RuleOutcome.Passed, Run(new ScalarUdfRule(), Plans(Statement(udf, workerUs: 5_000_000))).Outcome);
+    }
+
+    [Fact]
+    public void Spills_per_execution_above_threshold()
+    {
+        Assert.Equal(RuleOutcome.Findings, Run(new SpillRule(), Plans(Statement(execs: 100, spills: 500_000))).Outcome);
+        Assert.Equal(RuleOutcome.Passed, Run(new SpillRule(), Plans(Statement(execs: 100, spills: 5_000))).Outcome);
+    }
+
+    [Fact]
+    public void Oversized_grant_used_under_ten_percent()
+    {
+        Assert.Equal(RuleOutcome.Findings, Run(new ExcessiveGrantRule(), Plans(Statement(execs: 10, grantKb: 10 * 500 * 1024, usedKb: 10 * 20 * 1024))).Outcome);
+        Assert.Equal(RuleOutcome.Passed, Run(new ExcessiveGrantRule(), Plans(Statement(execs: 10, grantKb: 10 * 500 * 1024, usedKb: 10 * 300 * 1024))).Outcome);
+    }
+
+    [Fact]
+    public void Memory_rules_are_not_measurable_without_the_columns()
+    {
+        Assert.Equal(RuleOutcome.NotMeasurable, Run(new SpillRule(), WithVersion("14.0.1000.169", new PlanAnalysisSnapshot { CapturedAt = Now, Statements = [] })).Outcome);
+    }
+
+    [Fact]
+    public void Findings_are_capped_and_heaviest_first()
+    {
+        var statements = Enumerable.Range(1, 30).Select(i => Statement(new PlanWarnings { NoJoinPredicate = true }, workerUs: i * 1_000_000) with { QueryHash = $"0x{i:X4}" }).ToArray();
+
+        var findings = Run(new NoJoinPredicateRule(), Plans(statements)).Findings;
+
+        Assert.Equal(PlanRule.MaxFindingsPerRule, findings.Count);
+        Assert.Equal("statement: 0x001E", findings[0].Subject);
+    }
+}