From f1d3f5188e405669cf75f227de156d6a8117bb3c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 07:40:09 +0000 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01E3EExwndy8Z22EK2Qpt6BK --- .../BuiltInCollectors.cs | 1 + .../Performance/PlanAnalysisCollector.cs | 68 +++++++ .../Performance/ShowplanAnalyzer.cs | 63 ++++++ src/SQLInsight.Core/Data/PlanAnalysis.cs | 54 +++++ src/SQLInsight.Core/Instances/Capability.cs | 6 + .../Instances/InstanceCapabilities.cs | 7 + src/SQLInsight.Rules/BuiltInRules.cs | 5 + src/SQLInsight.Rules/Performance/PlanRules.cs | 185 ++++++++++++++++++ .../ShowplanAnalyzerTests.cs | 54 +++++ .../InstanceCapabilitiesTests.cs | 16 ++ .../CollectorTests.cs | 41 ++++ tests/SQLInsight.Rules.Tests/PlanRuleTests.cs | 104 ++++++++++ 12 files changed, 604 insertions(+) create mode 100644 src/SQLInsight.Collectors/Performance/PlanAnalysisCollector.cs create mode 100644 src/SQLInsight.Collectors/Performance/ShowplanAnalyzer.cs create mode 100644 src/SQLInsight.Core/Data/PlanAnalysis.cs create mode 100644 src/SQLInsight.Rules/Performance/PlanRules.cs create mode 100644 tests/SQLInsight.Collectors.Tests/ShowplanAnalyzerTests.cs create mode 100644 tests/SQLInsight.Rules.Tests/PlanRuleTests.cs diff --git a/src/SQLInsight.Collectors/BuiltInCollectors.cs b/src/SQLInsight.Collectors/BuiltInCollectors.cs index bf1d667..2f36b9f 100644 --- a/src/SQLInsight.Collectors/BuiltInCollectors.cs +++ b/src/SQLInsight.Collectors/BuiltInCollectors.cs @@ -22,5 +22,6 @@ public static class BuiltInCollectors new BlockingCollector(), new DeadlockCollector(), new QueryStoreCollector(), + new PlanAnalysisCollector(), ]; } diff --git a/src/SQLInsight.Collectors/Performance/PlanAnalysisCollector.cs b/src/SQLInsight.Collectors/Performance/PlanAnalysisCollector.cs new file mode 100644 index 0000000..21c91ca --- /dev/null +++ b/src/SQLInsight.Collectors/Performance/PlanAnalysisCollector.cs @@ -0,0 +1,68 @@ +using SQLInsight.Core.Data; +using SQLInsight.Core.Instances; + +namespace SQLInsight.Collectors.Performance; + +/// +/// The heaviest statements (CPU, last 24 hours) from the plan cache, with their cached plans analyzed. +/// Plans are fetched as text with sys.dm_exec_text_query_plan: sys.dm_exec_query_plan returns NULL +/// for plans nested deeper than the xml type allows. TOP is applied before any plan is fetched. +/// +public sealed class PlanAnalysisCollector : Collector +{ + public const int TopStatements = 50; + + public override string Name => "plan-analysis"; + public override TimeSpan DefaultInterval => TimeSpan.FromMinutes(30); + public override IReadOnlyCollection RequiredCapabilities => [Capability.DynamicManagementViews]; + + protected override async Task 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; + """; + } +} diff --git a/src/SQLInsight.Collectors/Performance/ShowplanAnalyzer.cs b/src/SQLInsight.Collectors/Performance/ShowplanAnalyzer.cs new file mode 100644 index 0000000..bc7d3dc --- /dev/null +++ b/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; + +/// +/// Reads the warnings that matter out of a showplan XML document. +/// +internal static class ShowplanAnalyzer +{ + private static readonly XNamespace Ns = "http://schemas.microsoft.com/sqlserver/2004/07/showplan"; + + /// Plans larger than this are skipped: parsing them costs more than it tells. + 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() + .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() + .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, + }; + } +} diff --git a/src/SQLInsight.Core/Data/PlanAnalysis.cs b/src/SQLInsight.Core/Data/PlanAnalysis.cs new file mode 100644 index 0000000..def941d --- /dev/null +++ b/src/SQLInsight.Core/Data/PlanAnalysis.cs @@ -0,0 +1,54 @@ +namespace SQLInsight.Core.Data; + +/// +/// The heaviest statements in the plan cache with what their cached (estimated) plans reveal. +/// +public sealed record PlanAnalysisSnapshot +{ + public required DateTimeOffset CapturedAt { get; init; } + public required IReadOnlyList Statements { get; init; } +} + +public sealed record AnalyzedStatement +{ + /// query_hash as hex; stable across recompiles and identical statements. + 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; } + + /// Pages spilled to tempdb (2016 SP2+/2017 CU3+). + 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; +} + +/// +/// Problems found in a showplan. +/// +public sealed record PlanWarnings +{ + /// CONVERT_IMPLICIT expressions that prevent an index seek ("Seek Plan"). + public IReadOnlyList SeekPreventingConversions { get; init; } = []; + + /// An operator joins without a predicate (a cartesian product). + public bool NoJoinPredicate { get; init; } + + /// Scalar user-defined functions executed per row (not inlined). + public IReadOnlyList ScalarUdfs { get; init; } = []; + + /// Columns the optimizer had no statistics for. + public IReadOnlyList ColumnsWithNoStatistics { get; init; } = []; + + /// The plan was too large to analyze or could not be read. + public bool NotAnalyzed { get; init; } +} diff --git a/src/SQLInsight.Core/Instances/Capability.cs b/src/SQLInsight.Core/Instances/Capability.cs index d0dd29d..8d114c2 100644 --- a/src/SQLInsight.Core/Instances/Capability.cs +++ b/src/SQLInsight.Core/Instances/Capability.cs @@ -44,6 +44,12 @@ public enum Capability /// sys.dm_db_log_info, so VLFs can be counted without sysadmin. SQL Server 2016 SP2+. LogInfoDmv, + /// + /// Memory grant and spill columns in sys.dm_exec_query_stats (total_grant_kb, total_spills …). + /// SQL Server 2016 SP2+ and 2017 CU3+. + /// + QueryStatsMemoryColumns, + /// sys.dm_exec_query_plan_stats (last actual execution plan). SQL Server 2019+. LastActualPlan, diff --git a/src/SQLInsight.Core/Instances/InstanceCapabilities.cs b/src/SQLInsight.Core/Instances/InstanceCapabilities.cs index d9a534e..d249726 100644 --- a/src/SQLInsight.Core/Instances/InstanceCapabilities.cs +++ b/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); diff --git a/src/SQLInsight.Rules/BuiltInRules.cs b/src/SQLInsight.Rules/BuiltInRules.cs index 4ea6aea..b7de065 100644 --- a/src/SQLInsight.Rules/BuiltInRules.cs +++ b/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(), diff --git a/src/SQLInsight.Rules/Performance/PlanRules.cs b/src/SQLInsight.Rules/Performance/PlanRules.cs new file mode 100644 index 0000000..71c14e5 --- /dev/null +++ b/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; + +/// +/// Base for rules that look at the heaviest statements in the plan cache. +/// +public abstract class PlanRule : Rule +{ + public const int MaxFindingsPerRule = 10; + + public override IReadOnlyCollection RequiredCapabilities => [Capability.DynamicManagementViews]; + + public override RuleEvaluation Evaluate(RuleContext context) + { + ArgumentNullException.ThrowIfNull(context); + return Result(context.Data.Get().Statements + .OrderByDescending(s => s.TotalWorkerTimeUs) + .Select(s => Check(context, s)) + .OfType() + .Take(MaxFindingsPerRule)); + } + + protected abstract Finding? Check(RuleContext context, AnalyzedStatement s); + + protected static List 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 +{ + /// Average pages spilled per execution before it matters (≈ 8 MB). + 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 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 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")); + } +} diff --git a/tests/SQLInsight.Collectors.Tests/ShowplanAnalyzerTests.cs b/tests/SQLInsight.Collectors.Tests/ShowplanAnalyzerTests.cs new file mode 100644 index 0000000..697553f --- /dev/null +++ b/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 = """ + + + + + + + + + + + + + + + + + + + """; + + [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("]>&e;").NotAnalyzed); + } +} diff --git a/tests/SQLInsight.Core.Tests/InstanceCapabilitiesTests.cs b/tests/SQLInsight.Core.Tests/InstanceCapabilitiesTests.cs index 9751ddf..900289a 100644 --- a/tests/SQLInsight.Core.Tests/InstanceCapabilitiesTests.cs +++ b/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)); + } +} diff --git a/tests/SQLInsight.IntegrationTests/CollectorTests.cs b/tests/SQLInsight.IntegrationTests/CollectorTests.cs index 653613b..f3a96ae 100644 --- a/tests/SQLInsight.IntegrationTests/CollectorTests.cs +++ b/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(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 WaitForDeadlockAsync(AgentSession session, DeadlockCollector collector, DeadlockSnapshot previous) { // system_health dispatches events to its targets with up to 120 seconds latency. diff --git a/tests/SQLInsight.Rules.Tests/PlanRuleTests.cs b/tests/SQLInsight.Rules.Tests/PlanRuleTests.cs new file mode 100644 index 0000000..1076492 --- /dev/null +++ b/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); + } +}